* Re: [RFC PATCH 1/2] gianfar: convert to phylink
From: Vladimir Oltean @ 2019-08-24 15:21 UTC (permalink / raw)
To: Arseny Solokha
Cc: Claudiu Manoil, Russell King, Ioana Ciornei, Andrew Lunn, netdev,
Florian Fainelli
In-Reply-To: <87lfwfio13.fsf@eldim>
Hi Arseny.
On Tue, 30 Jul 2019 at 17:40, Arseny Solokha <asolokha@kb.kras.ru> wrote:
>
> > Hi Arseny,
> >
> > Nice project!
>
> Vladimir, Russell, thanks for your review. I'm on vacation now, so won't fully
> address your comments in a few weeks: while I can build the code, I won't have
> access to hardware to test.
>
> So it seems this patch will turn into a series where we'll have some cleanup
> patches preceding the actual conversion (and the latter will also contain a
> documentation change in Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
> which I've overlooked in the first submission). I'll try to post trivial
> cleanups independently while still on vacation.
>
Yes, ideally the cleanup would be separate from the conversion.
>
> >> @@ -891,11 +912,21 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
> >>
> >> err = of_property_read_string(np, "phy-connection-type", &ctype);
> >>
> >> - /* We only care about rgmii-id. The rest are autodetected */
> >> - if (err == 0 && !strcmp(ctype, "rgmii-id"))
> >> - priv->interface = PHY_INTERFACE_MODE_RGMII_ID;
> >> - else
> >> + /* We only care about rgmii-id and sgmii - the former
> >> + * is indistinguishable from rgmii in hardware, and phylink needs
> >> + * the latter to be set appropriately for correct phy configuration.
> >> + * The rest are autodetected
> >> + */
> >> + if (err == 0) {
> >> + if (!strcmp(ctype, "rgmii-id"))
> >> + priv->interface = PHY_INTERFACE_MODE_RGMII_ID;
> >> + else if (!strcmp(ctype, "sgmii"))
> >> + priv->interface = PHY_INTERFACE_MODE_SGMII;
> >> + else
> >> + priv->interface = PHY_INTERFACE_MODE_MII;
> >> + } else {
> >> priv->interface = PHY_INTERFACE_MODE_MII;
> >> + }
> >>
> >
> > No. Don't do this. Just do:
> >
> > err = of_get_phy_mode(np);
> > if (err < 0)
> > goto err_grp_init;
> >
> > priv->interface = err;
> >
> >> if (of_find_property(np, "fsl,magic-packet", NULL))
> >> priv->device_flags |= FSL_GIANFAR_DEV_HAS_MAGIC_PACKET;
> >> @@ -903,19 +934,21 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
> >> if (of_get_property(np, "fsl,wake-on-filer", NULL))
> >> priv->device_flags |= FSL_GIANFAR_DEV_HAS_WAKE_ON_FILER;
> >>
> >> - priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
> >> + priv->device_node = np;
> >> + priv->speed = SPEED_UNKNOWN;
> >>
> >> - /* In the case of a fixed PHY, the DT node associated
> >> - * to the PHY is the Ethernet MAC DT node.
> >> - */
> >> - if (!priv->phy_node && of_phy_is_fixed_link(np)) {
> >> - err = of_phy_register_fixed_link(np);
> >> - if (err)
> >> - goto err_grp_init;
> >> + priv->phylink_config.dev = &priv->ndev->dev;
> >> + priv->phylink_config.type = PHYLINK_NETDEV;
> >>
> >> - priv->phy_node = of_node_get(np);
> >> + phylink = phylink_create(&priv->phylink_config, of_fwnode_handle(np),
> >> + priv->interface, &gfar_phylink_ops);
> >
> > You introduced a bug here.
> > of_phy_connect used to take the PHY interface type (for good or bad)
> > from gfar_get_interface() (which is reconstructing it from the MAC
> > registers).
> > You are now passing the PHY interface type to phylink_create from the
> > "phy-connection-type" DT property.
> > At the very least, you are breaking LS1021A which uses phy-mode
> > instead of phy-connection-type (hence my comment above to use the
> > generic OF helper).
> > Actually I think you just uncovered a latent bug, in that the DT
> > bindings for phy-mode didn't mean much at all to the driver - it would
> > rely on what the bootloader had set up.
> > Actually DT bindings for phy-connection-type were most likely simply
> > bolt on on top of gianfar when they figured they couldn't just
> > auto-detect the various species of required RGMII delays.
> > But gfar_get_interface is a piece of history that was introduced in
> > the same commit as the enum phy_interface_t itself: e8a2b6a42073
> > ("[PATCH] PHY: Add support for configuring the PHY connection
> > interface"). Its time has come.
>
> <…>
>
> >> }
> >>
> >> + priv->tbi_phy = NULL;
> >> + interface = gfar_get_interface(dev);
> >
> > Be consistent and just go for priv->interface. Nobody's changing it anyway.
>
> So if I get you right, I'm supposed to drop gfar_get_interface() and rely on DT
> bindings entirely?
>
Oof, I checked arch/powerpc/boot/dts/fsl and the following boards
using the gianfar driver are guilty of populating phy-handle but not
phy-connection-type:
mpc8540ads
mpc8541cds
mpc8548cds_32b
mpc8548cds_36b
mpc8555cds
mpc8560ads
mpc8568mds
ppa8548
I think a sane logic for populating priv->interface would be:
- Attempt of_get_phy_mode
- If phy-mode or phy-connection-type properties are not found, revert
to gfar_get_interface for the legacy blobs above.
>
> >> @@ -3387,23 +3384,6 @@ static irqreturn_t gfar_interrupt(int irq, void *grp_id)
> >> return IRQ_HANDLED;
> >> }
> >>
> >> -/* Called every time the controller might need to be made
> >> - * aware of new link state. The PHY code conveys this
> >> - * information through variables in the phydev structure, and this
> >> - * function converts those variables into the appropriate
> >> - * register values, and can bring down the device if needed.
> >> - */
> >> -static void adjust_link(struct net_device *dev)
> >> -{
> >> - struct gfar_private *priv = netdev_priv(dev);
> >> - struct phy_device *phydev = dev->phydev;
> >> -
> >> - if (unlikely(phydev->link != priv->oldlink ||
> >> - (phydev->link && (phydev->duplex != priv->oldduplex ||
> >> - phydev->speed != priv->oldspeed))))
> >> - gfar_update_link_state(priv);
> >> -}
> >
> > Getting rid of the cruft from this function deserves its own patch.
>
> How am I supposed to remove it without breaking the PHYLIB-based driver? Or do
> you mean making it call gfar_update_link_state() just before the conversion
> which will then remove adjust_link() altogether?
>
I don't know, if you can't refactor without breaking anything then ok.
>
> >>
> >> if (unlikely(test_bit(GFAR_RESETTING, &priv->state)))
> >> return;
> >>
> >> - if (phydev->link) {
> >> - u32 tempval1 = gfar_read(®s->maccfg1);
> >> - u32 tempval = gfar_read(®s->maccfg2);
> >> - u32 ecntrl = gfar_read(®s->ecntrl);
> >> - u32 tx_flow_oldval = (tempval1 & MACCFG1_TX_FLOW);
> >> + if (unlikely(phylink_autoneg_inband(mode)))
> >> + return;
> >>
> >> - if (phydev->duplex != priv->oldduplex) {
> >> - if (!(phydev->duplex))
> >> - tempval &= ~(MACCFG2_FULL_DUPLEX);
> >> - else
> >> - tempval |= MACCFG2_FULL_DUPLEX;
> >> + maccfg1 = gfar_read(®s->maccfg1);
> >> + maccfg2 = gfar_read(®s->maccfg2);
> >> + ecntrl = gfar_read(®s->ecntrl);
> >>
> >> - priv->oldduplex = phydev->duplex;
> >> - }
> >> + new_maccfg2 = maccfg2 & ~(MACCFG2_FULL_DUPLEX | MACCFG2_IF);
> >> + new_ecntrl = ecntrl & ~ECNTRL_R100;
> >>
> >> - if (phydev->speed != priv->oldspeed) {
> >> - switch (phydev->speed) {
> >> - case 1000:
> >> - tempval =
> >> - ((tempval & ~(MACCFG2_IF)) | MACCFG2_GMII);
> >> + if (state->duplex)
> >> + new_maccfg2 |= MACCFG2_FULL_DUPLEX;
> >>
> >> - ecntrl &= ~(ECNTRL_R100);
> >> - break;
> >> - case 100:
> >> - case 10:
> >> - tempval =
> >> - ((tempval & ~(MACCFG2_IF)) | MACCFG2_MII);
> >> -
> >> - /* Reduced mode distinguishes
> >> - * between 10 and 100
> >> - */
> >> - if (phydev->speed == SPEED_100)
> >> - ecntrl |= ECNTRL_R100;
> >> - else
> >> - ecntrl &= ~(ECNTRL_R100);
> >> - break;
> >> - default:
> >> - netif_warn(priv, link, priv->ndev,
> >> - "Ack! Speed (%d) is not 10/100/1000!\n",
> >> - phydev->speed);
> >
> > Please 1. remove "Ack!" 2. treat SPEED_UNKNOWN here by setting the MAC
> > into a low-power state (e.g. 10 Mbps - the power savings are real).
> > Don't print that Speed -1 is not 10/100/1000, we know that.
>
> In my first conversion attempt I see "Ack!" when changing link speed on when
> shutting it down, so switching to 10 Mbps doesn't seem right for me—hence early
> return in this case. Maybe I'm doing something wrong here.
>
When mac_config calls with SPEED_UNKNOWN, the link is down, and you
can put the MAC in the lowest energy state it can go to (10 Mbps, in
this case). Or so I've been told. Maybe Russell can chime in. Anyway,
you don't need to print anything, there's lots of prints from PHYLINK
already.
>
> >> diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
> >> index 3433b46b90c1..146b30d07789 100644
> >> --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
> >> +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
> >> @@ -35,7 +35,7 @@
> >> #include <asm/types.h>
> >> #include <linux/ethtool.h>
> >> #include <linux/mii.h>
> >> -#include <linux/phy.h>
> >> +#include <linux/phylink.h>
> >> #include <linux/sort.h>
> >> #include <linux/if_vlan.h>
> >> #include <linux/of_platform.h>
> >> @@ -207,12 +207,10 @@ static void gfar_get_regs(struct net_device *dev, struct ethtool_regs *regs,
> >> static unsigned int gfar_usecs2ticks(struct gfar_private *priv,
> >> unsigned int usecs)
> >> {
> >> - struct net_device *ndev = priv->ndev;
> >> - struct phy_device *phydev = ndev->phydev;
> >
> > Are you sure this still works? You missed a ndev->phydev check from
> > gfar_gcoalesce, where this is called from. Technically you can still
> > check ndev->phydev, it's just that PHYLINK doesn't guarantee you'll
> > have one (e.g. fixed-link interfaces).
>
> It still works for RGMII PHYs, SGMII and 1000Base-X in my testing. I didn't
> check it with fixed-link, though.
>
>
> >> @@ -1519,6 +1472,24 @@ static int gfar_get_ts_info(struct net_device *dev,
> >> return 0;
> >> }
> >>
> >> +/* Set link ksettings (phy address, speed) for ethtools */
> >
> > ethtool, not ethtools. Also, I'm not quite sure what you mean by
> > setting the "phy address" with ethtool.
>
> Well, I know where I've copied it from… Thanks for pointing it out.
Regards,
-Vladimir
^ permalink raw reply
* Re: [PATCH net-next 0/1] Add BASE-T1 PHY support
From: Heiner Kallweit @ 2019-08-24 15:03 UTC (permalink / raw)
To: Christian Herber, Andrew Lunn
Cc: davem@davemloft.net, Florian Fainelli, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <AM6PR0402MB3798C702793071E34A5659ED86A50@AM6PR0402MB3798.eurprd04.prod.outlook.com>
On 22.08.2019 09:18, Christian Herber wrote:
> On 21.08.2019 20:57, Andrew Lunn wrote:
>>
>>> The current patch set IMO is a little bit hacky. I'm not 100% happy
>>> with the implicit assumption that there can't be devices supporting
>>> T1 and classic BaseT modes or fiber modes.
>>
>>> Andrew: Do you have an opinion on that?
>>
>> Hi Heiner
>>
>> I would also like cleaner integration. I doubt here is anything in the
>> standard which says you cannot combine these modes. It is more a
>> marketing question if anybody would build such a device. Maybe not
>> directly into a vehicle, but you could imaging a mobile test device
>> which uses T1 to talk to the car and T4 to connect to the garage
>> network?
>>
>> So i don't think we should limit ourselves. phylib should provide a
>> clean, simple set of helpers to perform standard operations for
>> various modes. Drivers can make use of those helpers. That much should
>> be clear. If we try to make genphy support them all simultaneously, is
>> less clear.
>>
>> Andrew
>>
>
> If you want to go down this path, then i think we have to ask some more
> questions. Clause 45 is a very scalable register scheme, it is not a
> specific class of devices and will be extended and extended.
>
> Currently, the phy-c45.c supports 10/100/1000/2500/5000/10000 Mbps
> consumer/enterprise PHYs. This is also an implicit assumption. The
> register set (e.g. on auto-neg) used for this will also only support
> these modes and nothing more, as it is done scaling.
>
> Currently not supported, but already present in IEEE 802.3:
> - MultiGBASE-T (25/40 Gbps) (see e.g. MultiGBASE-T AN control 1 register)
> - BASE-T1
> - 10BASE-T1
> - NGBASE-T1
>
> And surely there are some on the way or already there that I am not
> aware of.
>
> To me, one architectural decision point is if you want to have generic
> support for all C45 PHYs in one file, or if you want to split it by
> device class. I went down the first path with my patch, as this is the
> road gone also with the existing code.
>
> If you want to split BASE-T1, i think you will need one basic C45
> library (genphy_c45_pma_read_abilities() is a good example of a function
> that is not specific to a device class). On the other hand,
> genphy_c45_pma_setup_forced() is not a generic function at this point as
> it supports only a subset of devices managed in C45.
>
> I tend to agree with you that splitting is the best way to go in the
> long run, but that also requires a split of the existing phy-c45.c into
> two IMHO.
>
BASE-T1 seems to be based on Clause 45 (at least Clause 45 MDIO),
but it's not fully compliant with Clause 45. Taking AN link status
as an example: 45.2.7.2.7 states that link-up is signaled in bit 7.1.2.
If BASE-T1 uses a different register, then it's not fully Clause 45
compatible.
Therefore also my question for the datasheet of an actual BASE-T1 PHY,
as I would be curious whether it shadows the link-up bit from 7.513.2
to 7.1.2 to be Clause 45 compliant. Definitely reading bit 7.513.2
is nothing that belongs into a genphy_c45_ function.
The extension to genphy_c45_pma_read_abilities() looks good to me,
for the other parts I'd like to see first how real world BASE-T1 PHYs
handle it. If they shadow the T1-specific bits to the Clause 45
standard ones, we should be fine. Otherwise IMO we have to add
separate T1 functions to phylib.
Heiner
^ permalink raw reply
* Re: [PATCH net-next v2 03/10] net: sched: refactor block offloads counter usage
From: Vlad Buslov @ 2019-08-24 14:44 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Vlad Buslov, netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
pablo@netfilter.org
In-Reply-To: <20190823172648.7777e2b6@cakuba.netronome.com>
On Sat 24 Aug 2019 at 03:26, Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Fri, 23 Aug 2019 21:50:49 +0300, Vlad Buslov wrote:
>> @@ -1201,14 +1199,11 @@ static int u32_reoffload_knode(struct tcf_proto *tp, struct tc_u_knode *n,
>> cls_u32.knode.link_handle = ht->handle;
>> }
>>
>> - err = cb(TC_SETUP_CLSU32, &cls_u32, cb_priv);
>> - if (err) {
>> - if (add && tc_skip_sw(n->flags))
>> - return err;
>> - return 0;
>> - }
>> -
>> - tc_cls_offload_cnt_update(block, &n->in_hw_count, &n->flags, add);
>> + err = tc_setup_cb_reoffload(block, tp, add, cb, TC_SETUP_CLSU32,
>> + &cls_u32, cb_priv, &n->flags,
>> + &n->in_hw_count);
>> + if (err && add && tc_skip_sw(n->flags))
>> + return err;
>
> Could this be further simplified by adding something along the lines of:
>
> if (!add || !tc_skip_sw(*flags))
> err = 0;
>
> to tc_setup_cb_reoffload() ?
Indeed, all the users of tc_setup_cb_reoffload() have same error check
that can be moved into the function. I will refactor it in V3.
>
>>
>> return 0;
>> }
^ permalink raw reply
* Re: [PATCH] igb/igc: Don't warn on fatal read failures when the device is removed
From: Neftin, Sasha @ 2019-08-24 14:40 UTC (permalink / raw)
To: Lyude Paul, intel-wired-lan, netdev
Cc: Feng Tang, Jeff Kirsher, David S. Miller, linux-kernel
In-Reply-To: <20190822183318.27634-1-lyude@redhat.com>
On 8/22/2019 21:33, Lyude Paul wrote:
> Fatal read errors are worth warning about, unless of course the device
> was just unplugged from the machine - something that's a rather normal
> occurence when the igb/igc adapter is located on a Thunderbolt dock. So,
> let's only WARN() if there's a fatal read error while the device is
> still present.
>
> This fixes the following WARN splat that's been appearing whenever I
> unplug my Caldigit TS3 Thunderbolt dock from my laptop:
>
> igb 0000:09:00.0 enp9s0: PCIe link lost
> ------------[ cut here ]------------
> igb: Failed to read reg 0x18!
> WARNING: CPU: 7 PID: 516 at
> drivers/net/ethernet/intel/igb/igb_main.c:756 igb_rd32+0x57/0x6a [igb]
> Modules linked in: igb dca thunderbolt fuse vfat fat elan_i2c mei_wdt
> mei_hdcp i915 wmi_bmof intel_wmi_thunderbolt iTCO_wdt
> iTCO_vendor_support x86_pkg_temp_thermal intel_powerclamp joydev
> coretemp crct10dif_pclmul crc32_pclmul i2c_algo_bit ghash_clmulni_intel
> intel_cstate drm_kms_helper intel_uncore syscopyarea sysfillrect
> sysimgblt fb_sys_fops intel_rapl_perf intel_xhci_usb_role_switch mei_me
> drm roles idma64 i2c_i801 ucsi_acpi typec_ucsi mei intel_lpss_pci
> processor_thermal_device typec intel_pch_thermal intel_soc_dts_iosf
> intel_lpss int3403_thermal thinkpad_acpi wmi int340x_thermal_zone
> ledtrig_audio int3400_thermal acpi_thermal_rel acpi_pad video
> pcc_cpufreq ip_tables serio_raw nvme nvme_core crc32c_intel uas
> usb_storage e1000e i2c_dev
> CPU: 7 PID: 516 Comm: kworker/u16:3 Not tainted 5.2.0-rc1Lyude-Test+ #14
> Hardware name: LENOVO 20L8S2N800/20L8S2N800, BIOS N22ET35W (1.12 ) 04/09/2018
> Workqueue: kacpi_hotplug acpi_hotplug_work_fn
> RIP: 0010:igb_rd32+0x57/0x6a [igb]
> Code: 87 b8 fc ff ff 48 c7 47 08 00 00 00 00 48 c7 c6 33 42 9b c0 4c 89
> c7 e8 47 45 cd dc 89 ee 48 c7 c7 43 42 9b c0 e8 c1 94 71 dc <0f> 0b eb
> 08 8b 00 ff c0 75 b0 eb c8 44 89 e0 5d 41 5c c3 0f 1f 44
> RSP: 0018:ffffba5801cf7c48 EFLAGS: 00010286
> RAX: 0000000000000000 RBX: ffff9e7956608840 RCX: 0000000000000007
> RDX: 0000000000000000 RSI: ffffba5801cf7b24 RDI: ffff9e795e3d6a00
> RBP: 0000000000000018 R08: 000000009dec4a01 R09: ffffffff9e61018f
> R10: 0000000000000000 R11: ffffba5801cf7ae5 R12: 00000000ffffffff
> R13: ffff9e7956608840 R14: ffff9e795a6f10b0 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff9e795e3c0000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000564317bc4088 CR3: 000000010e00a006 CR4: 00000000003606e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> igb_release_hw_control+0x1a/0x30 [igb]
> igb_remove+0xc5/0x14b [igb]
> pci_device_remove+0x3b/0x93
> device_release_driver_internal+0xd7/0x17e
> pci_stop_bus_device+0x36/0x75
> pci_stop_bus_device+0x66/0x75
> pci_stop_bus_device+0x66/0x75
> pci_stop_and_remove_bus_device+0xf/0x19
> trim_stale_devices+0xc5/0x13a
> ? __pm_runtime_resume+0x6e/0x7b
> trim_stale_devices+0x103/0x13a
> ? __pm_runtime_resume+0x6e/0x7b
> trim_stale_devices+0x103/0x13a
> acpiphp_check_bridge+0xd8/0xf5
> acpiphp_hotplug_notify+0xf7/0x14b
> ? acpiphp_check_bridge+0xf5/0xf5
> acpi_device_hotplug+0x357/0x3b5
> acpi_hotplug_work_fn+0x1a/0x23
> process_one_work+0x1a7/0x296
> worker_thread+0x1a8/0x24c
> ? process_scheduled_works+0x2c/0x2c
> kthread+0xe9/0xee
> ? kthread_destroy_worker+0x41/0x41
> ret_from_fork+0x35/0x40
> ---[ end trace 252bf10352c63d22 ]---
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Fixes: 47e16692b26b ("igb/igc: warn when fatal read failure happens")
> Cc: Feng Tang <feng.tang@intel.com>
> Cc: Sasha Neftin <sasha.neftin@intel.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 3 ++-
> drivers/net/ethernet/intel/igc/igc_main.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index e5b7e638df28..1a7f7cd28df9 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -753,7 +753,8 @@ u32 igb_rd32(struct e1000_hw *hw, u32 reg)
> struct net_device *netdev = igb->netdev;
> hw->hw_addr = NULL;
> netdev_err(netdev, "PCIe link lost\n");
> - WARN(1, "igb: Failed to read reg 0x%x!\n", reg);
> + WARN(pci_device_is_present(igb->pdev),
> + "igb: Failed to read reg 0x%x!\n", reg);
> }
>
> return value;
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 28072b9aa932..f873a4b35eaf 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -3934,7 +3934,8 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg)
> hw->hw_addr = NULL;
> netif_device_detach(netdev);
> netdev_err(netdev, "PCIe link lost, device now detached\n");
> - WARN(1, "igc: Failed to read reg 0x%x!\n", reg);
> + WARN(pci_device_is_present(igc->pdev),
> + "igc: Failed to read reg 0x%x!\n", reg);
> }
>
> return value;
>
Thanks, for igc
Acked-by: Sasha Neftin <sasha.neftin@intel.com>
^ permalink raw reply
* Re: [PATCH net-next v3 4/8] MIPS: dts: mscc: describe the PTP ready interrupt
From: Paul Burton @ 2019-08-24 14:18 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem@davemloft.net, richardcochran@gmail.com,
alexandre.belloni@bootlin.com, UNGLinuxDriver@microchip.com,
ralf@linux-mips.org, Paul Burton, jhogan@kernel.org,
Antoine Tenart, netdev@vger.kernel.org,
linux-mips@vger.kernel.org, thomas.petazzoni@bootlin.com,
allan.nielsen@microchip.com, linux-mips@vger.kernel.org
In-Reply-To: <20190724081715.29159-5-antoine.tenart@bootlin.com>
Hello,
Antoine Tenart wrote:
> This patch adds a description of the PTP ready interrupt, which can be
> triggered when a PTP timestamp is available on an hardware FIFO.
Applied to mips-next.
> commit b4742e6682d5
> https://git.kernel.org/mips/c/b4742e6682d5
>
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
Thanks,
Paul
[ This message was auto-generated; if you believe anything is incorrect
then please email paul.burton@mips.com to report it. ]
^ permalink raw reply
* Re: [PATCH net-next v3 2/8] MIPS: dts: mscc: describe the PTP register range
From: Paul Burton @ 2019-08-24 14:18 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem@davemloft.net, richardcochran@gmail.com,
alexandre.belloni@bootlin.com, UNGLinuxDriver@microchip.com,
ralf@linux-mips.org, Paul Burton, jhogan@kernel.org,
Antoine Tenart, netdev@vger.kernel.org,
linux-mips@vger.kernel.org, thomas.petazzoni@bootlin.com,
allan.nielsen@microchip.com, linux-mips@vger.kernel.org
In-Reply-To: <20190724081715.29159-3-antoine.tenart@bootlin.com>
Hello,
Antoine Tenart wrote:
> This patch adds one register range within the mscc,vsc7514-switch node,
> to describe the PTP registers.
Applied to mips-next.
> commit 048dc3abe827
> https://git.kernel.org/mips/c/048dc3abe827
>
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
Thanks,
Paul
[ This message was auto-generated; if you believe anything is incorrect
then please email paul.burton@mips.com to report it. ]
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] net: ethernet: mediatek: Re-add support SGMII
From: Russell King - ARM Linux admin @ 2019-08-24 13:32 UTC (permalink / raw)
To: René van Dorst
Cc: John Crispin, Sean Wang, Nelson Chang, David S . Miller,
Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek,
linux-mips, Frank Wunderlich, Stefan Roese
In-Reply-To: <20190824131117.Horde.vSCF_CQ5jCMHcSTWkh7Woxm@www.vdorst.com>
Hi René,
On Sat, Aug 24, 2019 at 01:11:17PM +0000, René van Dorst wrote:
> Hi Russell,
>
> Mediatek calls it Turbo RGMII. It is a overclock version of RGMII mode.
> It is used between first GMAC and port 6 of the mt7530 switch. Can be used
> with
> an internal and an external mt7530 switch.
>
> TRGMII speed are:
> * mt7621: 1200Mbit
> * mt7623: 2000Mbit and 2600Mbit.
>
> I think that TRGMII is only used in a fixed-link situation in combination
> with a
> mt7530 switch and running and maximum speed/full duplex. So reporting
> 1000baseT_Full seems to me the right option.
I think we can ignore this one for the purposes of merging this patch
set, since this seems to be specific to this setup. Neither 1000BaseT
nor 1000BaseX fit very well, but we have to choose something.
> PHY_INTERFACE_MODE_GMII:
> 10baseT_Half
> 10baseT_Full
> 100baseT_Half
> 100baseT_Full
> 1000baseT_Half
> 1000baseT_Full
I think GMII can be connected to a PHY that can convert to 1000BaseX, so
should probably include that here too.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* RE: [RFC 4/4] net: cdc_ncm: Add ACPI MAC address pass through functionality
From: Mario.Limonciello @ 2019-08-24 13:26 UTC (permalink / raw)
To: bjorn, Charles.Hyde
Cc: linux-usb, linux-acpi, gregkh, oliver, netdev, nic_swsd
In-Reply-To: <877e722691.fsf@miraculix.mork.no>
> -----Original Message-----
> From: Bjørn Mork <bjorn@mork.no>
> Sent: Saturday, August 24, 2019 5:44 AM
> To: Hyde, Charles - Dell Team
> Cc: linux-usb@vger.kernel.org; linux-acpi@vger.kernel.org;
> gregkh@linuxfoundation.org; Limonciello, Mario; oliver@neukum.org;
> netdev@vger.kernel.org; nic_swsd@realtek.com
> Subject: Re: [RFC 4/4] net: cdc_ncm: Add ACPI MAC address pass through
> functionality
>
>
> [EXTERNAL EMAIL]
>
> <Charles.Hyde@dellteam.com> writes:
>
> > @@ -930,11 +931,18 @@ int cdc_ncm_bind_common(struct usbnet *dev,
> struct usb_interface *intf, u8 data_
> > usb_set_intfdata(ctx->control, dev);
> >
> > if (ctx->ether_desc) {
> > + struct sockaddr sa;
> > +
> > temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc-
> >iMACAddress);
> > if (temp) {
> > dev_dbg(&intf->dev, "failed to get mac address\n");
> > goto error2;
> > }
> > + if (get_acpi_mac_passthru(&intf->dev, &sa) == 0) {
> > + memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
> > + if (usbnet_set_ethernet_addr(dev) < 0)
> > + usbnet_get_ethernet_addr(dev, ctx-
> >ether_desc->iMACAddress);
> > + }
> > dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net-
> >dev_addr);
> > }
>
> So you want to run a Dell specific ACPI method every time anyone plugs some
> NCM class device into a host supporing ACPI? That's going to annoy the hell out
> of 99.9997% of the x86, ia64 and arm64 users of this driver.
>
> Call ACPI once when the driver loads, and only if running on an actual Dell
> system where this method is supported. There must be some ACPI device ID you
> can match on to know if this method is supported or not?
>
>
> Bjørn
I have to agree - this is missing an identifying factor of the D6000. It shouldn't be
running on "just any" cdc_ncm device.
The code that is in get_acpi_mac_passthrough checks for a properly built ACPI method
though.
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] net: ethernet: mediatek: Re-add support SGMII
From: René van Dorst @ 2019-08-24 13:11 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: John Crispin, Sean Wang, Nelson Chang, David S . Miller,
Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek,
linux-mips, Frank Wunderlich, Stefan Roese
In-Reply-To: <20190824092156.GD13294@shell.armlinux.org.uk>
Hi Russell,
Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>:
> On Fri, Aug 23, 2019 at 03:45:15PM +0200, René van Dorst wrote:
>> + switch (state->interface) {
>> + case PHY_INTERFACE_MODE_SGMII:
>> + phylink_set(mask, 10baseT_Half);
>> + phylink_set(mask, 10baseT_Full);
>> + phylink_set(mask, 100baseT_Half);
>> + phylink_set(mask, 100baseT_Full);
>
> You also want 1000baseX_Full here - the connected PHY could have a fiber
> interface on it.
Ok, I shall add that mode too.
>
>> + /* fall through */
>> + case PHY_INTERFACE_MODE_TRGMII:
>> phylink_set(mask, 1000baseT_Full);
>
> I don't know enough about this interface type to comment whether it
> should support 1000baseX_Full - if this is connected to a PHY that may
> support fiber, then it ought to set it.
Mediatek calls it Turbo RGMII. It is a overclock version of RGMII mode.
It is used between first GMAC and port 6 of the mt7530 switch. Can be
used with
an internal and an external mt7530 switch.
TRGMII speed are:
* mt7621: 1200Mbit
* mt7623: 2000Mbit and 2600Mbit.
I think that TRGMII is only used in a fixed-link situation in
combination with a
mt7530 switch and running and maximum speed/full duplex. So reporting
1000baseT_Full seems to me the right option.
>
>> + break;
>> + case PHY_INTERFACE_MODE_2500BASEX:
>> + phylink_set(mask, 2500baseX_Full);
>> + /* fall through */
>> + case PHY_INTERFACE_MODE_1000BASEX:
>> + phylink_set(mask, 1000baseX_Full);
>
> Both should be set. The reasoning here is that if you have a
> Fiberchannel 4Gbaud SFP plugged in and connected directly to the
> MAC, it can operate at either 2500Base-X or 1000Base-X. If we
> decide to operate at 2500Base-X, then PHY_INTERFACE_MODE_2500BASEX
> will be chosen. Otherwise, PHY_INTERFACE_MODE_1000BASEX will be
> used.
>
> The user can use ethtool to control which interface mode is used
> by adjusting the advertise mask and/or placing the interface in
> manual mode and setting the speed directly. This will change
> the PHY_INTERFACE_MODE_xxxxBASEX (via phylink_helper_basex_speed())
> between the two settings.
>
> If we lose 2500baseX_Full when 1000Base-X is selected, the user
> will not be able to go back to 2500Base-X mode.
>
> Yes, it's a little confusing and has slightly different rules
> from the other modes - partly due to phylink_helper_basex_speed().
> These are the only interface modes that we dynamically switch
> between depending on the settings that the user configures via
> ethtool.
Thanks for this extra information.
I made a list for each mode what that mode should report back when chosen.
PHY_INTERFACE_MODE_SGMII:
10baseT_Half
10baseT_Full
100baseT_Half
100baseT_Full
1000baseT_Full
1000baseX_Full
PHY_INTERFACE_MODE_1000BASEX:
PHY_INTERFACE_MODE_2500BASEX:
1000baseX_Full
2500baseX_Full
PHY_INTERFACE_MODE_TRGMII:
1000baseT_Full
PHY_INTERFACE_MODE_RGMII:
PHY_INTERFACE_MODE_RGMII_ID:
PHY_INTERFACE_MODE_RGMII_RXID:
PHY_INTERFACE_MODE_RGMII_TXID:
10baseT_Half
10baseT_Full
100baseT_Half
100baseT_Full
1000baseT_Half
1000baseT_Full
1000baseX_Full
PHY_INTERFACE_MODE_GMII:
10baseT_Half
10baseT_Full
100baseT_Half
100baseT_Full
1000baseT_Half
1000baseT_Full
PHY_INTERFACE_MODE_MII:
PHY_INTERFACE_MODE_RMII:
PHY_INTERFACE_MODE_REVMII:
10baseT_Half
10baseT_Full
100baseT_Half
100baseT_Full
case PHY_INTERFACE_MODE_NA:
10baseT_Half
10baseT_Full
100baseT_Half
100baseT_Full
1000baseT_Half
1000baseT_Full
1000baseX_Full
2500baseX_Full
I think this is the full list.
Or do I miss something?
Greats,
René
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH 14/16] include/linux: prefer __section from compiler_attributes.h
From: Miguel Ojeda @ 2019-08-24 12:51 UTC (permalink / raw)
To: Will Deacon
Cc: Nick Desaulniers, Andrew Morton, Sedat Dilek, Josh Poimboeuf,
Yonghong Song, clang-built-linux, Luc Van Oostenryck,
Lai Jiangshan, Paul E. McKenney, Josh Triplett, Steven Rostedt,
Mathieu Desnoyers, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Thomas Gleixner, Ingo Molnar,
Peter Zijlstra (Intel), Nicholas Piggin, Jiri Kosina,
Ard Biesheuvel, Michael Ellerman, Masahiro Yamada,
Hans Liljestrand, Elena Reshetova, David Windsor, Marc Zyngier,
Ming Lei, Dou Liyang, Julien Thierry, Mauro Carvalho Chehab,
Jens Axboe, linux-kernel, linux-sparse, rcu, Network Development,
bpf
In-Reply-To: <20190813083257.nnsxf5khnqagl46s@willie-the-truck>
On Tue, Aug 13, 2019 at 10:33 AM Will Deacon <will@kernel.org> wrote:
>
> -ENOCOMMITMESSAGE
>
> Otherwise, patch looks good to me.
Do you want Ack, Review or nothing?
Cheers,
Miguel
^ permalink raw reply
* Re: [RFC PATCH 1/2] gianfar: convert to phylink
From: Vladimir Oltean @ 2019-08-24 12:49 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Arseny Solokha, Claudiu Manoil, Ioana Ciornei, Andrew Lunn,
netdev, Florian Fainelli
In-Reply-To: <20190730102329.GZ1330@shell.armlinux.org.uk>
Hi Russell,
On Tue, 30 Jul 2019 at 13:23, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Tue, Jul 30, 2019 at 02:39:58AM +0300, Vladimir Oltean wrote:
> > To be honest I don't have a complete answer to that question. The
> > literature recommends writing 0x01a0 to the MII_ADVERTISE (0x4)
> > register of the MAC PCS for 1000Base-X, and 0x4001 for SGMII.
>
> That looks entirely sane for both modes.
>
> 0x01a0 for 802.3z (1000BASE-X) is defined in 802.3 37.2.5.1.3 as:
> bit 5 - full duplex = 1
> bit 6 - half duplex = 0
> bit 7 - pause = 1
> bit 8 - asym_pause = 1
>
> The description of the bits match the config word that is sent in the
> link with the exception of bit 14, which is the acknowledgement bit.
> Normally, in 802.3z, bit 14 will not be set in the transmitted config
> word until we have received the config word from the other end of the
> link.
>
> For SGMII, 0x4001 is the acknowledgement code word, which is defined
> in the SGMII spec as "tx_config_Reg[15:0] sent from the MAC to the
> PHY" which requires:
> bit 0 - must be 1
> bit 1 .. 13, 15 - must be 0, reserved for future use
> bit 14 - must be 1 (auto-negotiation acknowledgement in 802.3z)
>
> > The FMan driver which uses the TSEC MAC does exactly that:
> > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/ethernet/freescale/fman/fman_dtsec.c#n58
> > However I can't seem to be able to trace down the definition of bit 14
> > from 0x4001 - it's reserved in all the manuals I see. I have a hunch
> > that it selects the format of the Config_Reg base page between
> > 1000Base-X and SGMII.
>
> It could be that is what bit 14 is being used for, or it could just be
> that they've taken the values from the appropriate specs, and the
> hardware behaves the same way irrespective of whether it is in SGMII
> or 1000BASE-X mode.
>
Mystery solved - indeed it appears that there is no hardware logic for
propagating the AN results from the PCS to the MAC. Hence there is no
hardware distinction between 1000Base-X and SGMII. That must be
handled in PHYLINK.
> > > +static int gfar_mac_link_state(struct phylink_config *config,
> > > + struct phylink_link_state *state)
> > > +{
> > > + if (state->interface == PHY_INTERFACE_MODE_SGMII ||
> > > + state->interface == PHY_INTERFACE_MODE_1000BASEX) {
> >
> > What if you reduce the indentation level by 1 here, by just exiting if
> > the interface mode is not SGMII?
>
> It's only called for in-band negotiation modes anyway, so the above
> protection probably doesn't gain much.
>
Or that.
> > > + struct gfar_private *priv =
> > > + netdev_priv(to_net_dev(config->dev));
> > > + u16 tbi_cr;
> > > +
> > > + if (!priv->tbi_phy)
> > > + return -ENODEV;
> > > +
> > > + tbi_cr = phy_read(priv->tbi_phy, MII_TBI_CR);
> > > +
> > > + state->duplex = !!(tbi_cr & TBI_CR_FULL_DUPLEX);
> >
> > Woah there. Aren't you supposed to first ensure state->an_complete is
> > ok, based on TBI_MII_Register_Set_SR[AN_Done]? There's also a
> > Link_Status bit in that register that you could retrieve.
>
> Indeed.
>
> > > + if ((tbi_cr & TBI_CR_SPEED_1000_MASK) == TBI_CR_SPEED_1000_MASK)
> > > + state->speed = SPEED_1000;
> > > + }
> >
> > See the Speed_Bit table from TBI_MII_Register_Set_ANLPBPA_SGMII for
> > the link partner (aka SGMII PHY) advertisement. You have to do a
> > logical-and between that and your own. Also please make sure you
> > really are in SGMII AN and not 1000 Base-X when interpreting registers
> > 4 & 5 as one way or another.
>
> From what you've said above, yes, this needs to do exactly that.
>
> > > -static noinline void gfar_update_link_state(struct gfar_private *priv)
> > > +static void gfar_mac_config(struct phylink_config *config, unsigned int mode,
> > > + const struct phylink_link_state *state)
> > > {
> > > + struct gfar_private *priv = netdev_priv(to_net_dev(config->dev));
> > > struct gfar __iomem *regs = priv->gfargrp[0].regs;
> > > - struct net_device *ndev = priv->ndev;
> > > - struct phy_device *phydev = ndev->phydev;
> > > - struct gfar_priv_rx_q *rx_queue = NULL;
> > > - int i;
> > > + u32 maccfg1, new_maccfg1;
> > > + u32 maccfg2, new_maccfg2;
> > > + u32 ecntrl, new_ecntrl;
> > > + u32 tx_flow, new_tx_flow;
> >
> > Don't introduce new_ variables. Is there any issue if you
> > unconditionally write to the MAC registers?
>
> We do this in every driver, as mac_config() can be called with only a
> small number of changes in the settings, and it is important not to
> upset the MAC for minor updates.
>
> An example of this is when we are in SGMII mode with an attached PHY.
> SGMII will communicate the speed and duplex, but not the results of
> the pause negotiation. We read that from the attached PHY and report
> it back by calling mac_config() - but at that point, we don't want to
> cause the established link to bounce.
>
> So, mac_config() should be implemented to avoid upsetting an already
> established link where possible (unless the configuration items that
> affect the link have changed.)
>
Ok.
> > > +static void gfar_mac_an_restart(struct phylink_config *config)
> > > +{
> > > + /* Not supported */
> > > +}
> >
> > What about running gfar_configure_serdes again?
>
> The intention here is to cause the 802.3z link to renegotiate...
>
Yes, gfar_configure_serdes does this:
phy_write(tbiphy, MII_BMCR,
BMCR_ANENABLE | BMCR_ANRESTART | BMCR_FULLDPLX |
BMCR_SPEED1000);
> >
> > > +
> > > +static void gfar_mac_link_down(struct phylink_config *config, unsigned int mode,
> > > + phy_interface_t interface)
> > > +{
> > > + /* Not supported */
> > > +}
> > > +
> >
> > What about disabling RX_EN and TX_EN from MACCFG1?
> >
> > > +static void gfar_mac_link_up(struct phylink_config *config, unsigned int mode,
> > > + phy_interface_t interface, struct phy_device *phy)
> > > +{
> > > + /* Not supported */
> > > }
> > >
> >
> > What about enabling RX_EN and TX_EN from MACCFG1?
>
> Note that both of these functions must still allow the link to be
> established if we are using in-band negotiation - but they are
> expected to start/stop the transmission of packets.
>
I don't think AN pages count as Ethernet frames, and the RX_EN and
TX_EN bits are MAC settings anyway - it is the PCS below it that would
process them.
> > > @@ -149,8 +148,13 @@ extern const char gfar_driver_version[];
> > > #define GFAR_SUPPORTED_GBIT SUPPORTED_1000baseT_Full
> > >
> > > /* TBI register addresses */
> > > +#define MII_TBI_CR 0x00
> > > #define MII_TBICON 0x11
> > >
> > > +/* TBI_CR register bit fields */
> > > +#define TBI_CR_FULL_DUPLEX 0x0100
> > > +#define TBI_CR_SPEED_1000_MASK 0x0040
> > > +
> >
> > I think BIT() definitions are preferred, even if that means you have
> > to convert existing code first.
>
> If MII_TBI_CR is the BMCR of the PCS, how about using the definitions
> that we already have in the kernel:
>
> BMCR_SPEED1000 is TBI_CR_SPEED_1000_MASK
> BMCR_FULLDPLX is TBI_CR_FULL_DUPLEX
>
> ?
Or that, yes.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
Thanks,
-Vladimir
^ permalink raw reply
* Re: [PATCH 12/16] arm64: prefer __section from compiler_attributes.h
From: Miguel Ojeda @ 2019-08-24 12:48 UTC (permalink / raw)
To: Will Deacon
Cc: Nick Desaulniers, Andrew Morton, Sedat Dilek, Josh Poimboeuf,
Yonghong Song, clang-built-linux, Catalin Marinas,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Andrey Konovalov, Greg Kroah-Hartman, Enrico Weigelt,
Suzuki K Poulose, Thomas Gleixner, Masayoshi Mizuma,
Shaokun Zhang, Alexios Zavras, Allison Randal, Linux ARM,
linux-kernel, Network Development, bpf
In-Reply-To: <20190824112542.7guulvdenm35ihs7@willie-the-truck>
On Sat, Aug 24, 2019 at 1:25 PM Will Deacon <will@kernel.org> wrote:
>
> Which bit are you pinging about? This patch (12/16) has been in -next for a
> while and is queued in the arm64 tree for 5.4. The Oops/boot issue is
> addressed in patch 14 which probably needs to be sent as a separate patch
> (with a commit message) if it's targetting 5.3 and, I assume, routed via
> somebody like akpm.
I was pinging about the bit I was quoting, i.e. whether the Oops in
the cover letter was #14 indeed. Also, since Nick said he wanted to
get this ASAP through compiler-attributes, I assumed he wanted it to
be in 5.3, but I have not seen the independent patch.
Since he seems busy, I will write a better commit message myself and
send it to Linus next week.
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH spi for-5.4 2/5] spi: Add a PTP system timestamp to the transfer structure
From: Vladimir Oltean @ 2019-08-24 12:38 UTC (permalink / raw)
To: Mark Brown
Cc: Hubert Feurstein, Miroslav Lichvar, Richard Cochran, Andrew Lunn,
Florian Fainelli, linux-spi, netdev
In-Reply-To: <20190822171137.GB23391@sirena.co.uk>
On Thu, 22 Aug 2019 at 21:19, Mark Brown <broonie@kernel.org> wrote:
>
> On Sun, Aug 18, 2019 at 09:25:57PM +0300, Vladimir Oltean wrote:
>
> > @@ -1391,6 +1402,13 @@ static void __spi_pump_messages(struct spi_controller *ctlr, bool in_kthread)
> > goto out;
> > }
> >
> > + if (!ctlr->ptp_sts_supported) {
> > + list_for_each_entry(xfer, &mesg->transfers, transfer_list) {
> > + xfer->ptp_sts_word_pre = 0;
> > + ptp_read_system_prets(xfer->ptp_sts);
> > + }
> > + }
> > +
>
> We can do better than this for controllers which use transfer_one().
>
You mean I should guard this "if", and the one below, with "&&
!ctlr->transfer_one"?
> > +const void *spi_xfer_ptp_sts_word(struct spi_transfer *xfer, bool pre)
> > +{
>
> xfer can be const here too.
>
> > + * @ptp_sts_supported: If the driver sets this to true, it must provide a
> > + * time snapshot in @spi_transfer->ptp_sts as close as possible to the
> > + * moment in time when @spi_transfer->ptp_sts_word_pre and
> > + * @spi_transfer->ptp_sts_word_post were transmitted.
> > + * If the driver does not set this, the SPI core takes the snapshot as
> > + * close to the driver hand-over as possible.
>
> A couple of issues here. The big one is that for PIO transfers
> this is going to either complicate the code or introduce overhead
> in individual drivers for an extremely niche use case. I guess
> most drivers won't implement it which makes this a bit moot but
> then this is a concern that pushes back against the idea of
> implementing the feature.
>
The concern is the overhead in terms of code, or runtime performance?
Arguably the applications that require deterministic latency are
actually going to push for overall less overhead at runtime, even if
that comes at a cost in terms of code size. The spi-fsl-dspi driver
does not perform worse by any metric after this rework.
> The other is that it's not 100% clear what you're looking to
> timestamp here - is it when the data goes on the wire, is it when
> the data goes on the FIFO (which could be relatively large)? I'm
> guessing you're looking for the physical transfer here, if that's
> the case should there be some effort to compensate for the delays
> in the controller?
The goal is to timestamp the moment when the SPI slave sees word N of
the data. Luckily the DSPI driver raises the TCF (Transfer Complete
Flag) once that word has been transmitted, which I used to my
advantage. The EOQ mode behaves similarly, but has a granularity of 4
words. The controller delays are hence implicitly included in the
software timestamp.
But the question is valid and I expect that such compensation might be
needed for some hardware, provided that it can be measured and
guaranteed. In fact Hubert did add such logic to the v3 of his MDIO
patch: https://lkml.org/lkml/2019/8/20/195 There were some objections
mainly related to the certainty of those offset corrections. I don't
want to "future-proof" the API now with features I have no use of, but
such compensation logic might come in the future.
Regards,
-Vladimir
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] net: ethernet: mediatek: Add basic PHYLINK support
From: René van Dorst @ 2019-08-24 12:33 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: John Crispin, Sean Wang, Nelson Chang, David S . Miller,
Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek,
linux-mips, Frank Wunderlich, Stefan Roese
In-Reply-To: <20190824091106.GC13294@shell.armlinux.org.uk>
Hi Russell,
Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>:
> On Fri, Aug 23, 2019 at 03:45:14PM +0200, René van Dorst wrote:
>> This convert the basics to PHYLINK API.
>> SGMII support is not in this patch.
>>
>> Signed-off-by: René van Dorst <opensource@vdorst.com>
>> --
>> v2->v3:
>> * Make link_down() similar as link_up() suggested by Russell King
>
> Yep, almost there, but...
>
>> +static void mtk_mac_link_down(struct phylink_config *config,
>> unsigned int mode,
>> + phy_interface_t interface)
>> +{
>> + struct mtk_mac *mac = container_of(config, struct mtk_mac,
>> + phylink_config);
>> + u32 mcr = mtk_r32(mac->hw, MTK_MAC_MCR(mac->id));
>>
>> + mcr &= (MAC_MCR_TX_EN | MAC_MCR_RX_EN);
>
> ... this clears all bits _except_ for the tx and rx enable (which will
> remain set) - you probably wanted a ~ before the (.
Yes that is what it should be.
I only want to clear the TX_EN en RX_EN bits.
Greats,
René
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* pull-request: ieee802154 for net 2019-08-24
From: Stefan Schmidt @ 2019-08-24 12:19 UTC (permalink / raw)
To: davem; +Cc: linux-wpan, alex.aring, netdev
Hello Dave.
An update from ieee802154 for your *net* tree.
Yue Haibing fixed two bugs discovered by KASAN in the hwsim driver for
ieee802154 and Colin Ian King cleaned up a redundant variable assignment.
If there are any problems let me know.
regards
Stefan Schmidt
The following changes since commit 6c0afef5fb0c27758f4d52b2210c61b6bd8b4470:
ipv6/flowlabel: wait rcu grace period before put_pid() (2019-04-29 23:30:13 -0400)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/sschmidt/wpan.git ieee802154-for-davem-2019-08-24
for you to fetch changes up to 074014abdf2bd2a00da3dd14a6ae04cafc1d62cc:
net: ieee802154: remove redundant assignment to rc (2019-08-14 01:10:41 +0200)
----------------------------------------------------------------
Colin Ian King (1):
net: ieee802154: remove redundant assignment to rc
YueHaibing (2):
ieee802154: hwsim: Fix error handle path in hwsim_init_module
ieee802154: hwsim: unregister hw while hwsim_subscribe_all_others fails
drivers/net/ieee802154/mac802154_hwsim.c | 8 +++++---
net/ieee802154/socket.c | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
^ permalink raw reply
* Re: [PATCH spi for-5.4 0/5] Deterministic SPI latency with NXP DSPI driver
From: Vladimir Oltean @ 2019-08-24 12:13 UTC (permalink / raw)
To: Richard Cochran
Cc: Mark Brown, Hubert Feurstein, Miroslav Lichvar, Andrew Lunn,
Florian Fainelli, linux-spi, netdev
In-Reply-To: <20190823052217.GD2502@localhost>
On Fri, 23 Aug 2019 at 08:22, Richard Cochran <richardcochran@gmail.com> wrote:
>
> On Thu, Aug 22, 2019 at 07:13:12PM +0300, Vladimir Oltean wrote:
> > You do think that I understand the problem? But I don't!
>
> ;^)
>
> > > And who generates Local_sync_resp?
> > >
> >
> > Local_sync_resp is the same as Local_sync_req except maybe with a
> > custom tag added by the switch. Irrelevant as long as the DSA master
> > can timestamp it.
>
> So this is point why it won't work. The time stamping logic in the
> switch only recognizes PTP frames.
>
> Thanks,
> Richard
So to summarize the pros and cons of a PHC-to-PHC loopback sync:
Pros:
- At least two orders of magnitude improvement in offset compared to a
software timestamping solution, even in the situation where the
software timestamp is fully optimized
- Does not depend on the availability of PPS hardware
- In the case where both MACs support this, the synchronization can
simply reuse the DSA link with no dedicated circuitry
Cons:
- DSA framework for retrieving timestamps would need to be reworked
- The solution would have to be implemented in the kernel
- A separate protocol from PTP would have to be devised
- Not all DSA masters support hardware timestamping. Of those that do,
not all may support timestamping generic frames
- Not all PTP-capable DSA switches support timestamping generic frames
- Not all DSA switches may be able to loop back traffic from their CPU
port. I think this is called "hairpinning".
- The solution only covers the sync of the top-most switch in the DSA
tree. The hairpinning described above would need to be selective as
well, not just possible.
So at this point, the solution is not generic enough for me to be
compelled to prototype it. Taking system clock timestamps in the SPI
driver is "good enough". We'll just need to work out a way with Mark
that this can be added to the SPI subsystem, given the valid
objections already expressed.
Thanks,
-Vladimir
^ permalink raw reply
* Re: [Bridge] [PATCH 0/3] Add NETIF_F_HW_BRIDGE feature
From: Stephen Hemminger @ 2019-08-24 12:05 UTC (permalink / raw)
To: Horatiu Vultur
Cc: roopa, nikolay, davem, UNGLinuxDriver, alexandre.belloni,
allan.nielsen, netdev, linux-kernel, bridge
In-Reply-To: <1566500850-6247-1-git-send-email-horatiu.vultur@microchip.com>
On Thu, 22 Aug 2019 21:07:27 +0200
Horatiu Vultur <horatiu.vultur@microchip.com> wrote:
> Current implementation of the SW bridge is setting the interfaces in
> promisc mode when they are added to bridge if learning of the frames is
> enabled.
> In case of Ocelot which has HW capabilities to switch frames, it is not
> needed to set the ports in promisc mode because the HW already capable of
> doing that. Therefore add NETIF_F_HW_BRIDGE feature to indicate that the
> HW has bridge capabilities. Therefore the SW bridge doesn't need to set
> the ports in promisc mode to do the switching.
> This optimization takes places only if all the interfaces that are part
> of the bridge have this flag and have the same network driver.
>
> If the bridge interfaces is added in promisc mode then also the ports part
> of the bridge are set in promisc mode.
>
> Horatiu Vultur (3):
> net: Add HW_BRIDGE offload feature
> net: mscc: Use NETIF_F_HW_BRIDGE
> net: mscc: Implement promisc mode.
>
> drivers/net/ethernet/mscc/ocelot.c | 26 ++++++++++++++++++++++++--
> include/linux/netdev_features.h | 3 +++
> net/bridge/br_if.c | 29 ++++++++++++++++++++++++++++-
> net/core/ethtool.c | 1 +
> 4 files changed, 56 insertions(+), 3 deletions(-)
>
IMHO there are already enough nerd knobs in bridge to support this.
If you hardware can't do real bridging, it is only doing MACVLAN so that
is what you should support instead.
^ permalink raw reply
* Fw: [Bug 204669] New: TLS: module crash with TLS record double free
From: Stephen Hemminger @ 2019-08-24 12:03 UTC (permalink / raw)
To: netdev
This may have already been fixed, but forwarding to check.
Begin forwarded message:
Date: Thu, 22 Aug 2019 12:34:10 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 204669] New: TLS: module crash with TLS record double free
https://bugzilla.kernel.org/show_bug.cgi?id=204669
Bug ID: 204669
Summary: TLS: module crash with TLS record double free
Product: Networking
Version: 2.5
Kernel Version: v5.3-rc4
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: blocking
Priority: P1
Component: IPV4
Assignee: stephen@networkplumber.org
Reporter: mallesh537@gmail.com
Regression: No
Created attachment 284565
--> https://bugzilla.kernel.org/attachment.cgi?id=284565&action=edit
tls crash log
TLS module is crashing While running SSL record encryption using
Klts_send_[file]
Precondition:
1) Installed 5.3-rc4.
2) Nitrox5 card pluggin.
Steps to produce the issue:
1) Install n5pf.ko.(drivers/crypto/cavium/nitrox)
2) Install tls.ko if not is installed by default(net/tls)
3) Taken uperf tool from git.
3.1) Modified uperf to use tls module by using setsocket.
3.2) Modified uperf tool to support sendfile with SSL.
Test:
1) Running uperf with 4threads.
2) Each Thread send the data using sendfile over SSL protocol.
After few seconds kernel is crashing because of record list corruption
[ 270.888952] ------------[ cut here ]------------
[ 270.890450] list_del corruption, ffff91cc3753a800->prev is LIST_POISON2
(dead000000000122)
[ 270.891194] WARNING: CPU: 1 PID: 7387 at lib/list_debug.c:50
__list_del_entry_valid+0x62/0x90
[ 270.892037] Modules linked in: n5pf(OE) netconsole tls(OE) bonding
intel_rapl_msr intel_rapl_common sb_edac x86_pkg_temp_thermal intel_powerclamp
coretemp kvm_intel kvm iTCO_wdt iTCO_vendor_support irqbypass crct10dif_pclmul
crc32_pclmul ghash_clmulni_intel aesni_intel crypto_simd mei_me cryptd
glue_helper ipmi_si sg mei lpc_ich pcspkr joydev ioatdma i2c_i801 ipmi_devintf
ipmi_msghandler wmi ip_tables xfs libcrc32c sd_mod mgag200 drm_vram_helper ttm
drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm isci libsas
ahci scsi_transport_sas libahci crc32c_intel serio_raw igb libata ptp pps_core
dca i2c_algo_bit dm_mirror dm_region_hash dm_log dm_mod [last unloaded:
nitrox_drv]
[ 270.896836] CPU: 1 PID: 7387 Comm: uperf Kdump: loaded Tainted: G
OE 5.3.0-rc4 #1
[ 270.897711] Hardware name: Supermicro SYS-1027R-N3RF/X9DRW, BIOS 3.0c
03/24/2014
[ 270.898597] RIP: 0010:__list_del_entry_valid+0x62/0x90
[ 270.899478] Code: 00 00 00 c3 48 89 fe 48 89 c2 48 c7 c7 e0 f9 ee 8d e8 b2
cf c8 ff 0f 0b 31 c0 c3 48 89 fe 48 c7 c7 18 fa ee 8d e8 9e cf c8 ff <0f> 0b 31
c0 c3 48 89 f2 48 89 fe 48 c7 c7 50 fa ee 8d e8 87 cf c8
[ 270.901321] RSP: 0018:ffffb6ea86eb7c20 EFLAGS: 00010282
[ 270.902240] RAX: 0000000000000000 RBX: ffff91cc3753c000 RCX:
0000000000000000
[ 270.903157] RDX: ffff91bc3f867080 RSI: ffff91bc3f857738 RDI:
ffff91bc3f857738
[ 270.904074] RBP: ffff91bc36020940 R08: 0000000000000560 R09:
0000000000000000
[ 270.904988] R10: 0000000000000000 R11: 0000000000000000 R12:
0000000000000000
[ 270.905902] R13: ffff91cc3753a800 R14: ffff91cc37cc6400 R15:
ffff91cc3753a800
[ 270.906809] FS: 00007f454a88d700(0000) GS:ffff91bc3f840000(0000)
knlGS:0000000000000000
[ 270.907715] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 270.908606] CR2: 00007f453c00292c CR3: 000000103554e003 CR4:
00000000001606e0
[ 270.909490] Call Trace:
[ 270.910373] tls_tx_records+0x138/0x1c0 [tls]
[ 270.911262] tls_sw_sendpage+0x3e0/0x420 [tls]
[ 270.912154] inet_sendpage+0x52/0x90
[ 270.913045] ? direct_splice_actor+0x40/0x40
[ 270.913941] kernel_sendpage+0x1a/0x30
[ 270.914831] sock_sendpage+0x20/0x30
[ 270.915714] pipe_to_sendpage+0x62/0x90
[ 270.916592] __splice_from_pipe+0x80/0x180
[ 270.917461] ? direct_splice_actor+0x40/0x40
[ 270.918334] splice_from_pipe+0x5d/0x90
[ 270.919208] direct_splice_actor+0x35/0x40
[ 270.920086] splice_direct_to_actor+0x103/0x230
[ 270.920966] ? generic_pipe_buf_nosteal+0x10/0x10
[ 270.921850] do_splice_direct+0x9a/0xd0
[ 270.922733] do_sendfile+0x1c9/0x3d0
[ 270.923612] __x64_sys_sendfile64+0x5c/0xc0
(gdb) list *(tls_tx_records+0x138)
0x2d18 is in tls_tx_records (./include/linux/list.h:131).
126 * Note: list_empty() on entry does not return true after this, the
entry is
127 * in an undefined state.
128 */
129 static inline void __list_del_entry(struct list_head *entry)
130 {
131 if (!__list_del_entry_valid(entry))
132 return;
133
134 __list_del(entry->prev, entry->next);
135 }
(gdb)
(gdb) list *(tls_sw_sendpage+0x3e0)
0x48e0 is in tls_sw_sendpage (/home/mjatharkonda/5_3_rc4/tls/tls_sw.c:1211).
1206
1207 if (num_async) {
1208 /* Transmit if any encryptions have completed */
1209 if (test_and_clear_bit(BIT_TX_SCHEDULED,
&ctx->tx_bitmask)) {
1210 cancel_delayed_work(&ctx->tx_work.work);
1211 tls_tx_records(sk, flags);
1212 }
1213 }
1214 sendpage_end:
1215 ret = sk_stream_error(sk, flags, ret);
(gdb)
Attached complete crash log
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Fw: [Bug 204681] New: Kernel BUG/Oops: tc qdisc delete with tc filter action xt -j CONNMARK
From: Stephen Hemminger @ 2019-08-24 11:57 UTC (permalink / raw)
To: netdev
Begin forwarded message:
Date: Fri, 23 Aug 2019 23:44:26 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 204681] New: Kernel BUG/Oops: tc qdisc delete with tc filter action xt -j CONNMARK
https://bugzilla.kernel.org/show_bug.cgi?id=204681
Bug ID: 204681
Summary: Kernel BUG/Oops: tc qdisc delete with tc filter
action xt -j CONNMARK
Product: Networking
Version: 2.5
Kernel Version: 5.2.6 (x86_64) / 4.19.62 (mips32)/ 4.14.133 (arm7l)
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: high
Priority: P1
Component: Other
Assignee: stephen@networkplumber.org
Reporter: itugrok@yahoo.com
Regression: No
Created attachment 284581
--> https://bugzilla.kernel.org/attachment.cgi?id=284581&action=edit
BUG/Oops: kernel 5.2.6/x86_64
Overview:
=========
Several uses of "tc filter .. action xt" work as expected and also allow final
qdisc/filter deletion: e.g. xt_DSCP and xt_CLASSIFY.
However, trying to delete a qdisc/filter using xt_CONNMARK results in a kernel
oops or hang/crash on all platforms and kernel versions tested.
Steps to Reproduce:
===================
# tc qdisc add dev lo clsact
# tc filter add dev lo egress protocol ip matchall action xt -j CONNMARK
--save-mark
# tc qdisc del dev lo clsact
<Kernel Oops>
Systems Tested:
===============
Ubuntu 18.04 LTS (mainline kernel 5.2.6/x86_64, iptables 1.6.1, iproute2 4.15)
(Kernel build: https://kernel.ubuntu.com/~kernel-ppa/mainline/v5.2.6/)
Ubuntu 18.04 LTS (distro kernel 4.15/x86_64, iptables 1.6.1, iproute2 4.15)
OpenWrt master, r10666-fc5d46dc62 (kernel 4.19.62, mips32_be, iptables 1.8.3,
iproute2 5.0.0)
OpenWrt 19.07, r10324-8bf8de95a2 (kernel 4.14.133, armv7l, iptables 1.8.3,
iproute2 5.0.0)
Kernel Logs:
============
The clearest call traces are from the Ubuntu systems, and are attached.
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Re: [PATCH 12/16] arm64: prefer __section from compiler_attributes.h
From: Will Deacon @ 2019-08-24 11:25 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Nick Desaulniers, Andrew Morton, Sedat Dilek, Josh Poimboeuf,
Yonghong Song, clang-built-linux, Catalin Marinas,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Andrey Konovalov, Greg Kroah-Hartman, Enrico Weigelt,
Suzuki K Poulose, Thomas Gleixner, Masayoshi Mizuma,
Shaokun Zhang, Alexios Zavras, Allison Randal, Linux ARM,
linux-kernel, Network Development, bpf
In-Reply-To: <CANiq72nfn4zxAO63GEEoUjumC6Jwi5_jdcD_5Xzt1vZRgh52fg@mail.gmail.com>
On Fri, Aug 23, 2019 at 09:35:08PM +0200, Miguel Ojeda wrote:
> On Thu, Aug 15, 2019 at 11:12 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > Btw, I guess that is the Oops you were mentioning in the cover letter?
>
> Pinging about this...
Which bit are you pinging about? This patch (12/16) has been in -next for a
while and is queued in the arm64 tree for 5.4. The Oops/boot issue is
addressed in patch 14 which probably needs to be sent as a separate patch
(with a commit message) if it's targetting 5.3 and, I assume, routed via
somebody like akpm.
Will
^ permalink raw reply
* Re: [RFC 4/4] net: cdc_ncm: Add ACPI MAC address pass through functionality
From: Bjørn Mork @ 2019-08-24 10:43 UTC (permalink / raw)
To: Charles.Hyde
Cc: linux-usb, linux-acpi, gregkh, Mario.Limonciello, oliver, netdev,
nic_swsd
In-Reply-To: <ec7435e0529243a99f6949ee9efbede5@AUSX13MPS303.AMER.DELL.COM>
<Charles.Hyde@dellteam.com> writes:
> @@ -930,11 +931,18 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
> usb_set_intfdata(ctx->control, dev);
>
> if (ctx->ether_desc) {
> + struct sockaddr sa;
> +
> temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
> if (temp) {
> dev_dbg(&intf->dev, "failed to get mac address\n");
> goto error2;
> }
> + if (get_acpi_mac_passthru(&intf->dev, &sa) == 0) {
> + memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
> + if (usbnet_set_ethernet_addr(dev) < 0)
> + usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
> + }
> dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
> }
So you want to run a Dell specific ACPI method every time anyone plugs
some NCM class device into a host supporing ACPI? That's going to annoy
the hell out of 99.9997% of the x86, ia64 and arm64 users of this
driver.
Call ACPI once when the driver loads, and only if running on an actual
Dell system where this method is supported. There must be some ACPI
device ID you can match on to know if this method is supported or not?
Bjørn
^ permalink raw reply
* Re: 5.3-rc3-ish VM crash: RIP: 0010:tcp_trim_head+0x20/0xe0
From: Sander Eikelenboom @ 2019-08-24 10:22 UTC (permalink / raw)
To: Eric Dumazet, netdev, linux-kernel
In-Reply-To: <59dd3497-6d08-1e0e-7a4f-b121b850a24f@gmail.com>
On 17/08/2019 18:35, Eric Dumazet wrote:
>
>
> On 8/17/19 10:24 AM, Sander Eikelenboom wrote:
>> On 12/08/2019 19:56, Eric Dumazet wrote:
>>>
>>>
>>> On 8/12/19 2:50 PM, Sander Eikelenboom wrote:
>>>> L.S.,
>>>>
>>>> While testing a somewhere-after-5.3-rc3 kernel (which included the latest net merge (33920f1ec5bf47c5c0a1d2113989bdd9dfb3fae9),
>>>> one of my Xen VM's (which gets quite some network load) crashed.
>>>> See below for the stacktrace.
>>>>
>>>> Unfortunately I haven't got a clear trigger, so bisection doesn't seem to be an option at the moment.
>>>> I haven't encountered this on 5.2, so it seems to be an regression against 5.2.
>>>>
>>>> Any ideas ?
>>>>
>>>> --
>>>> Sander
>>>>
>>>>
>>>> [16930.653595] general protection fault: 0000 [#1] SMP NOPTI
>>>> [16930.653624] CPU: 0 PID: 3275 Comm: rsync Not tainted 5.3.0-rc3-20190809-doflr+ #1
>>>> [16930.653657] RIP: 0010:tcp_trim_head+0x20/0xe0
>>>> [16930.653677] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 <8b> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8
>>>> [16930.653741] RSP: 0000:ffffc90000003ad8 EFLAGS: 00010286
>>>> [16930.653762] RAX: fffe888005bf62c0 RBX: ffff8880115fb800 RCX: 000000008010000b
>>>
>>> crash in " mov 0x20(%rax),%eax" and RAX=fffe888005bf62c0 (not a valid kernel address)
>>>
>>> Look like one bit corruption maybe.
>>>
>>> Nothing comes to mind really between 5.2 and 53 that could explain this.
>>>
>>>> [16930.653791] RDX: 00000000000005a0 RSI: ffff8880115fb800 RDI: ffff888016b00880
>>>> [16930.653819] RBP: ffff888016b00880 R08: 0000000000000001 R09: 0000000000000000
>>>> [16930.653848] R10: ffff88800ae00800 R11: 00000000bfe632e6 R12: 00000000000005a0
>>>> [16930.653875] R13: 0000000000000001 R14: 00000000bfe62d46 R15: 0000000000000004
>>>> [16930.653913] FS: 00007fe71fe2cb80(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
>>>> [16930.653943] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [16930.653965] CR2: 000055de0f3e7000 CR3: 0000000011f32000 CR4: 00000000000006f0
>>>> [16930.653993] Call Trace:
>>>> [16930.654005] <IRQ>
>>>> [16930.654018] tcp_ack+0xbb0/0x1230
>>>> [16930.654033] tcp_rcv_established+0x2e8/0x630
>>>> [16930.654053] tcp_v4_do_rcv+0x129/0x1d0
>>>> [16930.654070] tcp_v4_rcv+0xac9/0xcb0
>>>> [16930.654088] ip_protocol_deliver_rcu+0x27/0x1b0
>>>> [16930.654109] ip_local_deliver_finish+0x3f/0x50
>>>> [16930.654128] ip_local_deliver+0x4d/0xe0
>>>> [16930.654145] ? ip_protocol_deliver_rcu+0x1b0/0x1b0
>>>> [16930.654163] ip_rcv+0x4c/0xd0
>>>> [16930.654179] __netif_receive_skb_one_core+0x79/0x90
>>>> [16930.654200] netif_receive_skb_internal+0x2a/0xa0
>>>> [16930.654219] napi_gro_receive+0xe7/0x140
>>>> [16930.654237] xennet_poll+0x9be/0xae0
>>>> [16930.654254] net_rx_action+0x136/0x340
>>>> [16930.654271] __do_softirq+0xdd/0x2cf
>>>> [16930.654287] irq_exit+0x7a/0xa0
>>>> [16930.654304] xen_evtchn_do_upcall+0x27/0x40
>>>> [16930.654320] xen_hvm_callback_vector+0xf/0x20
>>>> [16930.654339] </IRQ>
>>>> [16930.654349] RIP: 0033:0x55de0d87db99
>>>> [16930.654364] Code: 00 00 48 89 7c 24 f8 45 39 fe 45 0f 42 fe 44 89 7c 24 f4 eb 09 0f 1f 40 00 83 e9 01 74 3e 89 f2 48 63 f8 4c 01 d2 44 38 1c 3a <75> 25 44 38 6c 3a ff 75 1e 41 0f b6 3c 24 40 38 3a 75 14 41 0f b6
>>>> [16930.654432] RSP: 002b:00007ffd5531eec8 EFLAGS: 00000a87 ORIG_RAX: ffffffffffffff0c
>>>> [16930.655004] RAX: 0000000000000002 RBX: 000055de0f3e8e50 RCX: 000000000000007f
>>>> [16930.655034] RDX: 000055de0f3dc2d2 RSI: 0000000000003492 RDI: 0000000000000002
>>>> [16930.655062] RBP: 0000000000007fff R08: 00000000000080ea R09: 00000000000001f0
>>>> [16930.655089] R10: 000055de0f3d8e40 R11: 0000000000000094 R12: 000055de0f3e0f2a
>>>> [16930.655116] R13: 0000000000000010 R14: 0000000000007f16 R15: 0000000000000080
>>>> [16930.655144] Modules linked in:
>>>> [16930.655200] ---[ end trace 533367c95501b645 ]---
>>>> [16930.655223] RIP: 0010:tcp_trim_head+0x20/0xe0
>>>> [16930.655243] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 <8b> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8
>>>> [16930.655312] RSP: 0000:ffffc90000003ad8 EFLAGS: 00010286
>>>> [16930.655331] RAX: fffe888005bf62c0 RBX: ffff8880115fb800 RCX: 000000008010000b
>>>> [16930.655360] RDX: 00000000000005a0 RSI: ffff8880115fb800 RDI: ffff888016b00880
>>>> [16930.655387] RBP: ffff888016b00880 R08: 0000000000000001 R09: 0000000000000000
>>>> [16930.655414] R10: ffff88800ae00800 R11: 00000000bfe632e6 R12: 00000000000005a0
>>>> [16930.655441] R13: 0000000000000001 R14: 00000000bfe62d46 R15: 0000000000000004
>>>> [16930.655475] FS: 00007fe71fe2cb80(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
>>>> [16930.655502] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [16930.655525] CR2: 000055de0f3e7000 CR3: 0000000011f32000 CR4: 00000000000006f0
>>>> [16930.655553] Kernel panic - not syncing: Fatal exception in interrupt
>>>> [16930.655789] Kernel Offset: disabled
>>>>
>>
>> Hi Eric,
>>
>> Got another VM crash, with a slightly different stacktrace this time around.
>> Still networking though.
>>
>> --
>> Sander
>>
>> [112522.697498] general protection fault: 0000 [#1] SMP NOPTI
>> [112522.697555] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.3.0-rc4-20190812-doflr+ #1
>> [112522.697592] RIP: 0010:skb_shift+0x63/0x430
>> [112522.697608] Code: bc 00 00 00 48 03 8f c0 00 00 00 f6 41 03 08 74 07 48 83 79 28 00 75 d0 8b 8e bc 00 00 00 48 03 8e c0 00 00 00 48 85 f6 74 0a <f6> 41 03 08 0f 85 09 03 00 00 49 89 fd 8b bf bc 00 00 00 41 89
>
>
>
> crash in "testb $0x8,0x3(%rcx)" with RCX==fffe8880117da6c0
>
> Same strange looking address on x86_64
>
> I have no idea.
>
>> [112522.697673] RSP: 0018:ffffc900000039b0 EFLAGS: 00010286
>> [112522.697693] RAX: 00000000000005a0 RBX: ffff8880117fb800 RCX: fffe8880117da6c0
>> [112522.697721] RDX: 00000000000005a0 RSI: ffff8880117fb800 RDI: ffff88800ae58000
>> [112522.697748] RBP: ffffc900000039e8 R08: 000000000004cfe0 R09: 00000000000005a0
>> [112522.697775] R10: 00000000000005a0 R11: ffff8880117fb800 R12: 0000000000000000
>> [112522.697803] R13: 00000000c95a98c2 R14: 0000000000000000 R15: ffff88800ae58000
>> [112522.697839] FS: 0000000000000000(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
>> [112522.697869] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [112522.697895] CR2: 00007f9210d8e078 CR3: 000000000b660000 CR4: 00000000000006f0
>> [112522.697925] Call Trace:
>> [112522.697938] <IRQ>
>> [112522.697951] tcp_sacktag_walk+0x2af/0x480
>> [112522.697967] tcp_sacktag_write_queue+0x34d/0x820
>> [112522.697986] ? ip_forward_options.cold.0+0x1c/0x1c
>> [112522.698007] tcp_ack+0xb8c/0x1230
>> [112522.698023] ? tcp_event_new_data_sent+0x4a/0x90
>> [112522.698043] tcp_rcv_established+0x14c/0x630
>> [112522.698064] tcp_v4_do_rcv+0x129/0x1d0
>> [112522.698081] tcp_v4_rcv+0xac9/0xcb0
>> [112522.698099] ip_protocol_deliver_rcu+0x27/0x1b0
>> [112522.698119] ip_local_deliver_finish+0x3f/0x50
>> [112522.698139] ip_local_deliver+0x4d/0xe0
>> [112522.698155] ? ip_protocol_deliver_rcu+0x1b0/0x1b0
>> [112522.698177] ip_rcv+0x4c/0xd0
>> [112522.698194] __netif_receive_skb_one_core+0x79/0x90
>> [112522.698215] netif_receive_skb_internal+0x2a/0xa0
>> [112522.698237] napi_gro_receive+0xe7/0x140
>> [112522.698255] xennet_poll+0x9be/0xae0
>> [112522.698271] net_rx_action+0x136/0x340
>> [112522.698288] __do_softirq+0xdd/0x2cf
>> [112522.698304] irq_exit+0x7a/0xa0
>> [112522.698321] xen_evtchn_do_upcall+0x27/0x40
>> [112522.698340] xen_hvm_callback_vector+0xf/0x20
>> [112522.698359] </IRQ>
>> [112522.698373] RIP: 0010:native_safe_halt+0xe/0x10
>> [112522.698392] Code: 48 8b 04 25 c0 6b 01 00 f0 80 48 02 20 48 8b 00 a8 08 75 c4 eb 80 90 90 90 90 90 90 e9 07 00 00 00 0f 00 2d 54 fb 41 00 fb f4 <c3> 90 e9 07 00 00 00 0f 00 2d 44 fb 41 00 f4 c3 90 90 41 55 41 54
>> [112522.699522] RSP: 0018:ffffffff82a03e90 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff0c
>> [112522.699552] RAX: 0001a54800000000 RBX: 0000000000000000 RCX: 0000000000000001
>> [112522.699580] RDX: 0000000002b9f9b6 RSI: 0000000000000087 RDI: 0000000000000000
>> [112522.699608] RBP: 0000000000000000 R08: 000000001eb5c3cb R09: ffffffff82a08460
>> [112522.699634] R10: 000000000002e46e R11: 0000000000000000 R12: 0000000000000000
>> [112522.699662] R13: 0000000000000000 R14: ffffffff8326e0a0 R15: 0000000000000000
>> [112522.699692] default_idle+0x17/0x140
>> [112522.699709] do_idle+0x1ee/0x210
>> [112522.699726] cpu_startup_entry+0x14/0x20
>> [112522.699743] start_kernel+0x4e9/0x50b
>> [112522.699760] secondary_startup_64+0xa4/0xb0
>> [112522.699780] Modules linked in:
>> [112522.699829] ---[ end trace 3b8db3603485e952 ]---
>> [112522.699850] RIP: 0010:skb_shift+0x63/0x430
>> [112522.699866] Code: bc 00 00 00 48 03 8f c0 00 00 00 f6 41 03 08 74 07 48 83 79 28 00 75 d0 8b 8e bc 00 00 00 48 03 8e c0 00 00 00 48 85 f6 74 0a <f6> 41 03 08 0f 85 09 03 00 00 49 89 fd 8b bf bc 00 00 00 41 89 d4
>> [112522.699938] RSP: 0018:ffffc900000039b0 EFLAGS: 00010286
>> [112522.699959] RAX: 00000000000005a0 RBX: ffff8880117fb800 RCX: fffe8880117da6c0
>> [112522.699986] RDX: 00000000000005a0 RSI: ffff8880117fb800 RDI: ffff88800ae58000
>> [112522.700013] RBP: ffffc900000039e8 R08: 000000000004cfe0 R09: 00000000000005a0
>> [112522.700041] R10: 00000000000005a0 R11: ffff8880117fb800 R12: 0000000000000000
>> [112522.700067] R13: 00000000c95a98c2 R14: 0000000000000000 R15: ffff88800ae58000
>> [112522.700111] FS: 0000000000000000(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
>> [112522.700140] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [112522.700165] CR2: 00007f9210d8e078 CR3: 000000000b660000 CR4: 00000000000006f0
>> [112522.700201] Kernel panic - not syncing: Fatal exception in interrupt
>> [112522.702992] Kernel Offset: disabled
>>
>>
And another one (on an RC5 kernel now), but I probably know the answer :( ...
Is there any extra debugging options I can turn on that could shed some more light on this ?
(it doesn't seem to be an incident and happens at about 3 to 4 days of uptime, so bisecting is not a realistic option).
--
Sander
[327011.399239] general protection fault: 0000 [#1] SMP NOPTI
[327011.399271] CPU: 0 PID: 19440 Comm: rsync Not tainted 5.3.0-rc5-20190820-doflr+ #1
[327011.399309] RIP: 0010:skb_shift+0x63/0x430
[327011.399326] Code: bc 00 00 00 48 03 8f c0 00 00 00 f6 41 03 08 74 07 48 83 79 28 00 75 d0 8b 8e bc 00 00 00 48 03 8e c0 00 00 00 48 85 f6 74 0a <f6> 41 03 08 0f 85 09 03 00 00 49 89 fd 8b bf bc 00 00 00 41 89 d4
[327011.399407] RSP: 0000:ffffc900000039b0 EFLAGS: 00010286
[327011.399427] RAX: 00000000000005a0 RBX: ffff8880115fb800 RCX: fffe8880115152c0
[327011.399456] RDX: 00000000000005a0 RSI: ffff8880115fb800 RDI: ffff88801b0f2200
[327011.399485] RBP: ffffc900000039e8 R08: 0000000000070260 R09: 00000000000005a0
[327011.399515] R10: 00000000000005a0 R11: ffff8880115fb800 R12: 0000000000000000
[327011.399544] R13: 000000000ca599b0 R14: 0000000000000000 R15: ffff88801b0f2200
[327011.399581] FS: 00007f20c1b8bb80(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
[327011.399610] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[327011.399633] CR2: 00007ffac4c245cc CR3: 0000000005544000 CR4: 00000000000006f0
[327011.399663] Call Trace:
[327011.399675] <IRQ>
[327011.399688] tcp_sacktag_walk+0x2af/0x480
[327011.399705] ? __alloc_skb+0x52/0x1d0
[327011.399721] tcp_sacktag_write_queue+0x34d/0x820
[327011.399743] ? xennet_tx_setup_grant+0xbd/0x140
[327011.399762] tcp_ack+0xb8c/0x1230
[327011.399778] ? gnttab_foreach_grant_in_range+0x83/0xf0
[327011.399799] ? xennet_tx_setup_grant+0x140/0x140
[327011.399819] ? xennet_make_txreqs+0x81/0xb0
[327011.399838] tcp_rcv_established+0x14c/0x630
[327011.399860] tcp_v4_do_rcv+0x129/0x1d0
[327011.399876] tcp_v4_rcv+0xac9/0xcb0
[327011.399892] ip_protocol_deliver_rcu+0x27/0x1b0
[327011.399912] ip_local_deliver_finish+0x3f/0x50
[327011.399932] ip_local_deliver+0x4d/0xe0
[327011.399957] ? ip_protocol_deliver_rcu+0x1b0/0x1b0
[327011.399977] ip_rcv+0x4c/0xd0
[327011.400312] __netif_receive_skb_one_core+0x79/0x90
[327011.400347] netif_receive_skb_internal+0x2a/0xa0
[327011.400367] napi_gro_receive+0xe7/0x140
[327011.400384] xennet_poll+0x9be/0xae0
[327011.400401] net_rx_action+0x136/0x340
[327011.400419] __do_softirq+0xdd/0x2cf
[327011.400437] irq_exit+0x7a/0xa0
[327011.400454] xen_evtchn_do_upcall+0x27/0x40
[327011.400474] xen_hvm_callback_vector+0xf/0x20
[327011.400501] </IRQ>
[327011.400514] RIP: 0033:0x5620499a31aa
[327011.400532] Code: 43 4c 0f b6 34 0e 8b 8b 80 00 00 00 d3 e7 89 f9 48 8b 7b 60 31 f1 23 4b 7c 48 8b 73 68 83 fd 07 89 4b 70 48 8d 0c 4e 0f b7 31 <66> 42 89 34 47 66 89 11 0f 84 88 01 00 00 8b 8b 90 00 00 00 8b 83
[327011.400606] RSP: 002b:00007ffe82874060 EFLAGS: 00000297 ORIG_RAX: ffffffffffffff0c
[327011.400638] RAX: 0000000000001f7c RBX: 000056204a861190 RCX: 000056204a8cd986
[327011.400671] RDX: 000000000000e084 RSI: 0000000000000000 RDI: 000056204a8bae50
[327011.400703] RBP: 0000000000000000 R08: 0000000000006084 R09: 0000000000003fdf
[327011.400735] R10: 0000000000000000 R11: 0000000000000001 R12: 00005620499b9500
[327011.400769] R13: 000000000000206a R14: 00000000ffffffff R15: 000056204a861190
[327011.400802] Modules linked in:
[327011.400884] ---[ end trace 660afb6bf8586996 ]---
[327011.400910] RIP: 0010:skb_shift+0x63/0x430
[327011.400929] Code: bc 00 00 00 48 03 8f c0 00 00 00 f6 41 03 08 74 07 48 83 79 28 00 75 d0 8b 8e bc 00 00 00 48 03 8e c0 00 00 00 48 85 f6 74 0a <f6> 41 03 08 0f 85 09 03 00 00 49 89 fd 8b bf bc 00 00 00 41 89 d4
[327011.401012] RSP: 0000:ffffc900000039b0 EFLAGS: 00010286
[327011.401042] RAX: 00000000000005a0 RBX: ffff8880115fb800 RCX: fffe8880115152c0
[327011.401081] RDX: 00000000000005a0 RSI: ffff8880115fb800 RDI: ffff88801b0f2200
[327011.401116] RBP: ffffc900000039e8 R08: 0000000000070260 R09: 00000000000005a0
[327011.401157] R10: 00000000000005a0 R11: ffff8880115fb800 R12: 0000000000000000
[327011.401198] R13: 000000000ca599b0 R14: 0000000000000000 R15: ffff88801b0f2200
[327011.401244] FS: 00007f20c1b8bb80(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
[327011.401293] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[327011.404822] CR2: 00007ffac4c245cc CR3: 0000000005544000 CR4: 00000000000006f0
[327011.404866] Kernel panic - not syncing: Fatal exception in interrupt
[327011.405103] Kernel Offset: disabled
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] net: ethernet: mediatek: Re-add support SGMII
From: Russell King - ARM Linux admin @ 2019-08-24 9:21 UTC (permalink / raw)
To: René van Dorst
Cc: John Crispin, Sean Wang, Nelson Chang, David S . Miller,
Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek,
linux-mips, Frank Wunderlich, Stefan Roese
In-Reply-To: <20190823134516.27559-3-opensource@vdorst.com>
On Fri, Aug 23, 2019 at 03:45:15PM +0200, René van Dorst wrote:
> + switch (state->interface) {
> + case PHY_INTERFACE_MODE_SGMII:
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 10baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 100baseT_Full);
You also want 1000baseX_Full here - the connected PHY could have a fiber
interface on it.
> + /* fall through */
> + case PHY_INTERFACE_MODE_TRGMII:
> phylink_set(mask, 1000baseT_Full);
I don't know enough about this interface type to comment whether it
should support 1000baseX_Full - if this is connected to a PHY that may
support fiber, then it ought to set it.
> + break;
> + case PHY_INTERFACE_MODE_2500BASEX:
> + phylink_set(mask, 2500baseX_Full);
> + /* fall through */
> + case PHY_INTERFACE_MODE_1000BASEX:
> + phylink_set(mask, 1000baseX_Full);
Both should be set. The reasoning here is that if you have a
Fiberchannel 4Gbaud SFP plugged in and connected directly to the
MAC, it can operate at either 2500Base-X or 1000Base-X. If we
decide to operate at 2500Base-X, then PHY_INTERFACE_MODE_2500BASEX
will be chosen. Otherwise, PHY_INTERFACE_MODE_1000BASEX will be
used.
The user can use ethtool to control which interface mode is used
by adjusting the advertise mask and/or placing the interface in
manual mode and setting the speed directly. This will change
the PHY_INTERFACE_MODE_xxxxBASEX (via phylink_helper_basex_speed())
between the two settings.
If we lose 2500baseX_Full when 1000Base-X is selected, the user
will not be able to go back to 2500Base-X mode.
Yes, it's a little confusing and has slightly different rules
from the other modes - partly due to phylink_helper_basex_speed().
These are the only interface modes that we dynamically switch
between depending on the settings that the user configures via
ethtool.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] net: ethernet: mediatek: Add basic PHYLINK support
From: Russell King - ARM Linux admin @ 2019-08-24 9:11 UTC (permalink / raw)
To: René van Dorst
Cc: John Crispin, Sean Wang, Nelson Chang, David S . Miller,
Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek,
linux-mips, Frank Wunderlich, Stefan Roese
In-Reply-To: <20190823134516.27559-2-opensource@vdorst.com>
On Fri, Aug 23, 2019 at 03:45:14PM +0200, René van Dorst wrote:
> This convert the basics to PHYLINK API.
> SGMII support is not in this patch.
>
> Signed-off-by: René van Dorst <opensource@vdorst.com>
> --
> v2->v3:
> * Make link_down() similar as link_up() suggested by Russell King
Yep, almost there, but...
> +static void mtk_mac_link_down(struct phylink_config *config, unsigned int mode,
> + phy_interface_t interface)
> +{
> + struct mtk_mac *mac = container_of(config, struct mtk_mac,
> + phylink_config);
> + u32 mcr = mtk_r32(mac->hw, MTK_MAC_MCR(mac->id));
>
> + mcr &= (MAC_MCR_TX_EN | MAC_MCR_RX_EN);
... this clears all bits _except_ for the tx and rx enable (which will
remain set) - you probably wanted a ~ before the (.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* [PATCH] r8152: Set memory to all 0xFFs on failed reg reads
From: Prashant Malani @ 2019-08-24 8:36 UTC (permalink / raw)
To: hayeswang, davem; +Cc: grundler, netdev, Prashant Malani
get_registers() blindly copies the memory written to by the
usb_control_msg() call even if the underlying urb failed.
This could lead to junk register values being read by the driver, since
some indirect callers of get_registers() ignore the return values. One
example is:
ocp_read_dword() ignores the return value of generic_ocp_read(), which
calls get_registers().
So, emulate PCI "Master Abort" behavior by setting the buffer to all
0xFFs when usb_control_msg() fails.
This patch is copied from the r8152 driver (v2.12.0) published by
Realtek (www.realtek.com).
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/net/usb/r8152.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0cc03a9ff545..eee0f5007ee3 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -799,8 +799,11 @@ int get_registers(struct r8152 *tp, u16 value, u16 index, u16 size, void *data)
ret = usb_control_msg(tp->udev, usb_rcvctrlpipe(tp->udev, 0),
RTL8152_REQ_GET_REGS, RTL8152_REQT_READ,
value, index, tmp, size, 500);
+ if (ret < 0)
+ memset(data, 0xff, size);
+ else
+ memcpy(data, tmp, size);
- memcpy(data, tmp, size);
kfree(tmp);
return ret;
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox