Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Greg KH @ 2016-11-25  9:53 UTC (permalink / raw)
  To: Mark Lord
  Cc: Francois Romieu, David Miller, hayeswang-Rasf1IRRPZFBDgjK7y7TUQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ed0c9790-57be-8bca-cdd6-3a54ca24f0bd-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>

On Thu, Nov 24, 2016 at 10:49:33PM -0500, Mark Lord wrote:
> There is no possibility for them to be used for anything other than
> USB receive buffers, for this driver only.  Nothing in the driver
> or kernel ever writes to those buffers after initial allocation,
> and only the driver and USB host controller ever have pointers to the buffers.

You really are going to have to break out that USB monitor to verify
that this is the data coming across the wire.  Note, there are "cheap"
USB monitors that can be quite handy and that work on Linux:
	http://www.totalphase.com/products/beagle-usb12/

Or most high-end scopes have a USB mode that you can use to catch stuff
like this (but they are usually harder to use/trigger and only store a
very limited buffer).

good luck!

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH net v2 0/3] Fix OdroidC2 Gigabit Tx link issue
From: Jerome Brunet @ 2016-11-25  9:55 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Florian Fainelli, Carlo Caione, Kevin Hilman, Giuseppe Cavallaro,
	Alexandre TORGUE, Andre Roth, Neil Armstrong,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAFBinCCg8cLuQdrRsxHA53JAvyWgfW7vfVMho0tj3bCCcu-YxA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, 2016-11-24 at 18:10 +0100, Martin Blumenstingl wrote:
> On Thu, Nov 24, 2016 at 5:01 PM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> wrote:
> > 
> > On Thu, 2016-11-24 at 15:40 +0100, Martin Blumenstingl wrote:
> > > 
> > > Hi Jerome,
> > > 
> > > On Mon, Nov 21, 2016 at 4:35 PM, Jerome Brunet <jbrunet@baylibre.
> > > com>
> > > wrote:
> > > > 
> > > > 
> > > > This patchset fixes an issue with the OdroidC2 board (DWMAC +
> > > > RTL8211F).
> > > > Initially reported as a low Tx throughput issue at gigabit
> > > > speed,
> > > > the
> > > > platform enters LPI too often. This eventually break the link
> > > > (both
> > > > Tx
> > > > and Rx), and require to bring the interface down and up again
> > > > to
> > > > get the
> > > > Rx path working again.
> > > > 
> > > > The root cause of this issue is not fully understood yet but
> > > > disabling EEE
> > > > advertisement on the PHY prevent this feature to be negotiated.
> > > > With this change, the link is stable and reliable, with the
> > > > expected
> > > > throughput performance.
> > > I have just sent a series which allows configuring the TX delay
> > > on
> > > the
> > > MAC (dwmac-meson8b glue) side: [0]
> > > Disabling the TX delay generated by the MAC fixes TX throughput
> > > for
> > > me, even when leaving EEE enabled in the RTL8211F PHY driver!
> > > 
> > > Unfortunately the RTL8211F PHY is a black-box for the community
> > > because there is no public datasheeet available.
> > > *maybe* (pure speculation!) they're enabling the TX delay based
> > > on
> > > some internal magic only when EEE is enabled.
> > 
> > Hi already tried acting on the register setting the TX_delay. I
> > also
> > tried on the PHY. I never been able to improve situation on the
> > Odroic2. Only disabling EEE improved the situation.
> OK, thanks for clarifying this!
> 
> > 
> > To make sure, i tried again with your patch but the result remains
> > unchanged. With Tx_delay disabled (either the mac or the phy), the
> > situation is even worse, it seems that nothing gets through
> This is interesting, because in your case you should have a 4ns TX
> delay (2ns from the MAC and presumably 2ns from the PHY).
> Maybe that is also the reason why the TX delay is configurable in 2ns
> steps in PRG_ETHERNET0 on Amlogic SoCs.
> 
> out of curiosity: have you tried setting a 4ns (half clock-cycle) TX
> delay for the MAC and disabling it in the PHY?

Just replied on the other thread. Long story short, Odroidc2 seems to
really require EEE to be switched off.

Again, thx for your help Martin

> 
> 
> Regards,
> Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 net-next 1/2] net: ethernet: slicoss: add slicoss gigabit ethernet driver
From: Greg KH @ 2016-11-25  9:59 UTC (permalink / raw)
  To: Lino Sanfilippo
  Cc: devel, andrew, f.fainelli, netdev, linux-kernel, liodot, davem
In-Reply-To: <1480029185-14140-2-git-send-email-LinoSanfilippo@gmx.de>

On Fri, Nov 25, 2016 at 12:13:04AM +0100, Lino Sanfilippo wrote:
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2015,6 +2015,24 @@
>  #define PCI_SUBDEVICE_ID_CCD_OV4S	0xE888
>  #define PCI_SUBDEVICE_ID_CCD_OV8S	0xE998
>  
> +#define PCI_VENDOR_ID_ALACRITECH		0x139A
> +#define PCI_DEVICE_ID_ALACRITECH_MOAVE		0x0005

<snip>

Please read the top of this file for why you should not need to ever
modify this file again :)

thanks,

greg k-h

^ permalink raw reply

* [PATCH] net: fec: turn on device when extracting statistics
From: Nikita Yushchenko @ 2016-11-25 10:02 UTC (permalink / raw)
  To: Fugang Duan, David S. Miller, Troy Kisky, Andrew Lunn,
	Eric Nelson, Philippe Reynes, Johannes Berg, netdev, linux-kernel
  Cc: Chris Healy, Nikita Yushchenko

Execution 'ethtool -S' on fec device that is down causes OOPS on Vybrid
board:

Unhandled fault: external abort on non-linefetch (0x1008) at 0xe0898200
pgd = ddecc000
[e0898200] *pgd=9e406811, *pte=400d1653, *ppte=400d1453
Internal error: : 1008 [#1] SMP ARM
...

Reason of OOPS is that fec_enet_get_ethtool_stats() accesses fec
registers while IPG clock is stopped by PM.

Fix that by wrapping statistics extraction into pm_runtime_get_sync()
... pm_runtime_put_autosuspend() braces.

Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5aa9d4ded214..9c7592b80ce8 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2317,10 +2317,19 @@ static void fec_enet_get_ethtool_stats(struct net_device *dev,
 	struct ethtool_stats *stats, u64 *data)
 {
 	struct fec_enet_private *fep = netdev_priv(dev);
-	int i;
+	int i, ret;
+
+	ret = pm_runtime_get_sync(&fep->pdev->dev);
+	if (IS_ERR_VALUE(ret)) {
+		memset(data, 0, sizeof(*data) * ARRAY_SIZE(fec_stats));
+		return;
+	}
 
 	for (i = 0; i < ARRAY_SIZE(fec_stats); i++)
 		data[i] = readl(fep->hwp + fec_stats[i].offset);
+
+	pm_runtime_mark_last_busy(&fep->pdev->dev);
+	pm_runtime_put_autosuspend(&fep->pdev->dev);
 }
 
 static void fec_enet_get_strings(struct net_device *netdev,
-- 
2.1.4

^ permalink raw reply related

* Aw: Re: [PATCH v2 net-next 1/2] net: ethernet: slicoss: add slicoss gigabit ethernet driver
From: Lino Sanfilippo @ 2016-11-25 10:22 UTC (permalink / raw)
  To: Greg KH; +Cc: devel, andrew, f.fainelli, netdev, linux-kernel, liodot, davem
In-Reply-To: <20161125095947.GA21048@kroah.com>

Hi,

>
> On Fri, Nov 25, 2016 at 12:13:04AM +0100, Lino Sanfilippo wrote:
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -2015,6 +2015,24 @@
> >  #define PCI_SUBDEVICE_ID_CCD_OV4S	0xE888
> >  #define PCI_SUBDEVICE_ID_CCD_OV8S	0xE998
> >  
> > +#define PCI_VENDOR_ID_ALACRITECH		0x139A
> > +#define PCI_DEVICE_ID_ALACRITECH_MOAVE		0x0005
> 
> <snip>
> 
> Please read the top of this file for why you should not need to ever
> modify this file again :)
> 

Youre right, I totally overlooked the comment. Will fix this in the next version,
thanks!

Regards,
Lino

^ permalink raw reply

* Re: [PATCH 3/8] irda: w83977af_ir: Remove unnecessary blank lines
From: Sergei Shtylyov @ 2016-11-25 10:31 UTC (permalink / raw)
  To: Joe Perches, Samuel Ortiz; +Cc: Arnd Bergmann, netdev, linux-kernel
In-Reply-To: <27a5cbfc2f59a978ad84ff19ddd76d6a333adfba.1480014088.git.joe@perches.com>

Hello.

On 11/24/2016 10:10 PM, Joe Perches wrote:

> These just add unnecessary vertical whitespace.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/net/irda/w83977af_ir.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
> index 4ad91f4f867f..5aa61413aea8 100644
> --- a/drivers/net/irda/w83977af_ir.c
> +++ b/drivers/net/irda/w83977af_ir.c
[...]
> @@ -553,6 +552,7 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
>  static void w83977af_dma_write(struct w83977af_ir *self, int iobase)
>  {
>  	__u8 set;
> +
>  	pr_debug("%s(), len=%d\n", __func__, self->tx_buff.len);
>
>  	/* Save current set */

    Removing, really? :-)

MBR, Sergei

^ permalink raw reply

* [patch v2 net-next] sfc: remove unneeded variable
From: Dan Carpenter @ 2016-11-25 10:43 UTC (permalink / raw)
  To: Solarflare linux maintainers, Edward Cree
  Cc: Bert Kenward, netdev, kernel-janitors
In-Reply-To: <698affdd-d4e0-53c0-fff0-9b66252504a0@solarflare.com>

We don't use ->heap_buf after commit 46d1efd852cc ("sfc: remove Software
TSO") so let's remove the last traces.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index f97f828..18aceb2 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -139,8 +139,6 @@ struct efx_special_buffer {
  * struct efx_tx_buffer - buffer state for a TX descriptor
  * @skb: When @flags & %EFX_TX_BUF_SKB, the associated socket buffer to be
  *	freed when descriptor completes
- * @heap_buf: When @flags & %EFX_TX_BUF_HEAP, the associated heap buffer to be
- *	freed when descriptor completes.
  * @option: When @flags & %EFX_TX_BUF_OPTION, a NIC-specific option descriptor.
  * @dma_addr: DMA address of the fragment.
  * @flags: Flags for allocation and DMA mapping type
@@ -151,10 +149,7 @@ struct efx_special_buffer {
  * Only valid if @unmap_len != 0.
  */
 struct efx_tx_buffer {
-	union {
-		const struct sk_buff *skb;
-		void *heap_buf;
-	};
+	const struct sk_buff *skb;
 	union {
 		efx_qword_t option;
 		dma_addr_t dma_addr;
@@ -166,7 +161,6 @@ struct efx_tx_buffer {
 };
 #define EFX_TX_BUF_CONT		1	/* not last descriptor of packet */
 #define EFX_TX_BUF_SKB		2	/* buffer is last part of skb */
-#define EFX_TX_BUF_HEAP		4	/* buffer was allocated with kmalloc() */
 #define EFX_TX_BUF_MAP_SINGLE	8	/* buffer was mapped with dma_map_single() */
 #define EFX_TX_BUF_OPTION	0x10	/* empty buffer for option descriptor */
 
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index 1aa728c..bb07034 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -84,8 +84,6 @@ static void efx_dequeue_buffer(struct efx_tx_queue *tx_queue,
 		netif_vdbg(tx_queue->efx, tx_done, tx_queue->efx->net_dev,
 			   "TX queue %d transmission id %x complete\n",
 			   tx_queue->queue, tx_queue->read_count);
-	} else if (buffer->flags & EFX_TX_BUF_HEAP) {
-		kfree(buffer->heap_buf);
 	}
 
 	buffer->len = 0;

^ permalink raw reply related

* RE: [PATCH net v2 4/5] net: fsl/fman: fix fixed-link-phydev reference leak
From: Madalin-Cristian Bucur @ 2016-11-25 10:54 UTC (permalink / raw)
  To: Johan Hovold, David S. Miller
  Cc: Florian Fainelli, Timur Tabi, Andrew Lunn, Vivien Didelot,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1480011691-13278-5-git-send-email-johan@kernel.org>

> -----Original Message-----
> From: Johan Hovold [mailto:jhovold@gmail.com] On Behalf Of Johan Hovold
> Sent: Thursday, November 24, 2016 8:22 PM
> 
> Make sure to drop the reference taken by of_phy_find_device() when
> looking up a fixed-link phydev during probe.
> 
> Fixes: 57ba4c9b56d8 ("fsl/fman: Add FMan MAC support")
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/net/ethernet/freescale/fman/mac.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c
> b/drivers/net/ethernet/freescale/fman/mac.c
> index 8fe6b3e253fa..736db9d9b0ad 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -892,6 +892,8 @@ static int mac_probe(struct platform_device *_of_dev)
>  		priv->fixed_link->duplex = phy->duplex;
>  		priv->fixed_link->pause = phy->pause;
>  		priv->fixed_link->asym_pause = phy->asym_pause;
> +
> +		put_device(&phy->mdio.dev);
>  	}
> 
>  	err = mac_dev->init(mac_dev);
> --
> 2.7.3

Thank you,
Madalin

^ permalink raw reply

* Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: Sebastian Frias @ 2016-11-25 11:13 UTC (permalink / raw)
  To: Florian Fainelli, Martin Blumenstingl, Jerome Brunet
  Cc: mark.rutland, devicetree, Mans Rullgard, alexandre.torgue,
	Andrew Lunn, khilman, peppe.cavallaro, robh+dt, netdev, carlo,
	linux-amlogic, davem, linux-arm-kernel
In-Reply-To: <e6ca0941-e2e3-dd93-d4d3-8fbd76b60e17@gmail.com>

On 24/11/16 19:55, Florian Fainelli wrote:
> Le 24/11/2016 à 09:05, Martin Blumenstingl a écrit :
>> On Thu, Nov 24, 2016 at 4:56 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
>>> On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
>>>> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
>>>> cycle TX clock delay. This seems to work fine for many boards (for
>>>> example Odroid-C2 or Amlogic's reference boards) but there are some
>>>> others where TX traffic is simply broken.
>>>> There are probably multiple reasons why it's working on some boards
>>>> while it's broken on others:
>>>> - some of Amlogic's reference boards are using a Micrel PHY
>>>> - hardware circuit design
>>>> - maybe more...
>>>>
>>>> This raises a question though:
>>>> Which device is supposed to enable the TX delay when both MAC and PHY
>>>> support it? And should we implement it for each PHY / MAC separately
>>>> or should we think about a more generic solution (currently it's not
>>>> possible to disable the TX delay generated by the RTL8211F PHY via
>>>> devicetree when using phy-mode "rgmii")?
>>>
>>> Actually you can skip the part which activate the Tx-delay on the phy
>>> by setting "phy-mode = "rgmii-id" instead of "rgmii"
>>>
>>> phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
>>> but PHY_INTERFACE_MODE_RGMII_ID.
>> unfortunately this is not true for RTL8211F (I did my previous tests
>> with the same expectation in mind)!
>> the code seems to suggest that TX-delay is disabled whenever mode !=
>> PHY_INTERFACE_MODE_RGMII.
>> BUT: on my device RTL8211F_TX_DELAY is set even before
>> "phy_write(phydev, 0x11, reg);"!

If you look at the Atheros 803x PHY and its driver
'drivers/net/phy/at803x.c':
- by default (as HW reset preset) the PHY has RX delay enabled, TX
delay disabled
- the driver only enables RX, or TX, or both, according to "rgmii-rxid",
"rgmii-txid", or "rgmii-id" respectively, but does not alter HW reset
presets. In other words:
  a "rgmii-rxid" results in RX enabled (expected)
  b "rgmii-txid" results in RX *and* TX enabled (unexpected?)
  c "rgmii-id"   results in RX *and* TX enabled (expected)
  d "rgmii"      results in RX enabled (unexpected?)

This is a bit surprising and I think that some boards and PHY<->MAC
combinations are working a little bit by chance, unless I'm missing
something.

> 
> (Adding Sebastian (and Mans, and Andrew) since he raised the same
> question a while ago. I think I now understand a bit better what
> Sebastian was after a couple of weeks ago)
> 

Thanks for CCing us, it is indeed a very similar issue.

>>
>> Based on what I found it seems that rgmii-id, rgmii-txid and
>> rgmii-rxid are supposed to be handled by the PHY.
> 
> Correct, the meaning of PHY_INTERFACE_MODE should be from the
> perspective of the PHY device:
> 
> - PHY_INTERFACE_MODE_RGMII_TXID means that the PHY is responsible for
> adding a delay when the MAC transmits (TX MAC -> PHY (delay) -> wire)
> - PHY_INTERFACE_MODE_RGMII_RXID means that the PHY is responsible for
> adding a delay when the MAC receives (RX MAC <- (delay) PHY) <- wire)
> 

Thanks for the explanation.
Actually I had thought that the delay was to account for board routing
(wires) between the MAC and the PHY.
From your explanation it appears that the delay is to account for board
routing (wires) between the PHY and the RJ45 socket.

>> That would mean that we have two problems here:
>> 1) drivers/net/phy/realtek.c:rtl8211f_config_init should check for
>> PHY_INTERFACE_MODE_RGMII_ID or PHY_INTERFACE_MODE_RGMII_TXID and
>> enable the TX-delay in that case - otherwise explicitly disable it
> 
> Agreed.
> 
>> 2) dwmac-meson8b.c should only use the configured TX-delay for
>> PHY_INTERFACE_MODE_RGMII
>> @Florian: could you please share your thoughts on this (who handles
>> the TX delay in which case)?
> 
> This also seems reasonable to do, provided that the PHY is also properly
> configured not to add delays in both directions, and therefore assumes
> that the MAC does it.
> 
> We have a fairly large problem with how RGMII delays are done in PHYLIB
> and Ethernet MAC drivers (or just in general), where we can't really
> intersect properly what a PHY is supporting (in terms of internal
> delays), and what the MAC supports either. One possible approach could
> be to update PHY drivers a list of PHY_INTERFACE_MODE_* that they
> support (ideally, even with normalized nanosecond delay values), 

Just to make sure I understood this, the DT would say something like:

phy-connection-type = "rgmii-txid";
txid-delay-ns = <3>;

For a 3ns TX delay, would that be good?

>and
> then intersect that with the requested phy_interface_t during
> phy_{attach,connect} time, and feed this back to the MAC with a special
> error code/callback, so we could gracefully try to choose another
> PHY_INTERFACE_MODE_* value that the MAC supports....
> 
> A larger problem is that a number of drivers have been deployed, and
> Device Trees, possibly with the meaning of "phy-mode" and
> "phy-connection-type" being from the MAC perspective, and not the PHY
> perspective *sigh*, good luck auditing those.
> 
> So from there, here is possibly what we could do
> 
> - submit a series of patches that update the PHYLIB documentation (there
> are other things missing here) and make it clear from which entity (PHY
> or MAC) does the delay apply to, document the "intersection" problem here

I think documenting is necessary, thanks in advance!

However, I'm wondering if there's a way to make this work in all cases.
Indeed, if we consider for example that TX delay is required, we have 4
cases:

       PHY         |       MAC          | Who applies?
TXID supported     | TXID supported     | PHY
TXID supported     | TXID not supported | PHY
TXID not supported | TXID supported     | MAC
TXID not supported | TXID not supported | cannot be done

That is basically what my patch:

https://marc.info/?l=linux-netdev&m=147869658031783&w=2

attempted to achieve. That would allow more combinations of MAC<->PHY to
work, right?

Nevertheless, I think we also need to keep in mind that most of this
discussion assumes the case where both, MAC and PHY have equal capabilities.
Could it happen that the PHY supports only 2ns delay, and the MAC only
1ns delay?
Could it happen that the delay is bigger than what is supported by
either the PHY or MAC alone? maybe if combined it could work, for example
a 3ns delay required and the PHY supporting 2ns and the MAC 1ns, combined
it could work?

I don't know if these are cases worth supporting, nor if they are valid.

> 
> - have you document the configured behavior for dwmac-meson8b that we
> just discussed here in v2 of this patch series
> 
> Sorry for the long post, here is a virtual potato: 0
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE()
From: Mark Rutland @ 2016-11-25 11:22 UTC (permalink / raw)
  To: Michael S. Tsirkin, Christian Borntraeger
  Cc: linux-kernel, dave, dbueso, dvyukov, jasowang, kvm, netdev,
	paulmck, virtualization
In-Reply-To: <20161124222357-mutt-send-email-mst@kernel.org>

On Thu, Nov 24, 2016 at 10:36:58PM +0200, Michael S. Tsirkin wrote:
> On Thu, Nov 24, 2016 at 10:25:11AM +0000, Mark Rutland wrote:
> > For several reasons, it would be beneficial to kill off ACCESS_ONCE()
> > tree-wide, in favour of {READ,WRITE}_ONCE(). These work with aggregate types,
> > more obviously document their intended behaviour, and are necessary for tools
> > like KTSAN to work correctly (as otherwise reads and writes cannot be
> > instrumented separately).
> > 
> > While it's possible to script the bulk of this tree-wide conversion, some cases
> > such as the virtio code, require some manual intervention. This series moves
> > the virtio and vringh code over to {READ,WRITE}_ONCE(), in the process fixing a
> > bug in the virtio headers.
> > 
> > Thanks,
> > Mark.
> 
> I don't have a problem with this specific patchset.

Good to hear. :)

Does that mean you're happy to queue these patches? Or would you prefer
a new posting at some later point, with ack/review tags accumulated?

> Though I really question the whole _ONCE APIs esp with
> aggregate types - these seem to generate a memcpy and
> an 8-byte read/writes sometimes, and I'm pretty sure this simply
> can't be read/written at once on all architectures.

Yes, in cases where the access is larger than the machine can perform in
a single access, this will result in a memcpy.

My understanding is that this has always been the case with
ACCESS_ONCE(), where multiple accesses were silently/implicitly
generated by the compiler.

We could add some compile-time warnings for those cases. I'm not sure if
there's a reason we avoided doing that so far; perhaps Christian has a
some idea.

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE()
From: Christian Borntraeger @ 2016-11-25 11:33 UTC (permalink / raw)
  To: Mark Rutland, Michael S. Tsirkin
  Cc: dave, kvm, dbueso, netdev, linux-kernel, virtualization, paulmck,
	dvyukov
In-Reply-To: <20161125112203.GA26611@leverpostej>

On 11/25/2016 12:22 PM, Mark Rutland wrote:
> On Thu, Nov 24, 2016 at 10:36:58PM +0200, Michael S. Tsirkin wrote:
>> On Thu, Nov 24, 2016 at 10:25:11AM +0000, Mark Rutland wrote:
>>> For several reasons, it would be beneficial to kill off ACCESS_ONCE()
>>> tree-wide, in favour of {READ,WRITE}_ONCE(). These work with aggregate types,
>>> more obviously document their intended behaviour, and are necessary for tools
>>> like KTSAN to work correctly (as otherwise reads and writes cannot be
>>> instrumented separately).
>>>
>>> While it's possible to script the bulk of this tree-wide conversion, some cases
>>> such as the virtio code, require some manual intervention. This series moves
>>> the virtio and vringh code over to {READ,WRITE}_ONCE(), in the process fixing a
>>> bug in the virtio headers.
>>>
>>> Thanks,
>>> Mark.
>>
>> I don't have a problem with this specific patchset.
> 
> Good to hear. :)
> 
> Does that mean you're happy to queue these patches? Or would you prefer
> a new posting at some later point, with ack/review tags accumulated?
> 
>> Though I really question the whole _ONCE APIs esp with
>> aggregate types - these seem to generate a memcpy and
>> an 8-byte read/writes sometimes, and I'm pretty sure this simply
>> can't be read/written at once on all architectures.
> 
> Yes, in cases where the access is larger than the machine can perform in
> a single access, this will result in a memcpy.
> 
> My understanding is that this has always been the case with
> ACCESS_ONCE(), where multiple accesses were silently/implicitly
> generated by the compiler.
> 
> We could add some compile-time warnings for those cases. I'm not sure if
> there's a reason we avoided doing that so far; perhaps Christian has a
> some idea.

My first version had this warning, but it was removed later on as requested
by Linus

http://lkml.iu.edu/hypermail/linux/kernel/1503.3/02670.html
---snip---

Get rid of the f*cking size checks etc on READ_ONCE() and friends.

They are about - wait for it - "reading a value once".

Note how it doesn't say ANYTHING about "atomic" or anything like that.
It's about reading *ONCE*.

---snip---

^ permalink raw reply

* Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: Måns Rullgård @ 2016-11-25 12:01 UTC (permalink / raw)
  To: Sebastian Frias
  Cc: Florian Fainelli, Martin Blumenstingl, Jerome Brunet,
	linux-amlogic, devicetree, netdev, davem, khilman, mark.rutland,
	robh+dt, linux-arm-kernel, alexandre.torgue, peppe.cavallaro,
	carlo, Andrew Lunn
In-Reply-To: <6a6af561-4e83-ca6e-d989-f421e18bce1e@laposte.net>

Sebastian Frias <sf84@laposte.net> writes:

> On 24/11/16 19:55, Florian Fainelli wrote:
>> Le 24/11/2016 à 09:05, Martin Blumenstingl a écrit :
>>> Based on what I found it seems that rgmii-id, rgmii-txid and
>>> rgmii-rxid are supposed to be handled by the PHY.
>> 
>> Correct, the meaning of PHY_INTERFACE_MODE should be from the
>> perspective of the PHY device:
>> 
>> - PHY_INTERFACE_MODE_RGMII_TXID means that the PHY is responsible for
>> adding a delay when the MAC transmits (TX MAC -> PHY (delay) -> wire)
>> - PHY_INTERFACE_MODE_RGMII_RXID means that the PHY is responsible for
>> adding a delay when the MAC receives (RX MAC <- (delay) PHY) <- wire)
>> 
>
> Thanks for the explanation.
> Actually I had thought that the delay was to account for board routing
> (wires) between the MAC and the PHY.
> From your explanation it appears that the delay is to account for board
> routing (wires) between the PHY and the RJ45 socket.

The delay pertains to the RGMII link between MAC and PHY.  The external
connection is self-clocking.

-- 
Måns Rullgård

^ permalink raw reply

* Re: [PATCH] net: stmmac: enable tx queue 0 for gmac4 IPs synthesized with multiple TX queues
From: Niklas Cassel @ 2016-11-25 12:10 UTC (permalink / raw)
  To: Alexandre Torgue, Giuseppe Cavallaro; +Cc: netdev, linux-kernel
In-Reply-To: <0516ebe9-6b66-5991-1f96-1cc10178977a@st.com>

On 11/24/2016 07:11 PM, Alexandre Torgue wrote:
> Hi Niklas,

Hello Alexandre

>
> On 11/24/2016 03:36 PM, Niklas Cassel wrote:
>> From: Niklas Cassel <niklas.cassel@axis.com>
>>
>> The dwmac4 IP can synthesized with 1-8 number of tx queues.
>> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
>> by default. For these IPs, the bitfield TXQEN is R/W.
>>
>> Always enable tx queue 0. The write will have no effect on IPs synthesized
>> with DWC_EQOS_NUM_TXQ == 1.
>>
>> The driver does still not utilize more than one tx queue in the IP.
>>
>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     |  3 +++
>>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++++++-
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> index 6f4f5ce25114..3e8d4fefa5e0 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> @@ -155,8 +155,11 @@ enum power_event {
>>  #define MTL_CHAN_RX_DEBUG(x)        (MTL_CHANX_BASE_ADDR(x) + 0x38)
>>
>>  #define MTL_OP_MODE_RSF            BIT(5)
>> +#define MTL_OP_MODE_TXQEN        BIT(3)
>>  #define MTL_OP_MODE_TSF            BIT(1)
>>
>> +#define MTL_OP_MODE_TQS_MASK        GENMASK(24, 16)
>> +
>>  #define MTL_OP_MODE_TTC_MASK        0x70
>>  #define MTL_OP_MODE_TTC_SHIFT        4
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>> index 116151cd6a95..577316de6ba8 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>> @@ -213,7 +213,17 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>>          else
>>              mtl_tx_op |= MTL_OP_MODE_TTC_512;
>>      }
>> -
>> +    /* For an IP with DWC_EQOS_NUM_TXQ == 1, the fields TXQEN and TQS are RO
>> +     * with reset values: TXQEN on, TQS == DWC_EQOS_TXFIFO_SIZE.
>> +     * For an IP with DWC_EQOS_NUM_TXQ > 1, the fields TXQEN and TQS are R/W
>> +     * with reset values: TXQEN off, TQS 256 bytes.
>> +     *
>> +     * Write the bits in both cases, since it will have no effect when RO.
>> +     * For DWC_EQOS_NUM_TXQ > 1, the top bits in MTL_OP_MODE_TQS_MASK might
>> +     * be RO, however, writing the whole TQS field will result in a value
>> +     * equal to DWC_EQOS_TXFIFO_SIZE, just like for DWC_EQOS_NUM_TXQ == 1.
>> +     */
>> +    mtl_tx_op |= MTL_OP_MODE_TXQEN | MTL_OP_MODE_TQS_MASK;
>
> Your patch sounds good. Just one question:
>
> In synopsys databook I use, I see that MTL_OP_MODE_TXQEN for channel 2 can take several values "disabled / enabled / Enabled in AV mode":
>
> Transmit Queue Enable
> This field is used to enable/disable the transmit queue 1. 00 R/W
> ■ 2'b00 - Not enabled
> ■ 2'b01 - Enable in AV mode (Reserved when Enable Audio Video
> Bridging is not selected while configuring the core)
> ■ 2'b10 - Enabled
> ■ 2'b11 - Reserved
>
> Do you plan to manage av mode in a future patch ?

We are not planning on using the AV mode.
We will probably not use TXQ1 at all.

I noticed that the MAC_HW_Feature2 Register actually has a TXQCNT field.
It is currently saved in priv->dma_cap.number_tx_channel.
If you prefer, I could do a patch V2 where we only set the bits if
priv->dma_cap.number_tx_channel > 1

However, I don't think the current patch is too bad either, since the bits
are RO when number_tx_channel == 1.


>
> Regards
> Alex
>
>>      writel(mtl_tx_op, ioaddr +  MTL_CHAN_TX_OP_MODE(channel));
>>
>>      mtl_rx_op = readl(ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>>

^ permalink raw reply

* Re: [PATCH] net: stmmac: enable tx queue 0 for gmac4 IPs synthesized with multiple TX queues
From: Niklas Cassel @ 2016-11-25 12:14 UTC (permalink / raw)
  To: Alexandre Torgue, Giuseppe Cavallaro; +Cc: netdev, linux-kernel
In-Reply-To: <95ed6d38-cc5e-402f-6772-710b6ad0c930@axis.com>

On 11/25/2016 01:10 PM, Niklas Cassel wrote:
> On 11/24/2016 07:11 PM, Alexandre Torgue wrote:
>> Hi Niklas,
> Hello Alexandre
>
>> On 11/24/2016 03:36 PM, Niklas Cassel wrote:
>>> From: Niklas Cassel <niklas.cassel@axis.com>
>>>
>>> The dwmac4 IP can synthesized with 1-8 number of tx queues.
>>> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
>>> by default. For these IPs, the bitfield TXQEN is R/W.
>>>
>>> Always enable tx queue 0. The write will have no effect on IPs synthesized
>>> with DWC_EQOS_NUM_TXQ == 1.
>>>
>>> The driver does still not utilize more than one tx queue in the IP.
>>>
>>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>>> ---
>>>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     |  3 +++
>>>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++++++-
>>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>> index 6f4f5ce25114..3e8d4fefa5e0 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>> @@ -155,8 +155,11 @@ enum power_event {
>>>  #define MTL_CHAN_RX_DEBUG(x)        (MTL_CHANX_BASE_ADDR(x) + 0x38)
>>>
>>>  #define MTL_OP_MODE_RSF            BIT(5)
>>> +#define MTL_OP_MODE_TXQEN        BIT(3)
>>>  #define MTL_OP_MODE_TSF            BIT(1)
>>>
>>> +#define MTL_OP_MODE_TQS_MASK        GENMASK(24, 16)
>>> +
>>>  #define MTL_OP_MODE_TTC_MASK        0x70
>>>  #define MTL_OP_MODE_TTC_SHIFT        4
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>> index 116151cd6a95..577316de6ba8 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>> @@ -213,7 +213,17 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>>>          else
>>>              mtl_tx_op |= MTL_OP_MODE_TTC_512;
>>>      }
>>> -
>>> +    /* For an IP with DWC_EQOS_NUM_TXQ == 1, the fields TXQEN and TQS are RO
>>> +     * with reset values: TXQEN on, TQS == DWC_EQOS_TXFIFO_SIZE.
>>> +     * For an IP with DWC_EQOS_NUM_TXQ > 1, the fields TXQEN and TQS are R/W
>>> +     * with reset values: TXQEN off, TQS 256 bytes.
>>> +     *
>>> +     * Write the bits in both cases, since it will have no effect when RO.
>>> +     * For DWC_EQOS_NUM_TXQ > 1, the top bits in MTL_OP_MODE_TQS_MASK might
>>> +     * be RO, however, writing the whole TQS field will result in a value
>>> +     * equal to DWC_EQOS_TXFIFO_SIZE, just like for DWC_EQOS_NUM_TXQ == 1.
>>> +     */
>>> +    mtl_tx_op |= MTL_OP_MODE_TXQEN | MTL_OP_MODE_TQS_MASK;
>> Your patch sounds good. Just one question:
>>
>> In synopsys databook I use, I see that MTL_OP_MODE_TXQEN for channel 2 can take several values "disabled / enabled / Enabled in AV mode":
>>
>> Transmit Queue Enable
>> This field is used to enable/disable the transmit queue 1. 00 R/W
>> ■ 2'b00 - Not enabled
>> ■ 2'b01 - Enable in AV mode (Reserved when Enable Audio Video
>> Bridging is not selected while configuring the core)
>> ■ 2'b10 - Enabled
>> ■ 2'b11 - Reserved
>>
>> Do you plan to manage av mode in a future patch ?
> We are not planning on using the AV mode.
> We will probably not use TXQ1 at all.
>
> I noticed that the MAC_HW_Feature2 Register actually has a TXQCNT field.
> It is currently saved in priv->dma_cap.number_tx_channel.
> If you prefer, I could do a patch V2 where we only set the bits if
> priv->dma_cap.number_tx_channel > 1

Oh, sorry, that was number of tx _channels_,
not number of tx _queues_.

However, we could add a number_tx_queue to struct dma_features,
if you would prefer that.

>
> However, I don't think the current patch is too bad either, since the bits
> are RO when number_tx_channel == 1.
>
>
>> Regards
>> Alex
>>
>>>      writel(mtl_tx_op, ioaddr +  MTL_CHAN_TX_OP_MODE(channel));
>>>
>>>      mtl_rx_op = readl(ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>>>

^ permalink raw reply

* Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE()
From: Mark Rutland @ 2016-11-25 12:23 UTC (permalink / raw)
  To: Christian Borntraeger, peterz
  Cc: Michael S. Tsirkin, linux-kernel, dave, dbueso, dvyukov, jasowang,
	kvm, netdev, paulmck, virtualization
In-Reply-To: <32dfca07-59f3-b75a-3154-cf6b6c8538f0@de.ibm.com>

On Fri, Nov 25, 2016 at 12:33:48PM +0100, Christian Borntraeger wrote:
> On 11/25/2016 12:22 PM, Mark Rutland wrote:
> > On Thu, Nov 24, 2016 at 10:36:58PM +0200, Michael S. Tsirkin wrote:
> >> Though I really question the whole _ONCE APIs esp with
> >> aggregate types - these seem to generate a memcpy and
> >> an 8-byte read/writes sometimes, and I'm pretty sure this simply
> >> can't be read/written at once on all architectures.
> > 
> > Yes, in cases where the access is larger than the machine can perform in
> > a single access, this will result in a memcpy.
> > 
> > My understanding is that this has always been the case with
> > ACCESS_ONCE(), where multiple accesses were silently/implicitly
> > generated by the compiler.
> > 
> > We could add some compile-time warnings for those cases. I'm not sure if
> > there's a reason we avoided doing that so far; perhaps Christian has a
> > some idea.
> 
> My first version had this warning, but it was removed later on as requested
> by Linus
> 
> http://lkml.iu.edu/hypermail/linux/kernel/1503.3/02670.html
> ---snip---
> 
> Get rid of the f*cking size checks etc on READ_ONCE() and friends.
> 
> They are about - wait for it - "reading a value once".
> 
> Note how it doesn't say ANYTHING about "atomic" or anything like that.
> It's about reading *ONCE*.
> 
> ---snip---

I see. That's unfortunate, given that practically every use I'm aware of
assumes some atomicity (e.g. freedom from tearing when loading/storing
pointers or values up to the native width of the machine). I believe
that's the case here, for virtio, for example.

Perhaps we can add new accessors that are supposed to guarantee that,
into which we can drop appropriate warnings.

Naming will be problematic; calling them ATOMIC_* makes tham sound like
they work on atomic_t. That and I have no idea how to ensure correct
usage tree-wide; I'm not sure if/how Coccinelle can help.

Peter, thoughts?

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord @ 2016-11-25 12:34 UTC (permalink / raw)
  To: Greg KH
  Cc: Francois Romieu, David Miller, hayeswang-Rasf1IRRPZFBDgjK7y7TUQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161125095350.GA20653-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

On 16-11-25 04:53 AM, Greg KH wrote:
> Note, there are "cheap" USB monitors that can be quite handy and that work on Linux:
> 	http://www.totalphase.com/products/beagle-usb12/

USD$455/each in quantity, vs. USD$8 for the USB ethernet dongle.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord @ 2016-11-25 12:35 UTC (permalink / raw)
  To: Hayes Wang, David Miller
  Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB201056317@RTITMBSV03.realtek.com.tw>

On 16-11-25 01:51 AM, Hayes Wang wrote:
>
> Forgive me. I provide wrong information. This is about RTL8153, not RTL8152.

No problem.  Thanks for trying though.

^ permalink raw reply

* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord @ 2016-11-25 12:36 UTC (permalink / raw)
  To: Hayes Wang, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: nic_swsd, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB201056266-JIZ+AM9kKNzuvTFwvkocLypo8c9IxeqyAjHCUHv49ws@public.gmane.org>

On 16-11-25 01:11 AM, Hayes Wang wrote:
> Mark Lord [mailto:mlord-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org]
..
>> If that "return 0" statement is ever executed, doesn't it result
>> in the loss/leak of a buffer?
> 
> They would be found back by calling rtl_start_rx(), when the rx
> is restarted.

Good. I figured it was probably something like that, but wasn't
entirely sure about the control flow around stop/restart there.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE()
From: Peter Zijlstra @ 2016-11-25 12:40 UTC (permalink / raw)
  To: Mark Rutland
  Cc: dave, kvm, dbueso, netdev, Michael S. Tsirkin, linux-kernel,
	virtualization, paulmck, Linus Torvalds, dvyukov
In-Reply-To: <20161125122356.GB26611@leverpostej>

On Fri, Nov 25, 2016 at 12:23:56PM +0000, Mark Rutland wrote:
> Naming will be problematic; calling them ATOMIC_* makes tham sound like
> they work on atomic_t. That and I have no idea how to ensure correct
> usage tree-wide; I'm not sure if/how Coccinelle can help.
> 
> Peter, thoughts?

Something like so perhaps?

---

#ifdef CONFIG_DEBUG_ATOMIC_SLEEP
#define WARN_SINGLE_COPY_ALIGNMENT(ptr)	\
	WARN_ON_ONCE(((unsigned long)(ptr)) & (sizeof(*(ptr))-1))
#else
#define WARN_SINGLE_COPY_ALIGNMENT(ptr)
#endif

/*
 * Provide accessors for Single-Copy atomicy.
 *
 * That is, ensure that machine word sized loads/stores to naturally
 * aligned variables are single instructions.
 *
 * By reason of not being able to use C11 atomic crud, use our beloved
 * volatile qualifier. Since volatile tells the compiler the value can
 * be changed behind its back, it must use Single-Copy atomic loads and
 * stores to access them, otherwise it runs the risk of load/store
 * tearing.
 */

#define SINGLE_LOAD(x)						\
{(								\
	compiletime_assert_atomic_type(typeof(x));		\
	WARN_SINGLE_COPY_ALIGNMENT(&(x));			\
	READ_ONCE(x);						\
})

#define SINGLE_STORE(x, v)					\
({								\
	compiletime_assert_atomic_type(typeof(x));		\
	WARN_SINGLE_COPY_ALIGNMENT(&(x));			\
	WRITE_ONCE(x, v);					\
})

^ permalink raw reply

* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord @ 2016-11-25 12:41 UTC (permalink / raw)
  To: Greg KH
  Cc: Francois Romieu, David Miller, hayeswang, netdev, nic_swsd,
	linux-kernel, linux-usb
In-Reply-To: <4e370db9-f6b2-edd4-537b-d5a45b2ddea1@pobox.com>

On 16-11-25 07:34 AM, Mark Lord wrote:
> On 16-11-25 04:53 AM, Greg KH wrote:
>> Note, there are "cheap" USB monitors that can be quite handy and that work on Linux:
>> 	http://www.totalphase.com/products/beagle-usb12/
> 
> USD$455/each in quantity, vs. USD$8 for the USB ethernet dongle.

Oh, wrong model.  That one doesn't do USB2.
The USB2 version is a mere USD$1300 in quantity.

Seems like rather a lot of money just to report a bug in a USB driver.
Perhaps the Linux Foundation might purchase one and loan it for this task?

^ permalink raw reply

* Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE()
From: Peter Zijlstra @ 2016-11-25 12:44 UTC (permalink / raw)
  To: Mark Rutland
  Cc: dave, kvm, dbueso, netdev, Michael S. Tsirkin, linux-kernel,
	virtualization, paulmck, Linus Torvalds, dvyukov
In-Reply-To: <20161125124044.GN3092@twins.programming.kicks-ass.net>

On Fri, Nov 25, 2016 at 01:40:44PM +0100, Peter Zijlstra wrote:
> #define SINGLE_LOAD(x)						\
> {(								\
> 	compiletime_assert_atomic_type(typeof(x));		\

Should be:

	compiletime_assert_atomic_type(x);

> 	WARN_SINGLE_COPY_ALIGNMENT(&(x));			\
> 	READ_ONCE(x);						\
> })
> 
> #define SINGLE_STORE(x, v)					\
> ({								\
> 	compiletime_assert_atomic_type(typeof(x));		\

idem

> 	WARN_SINGLE_COPY_ALIGNMENT(&(x));			\
> 	WRITE_ONCE(x, v);					\
> })

^ permalink raw reply

* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord @ 2016-11-25 12:49 UTC (permalink / raw)
  To: Greg KH
  Cc: Francois Romieu, David Miller, hayeswang, netdev, nic_swsd,
	linux-kernel, linux-usb
In-Reply-To: <20161125095350.GA20653@kroah.com>

On 16-11-25 04:53 AM, Greg KH wrote:
> On Thu, Nov 24, 2016 at 10:49:33PM -0500, Mark Lord wrote:
>> There is no possibility for them to be used for anything other than
>> USB receive buffers, for this driver only.  Nothing in the driver
>> or kernel ever writes to those buffers after initial allocation,
>> and only the driver and USB host controller ever have pointers to the buffers.
> 
> You really are going to have to break out that USB monitor to verify
> that this is the data coming across the wire.

Not sure why, because there really is no other way for the data to
appear where it does at the beginning of that URB buffer.

This does seem a rather unexpected burden to place upon someone
reporting a regression in a USB network driver that corrupts user data.

I have already spent about 50 hours looking at this issue,
and everything now points firmly at some kind of FIFO overflow
within the dongle itself.  There is no evidence to the contrary.

I am very happy to test any driver updates, or data collection mods
provided by the author, to help the author find/fix the issue.

One idea, might be to have the author try testing with the dongle
connected through a USB1.1 hub, forcing it to slower speeds.
This might make reproducing the issue (if indeed a FIFO overflow)
easier, as the host transfers will then be slower than the
ethernet wire speed.

I have access to the hardware here next Tuesday.
If we can scrounge up the USB analyzer, cables, and a suitable
MS-Windows (ugh) machine of some kind, then I'll see if it can
be programmed to somewhow capture the event.  Probably just set it
in continuous capture mode, and have the target system halt
when it sees bad data at offset zero.

This can take days to reproduce, so don't hold your breaths.

Something useful to do in the meanwhile, is to then think
about "what next" after the analyzer confirms the issue.
-- 
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com

^ permalink raw reply

* [PATCH v2 4/7] ARM64: dts: meson-gxbb-odroidc2: add reset for the ethernet PHY
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
  To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
	mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
	will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 238fbea..cbaf024 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -143,10 +143,25 @@
 	pinctrl-names = "default";
 };
 
+&mdio0 {
+	ethernet_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+	};
+};
+
 &ethmac {
 	status = "okay";
 	pinctrl-0 = <&eth_rgmii_pins>;
 	pinctrl-names = "default";
+
+	phy-handle = <&ethernet_phy0>;
+
+	snps,reset-gpio = <&gpio GPIOZ_14 0>;
+	snps,reset-delays-us = <0 10000 1000000>;
+	snps,reset-active-low;
+
+	phy-mode = "rgmii";
 };
 
 &ir {
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 5/7] ARM64: dts: meson-gxbb-p20x: add reset for the ethernet PHY
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
  To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
	mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
	will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 203be28..2abc553 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -134,10 +134,25 @@
 	pinctrl-names = "default";
 };
 
+&mdio0 {
+	ethernet_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+	};
+};
+
 &ethmac {
 	status = "okay";
 	pinctrl-0 = <&eth_rgmii_pins>;
 	pinctrl-names = "default";
+
+	phy-handle = <&ethernet_phy0>;
+
+	snps,reset-gpio = <&gpio GPIOZ_14 0>;
+	snps,reset-delays-us = <0 10000 1000000>;
+	snps,reset-active-low;
+
+	phy-mode = "rgmii";
 };
 
 &ir {
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 6/7] ARM64: dts: meson-gxbb-vega-s95: add reset for the ethernet PHY
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
  To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
	mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
	will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index e59ad30..a0e92e3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -113,10 +113,25 @@
 	pinctrl-names = "default";
 };
 
+&mdio0 {
+	ethernet_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+	};
+};
+
 &ethmac {
 	status = "okay";
 	pinctrl-0 = <&eth_rgmii_pins>;
 	pinctrl-names = "default";
+
+	phy-handle = <&ethernet_phy0>;
+
+	snps,reset-gpio = <&gpio GPIOZ_14 0>;
+	snps,reset-delays-us = <0 10000 1000000>;
+	snps,reset-active-low;
+
+	phy-mode = "rgmii";
 };
 
 &usb0_phy {
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox