* Re: [PATCH v3 net-next 4/9] bnxt_en: Use ignore_ari devlink parameter
From: Vasundhara Volam @ 2018-10-01 4:09 UTC (permalink / raw)
To: gerlitz.or
Cc: David Miller, michael.chan@broadcom.com, Netdev, Jiri Pirko,
jakub.kicinski
In-Reply-To: <CAJ3xEMiYTCUOBhteUG9qZ_SJvPZnJc7q-3UHUqb30nf+Y+XVAw@mail.gmail.com>
On Sun, Sep 30, 2018 at 2:53 AM Or Gerlitz <gerlitz.or@gmail.com> wrote:
>
> On Fri, Sep 28, 2018 at 9:30 AM Vasundhara Volam
> <vasundhara-v.volam@broadcom.com> wrote:
> > This patch adds support for ignore_ari generic permanent mode
> > devlink parameter. This parameter is disabled by default. It can be
> > enabled using devlink param commands.
> >
> > ignore_ari - If enabled, device ignores ARI(Alternate Routing ID)
> > capability, even when platforms has the support and creates same number
> > of partitions when platform does not support ARI capability.
>
> why you want to ignore that?
In some Dell platforms, we don't want our adapter to create more number of
functions even when ARI is enabled.
>
>
> >
> > Cc: Michael Chan <michael.chan@broadcom.com>
> > Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> > ---
> > drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 6 ++++++
> > drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 2 ++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> > index 790c684..5173881 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> > @@ -24,6 +24,8 @@
> > static const struct bnxt_dl_nvm_param nvm_params[] = {
> > {DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV, NVM_OFF_ENABLE_SRIOV,
> > BNXT_NVM_SHARED_CFG, 1},
> > + {DEVLINK_PARAM_GENERIC_ID_IGNORE_ARI, NVM_OFF_IGNORE_ARI,
> > + BNXT_NVM_SHARED_CFG, 1},
> > };
> >
> > static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
> > @@ -108,6 +110,10 @@ static int bnxt_dl_nvm_param_set(struct devlink *dl, u32 id,
> > BIT(DEVLINK_PARAM_CMODE_PERMANENT),
> > bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
> > NULL),
> > + DEVLINK_PARAM_GENERIC(IGNORE_ARI,
> > + BIT(DEVLINK_PARAM_CMODE_PERMANENT),
> > + bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
> > + NULL),
> > };
> >
> > int bnxt_dl_register(struct bnxt *bp)
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
> > index 2f68dc0..da146492 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
> > @@ -33,6 +33,8 @@ static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl)
> > }
> > }
> >
> > +#define NVM_OFF_IGNORE_ARI 164
>
> > +#define NVM_OFF_HW_TC_OFFLOAD 170
>
> what is this (on/off for TC offloads?) and how it's related to ARI?
This was a mistake carried, I will remove it in next version. Thanks
>
> > #define NVM_OFF_ENABLE_SRIOV 401
^ permalink raw reply
* Re: [PATCH v3 net-next 9/9] devlink: Add Documentation/networking/devlink-params-bnxt.txt
From: Vasundhara Volam @ 2018-10-01 4:04 UTC (permalink / raw)
To: Jiří Pírko
Cc: David Miller, michael.chan@broadcom.com, Netdev, Jiri Pirko,
jakub.kicinski
In-Reply-To: <20180929125303.GE2209@nanopsycho.orion>
On Sat, Sep 29, 2018 at 6:27 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Fri, Sep 28, 2018 at 08:28:23AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >This patch adds a new file to add information about configuration
> >parameters that are supported by bnxt_en driver via devlink.
> >
> >Cc: "David S. Miller" <davem@davemloft.net>
> >Cc: Jonathan Corbet <corbet@lwn.net>
> >Cc: linux-doc@vger.kernel.org
> >Cc: Jiri Pirko <jiri@mellanox.com>
> >Cc: Michael Chan <michael.chan@broadcom.com>
> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> >---
> > Documentation/networking/devlink-params-bnxt.txt | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> > create mode 100644 Documentation/networking/devlink-params-bnxt.txt
> >
> >diff --git a/Documentation/networking/devlink-params-bnxt.txt b/Documentation/networking/devlink-params-bnxt.txt
> >new file mode 100644
> >index 0000000..c7bc9d8
> >--- /dev/null
> >+++ b/Documentation/networking/devlink-params-bnxt.txt
> >@@ -0,0 +1,22 @@
> >+enable_sriov [DEVICE, GENERIC]
> >+ Type: Boolean
>
> No need to list "Type" here. You have it in devlink-params.txt for
> generic params already.
Ok, I thought type will give clarity when configuration mode is listed and
when user checks out only driver specific readme file.
I can remove it in next version of patchset.
>
>
> >+ Configuration mode: Permanent
> >+
> >+ignore_ari [DEVICE, GENERIC]
> >+ Type: Boolean
> >+ Configuration mode: Permanent
> >+
> >+msix_vec_per_pf_max [DEVICE, GENERIC]
> >+ Type: u32
> >+ Configuration mode: Permanent
> >+
> >+msix_vec_per_pf_min [DEVICE, GENERIC]
> >+ Type: u32
> >+ Configuration mode: Permanent
> >+
> >+gre_ver_check [DEVICE, DRIVER-SPECIFIC]
> >+ Generic Routing Encapsulation (GRE) version check will
> >+ be enabled in the device. If disabled, device skips
> >+ version checking for incoming packets.
> >+ Type: Boolean
> >+ Configuration mode: Permanent
> >--
> >1.8.3.1
> >
^ permalink raw reply
* [PATCH] cxgb4: Use proper enum in IEEE_FAUX_SYNC
From: Nathan Chancellor @ 2018-10-01 3:47 UTC (permalink / raw)
To: Ganesh Goudar, David S. Miller
Cc: netdev, linux-kernel, Nick Desaulniers, Nathan Chancellor
Clang warns when one enumerated type is implicitly converted to another.
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c:390:4: warning: implicit
conversion from enumeration type 'enum cxgb4_dcb_state' to different
enumeration type 'enum cxgb4_dcb_state_input' [-Wenum-conversion]
IEEE_FAUX_SYNC(dev, dcb);
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h:70:10: note: expanded
from macro 'IEEE_FAUX_SYNC'
CXGB4_DCB_STATE_FW_ALLSYNCED);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Use the equivalent value of the expected type to silence Clang while
resulting in no functional change.
CXGB4_DCB_STATE_FW_ALLSYNCED = CXGB4_DCB_INPUT_FW_ALLSYNCED = 3
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h
index 02040b99c78a..484ee8290090 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h
@@ -67,7 +67,7 @@
do { \
if ((__dcb)->dcb_version == FW_PORT_DCB_VER_IEEE) \
cxgb4_dcb_state_fsm((__dev), \
- CXGB4_DCB_STATE_FW_ALLSYNCED); \
+ CXGB4_DCB_INPUT_FW_ALLSYNCED); \
} while (0)
/* States we can be in for a port's Data Center Bridging.
--
2.19.0
^ permalink raw reply related
* Re: [PATCH net-next v3 10/11] phy: add driver for Microsemi Ocelot SerDes muxing
From: Quentin Schulz @ 2018-10-01 10:02 UTC (permalink / raw)
To: Florian Fainelli
Cc: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, kishon, andrew, allan.nielsen, linux-mips,
devicetree, linux-kernel, netdev, thomas.petazzoni
In-Reply-To: <1dc66b5d-e60e-dad2-eed9-e283de260dc3@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3475 bytes --]
Hi Florian,
On Sat, Sep 15, 2018 at 02:20:25PM -0700, Florian Fainelli wrote:
>
>
> On 09/14/18 01:16, Quentin Schulz wrote:
> > The Microsemi Ocelot can mux SerDes lanes (aka macros) to different
> > switch ports or even make it act as a PCIe interface.
> >
> > This adds support for the muxing of the SerDes.
> >
> > Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> > ---
>
> [snip]
>
> > +
> > +struct serdes_macro {
> > + u8 idx;
> > + /* Not used when in QSGMII or PCIe mode */
> > + int port;
>
> u8 port to be consistent with the mux table?
>
Not wanted in the current implementation.
In serdes_phy_create, I put the port to -1. In serdes_simple_xlate, I
make sure that once port is set to anything else than -1, it cannot be
set again (cannot have 2+ PHYs sharing the same SerDes (except for
SERDES6G_0 which is used for QSGMII)).
I cannot use u8 for this simple reason. However, I'm all ears for a
nicer solution :)
> [snip]
>
> > +#define SERDES_MUX(_idx, _port, _mode, _mask, _mux) { \
> > + .idx = _idx, \
> > + .port = _port, \
> > + .mode = _mode, \
> > + .mask = _mask, \
> > + .mux = _mux, \
> > +}
> > +
> > +static const struct serdes_mux ocelot_serdes_muxes[] = {
> > + SERDES_MUX(SERDES1G_0, 0, PHY_MODE_SGMII, 0, 0),
> > + SERDES_MUX(SERDES1G_1, 1, PHY_MODE_SGMII, HSIO_HW_CFG_DEV1G_5_MODE, 0),
> > + SERDES_MUX(SERDES1G_1, 5, PHY_MODE_SGMII, HSIO_HW_CFG_QSGMII_ENA |
> > + HSIO_HW_CFG_DEV1G_5_MODE, HSIO_HW_CFG_DEV1G_5_MODE),
>
> Why not go one step further and define a SERDES_MUX_SGMII() macro which
> automatically resolves the correct bit definitions to use?
>
> The current macro does not make this particularly easy to read :/
>
I agree on the readability. But SERDES_MUX_SGMII would basically just
abstract the third argument (mode) and that's it, right? That's still
one argument less but do you see something even more intuitive and more
readable?
[...]
> > +
> > + for (i = 0; i < ARRAY_SIZE(ocelot_serdes_muxes); i++) {
> > + if (macro->idx != ocelot_serdes_muxes[i].idx ||
> > + mode != ocelot_serdes_muxes[i].mode)
> > + continue;
> > +
> > + if (mode != PHY_MODE_QSGMII &&
> > + macro->port != ocelot_serdes_muxes[i].port)
> > + continue;
> > +
> > + ret = regmap_update_bits(macro->ctrl->regs, HSIO_HW_CFG,
> > + ocelot_serdes_muxes[i].mask,
> > + ocelot_serdes_muxes[i].mux);
> > + if (ret)
> > + return ret;
> > +
> > + if (macro->idx < SERDES1G_MAX)
> > + return serdes_init_s1g(macro->ctrl->regs, macro->idx);
> > +
> > + /* SERDES6G and PCIe not supported yet */
> > + return 0;
>
> Would not returning -EOPNOTSUPP be more helpful rather than leaving the
> PHY unconfigured (or did the bootloader somehow configure it before for us)?
>
Yup, you're right, if the SerDes needs to be configured by the kernel,
the user of the SerDes mux is "broken" anyway so it makes sense to
return -EOPNOTSUPP.
[...]
> > +
> > + ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL);
> > + if (!ctrl)
> > + return -ENOMEM;
> > +
> > + ctrl->dev = &pdev->dev;
> > + ctrl->regs = syscon_node_to_regmap(pdev->dev.parent->of_node);
> > + if (!ctrl->regs)
> > + return -ENODEV;
> > +
> > + for (i = 0; i <= SERDES_MAX; i++) {
>
> Every other loop you have is using <, is this one off-by-one?
That is an error.
Thanks,
Quentin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH net-next] net: mscc: allow extracting the FCS into the skb
From: Antoine Tenart @ 2018-10-01 9:57 UTC (permalink / raw)
To: davem
Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
alexandre.belloni, quentin.schulz, allan.nielsen
This patch adds support for the NETIF_F_RXFCS feature in the Mscc
Ethernet driver. This feature is disabled by default and allow an user
to request the driver not to drop the FCS and to extract it into the skb
for debugging purposes.
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
drivers/net/ethernet/mscc/ocelot.c | 2 +-
drivers/net/ethernet/mscc/ocelot_board.c | 7 ++++++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 1a4f2bb48ead..eb5119e7e60b 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1606,7 +1606,7 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
dev->ethtool_ops = &ocelot_ethtool_ops;
dev->switchdev_ops = &ocelot_port_switchdev_ops;
- dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+ dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_RXFCS;
dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
memcpy(dev->dev_addr, ocelot->base_mac, ETH_ALEN);
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
index 3cdf63e35b53..245452a0f244 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -126,11 +126,16 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
len += sz;
} while (len < buf_len);
- /* Read the FCS and discard it */
+ /* Read the FCS */
sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
/* Update the statistics if part of the FCS was read before */
len -= ETH_FCS_LEN - sz;
+ if (unlikely(dev->features & NETIF_F_RXFCS)) {
+ buf = (u32 *)skb_put(skb, ETH_FCS_LEN);
+ *buf = val;
+ }
+
if (sz < 0) {
err = sz;
break;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net] net: mscc: fix the frame extraction into the skb
From: Alexandre Belloni @ 2018-10-01 9:49 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, netdev, linux-kernel, thomas.petazzoni, quentin.schulz,
allan.nielsen
In-Reply-To: <20180920100854.19044-1-antoine.tenart@bootlin.com>
On 20/09/2018 12:08:54+0200, Antoine Ténart wrote:
> When extracting frames from the Ocelot switch, the frame check sequence
> (FCS) is present at the end of the data extracted. The FCS was put into
> the sk buffer which introduced some issues (as length related ones), as
> the FCS shouldn't be part of an Rx sk buffer.
>
> This patch fixes the Ocelot switch extraction behaviour by discarding
> the FCS.
>
> Fixes: a556c76adc05 ("net: mscc: Add initial Ocelot switch support")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>
> Hi all,
>
> Once this patch will be accepted and net merged into net-next, I'll
> probably send another patch to fill skb->csum with the FCS based on
> NETIF_F_RXFCS.
>
> Thanks!
> Antoine
>
> drivers/net/ethernet/mscc/ocelot_board.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
> index 26bb3b18f3be..3cdf63e35b53 100644
> --- a/drivers/net/ethernet/mscc/ocelot_board.c
> +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> @@ -91,7 +91,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
> struct sk_buff *skb;
> struct net_device *dev;
> u32 *buf;
> - int sz, len;
> + int sz, len, buf_len;
> u32 ifh[4];
> u32 val;
> struct frame_info info;
> @@ -116,14 +116,20 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
> err = -ENOMEM;
> break;
> }
> - buf = (u32 *)skb_put(skb, info.len);
> + buf_len = info.len - ETH_FCS_LEN;
> + buf = (u32 *)skb_put(skb, buf_len);
>
> len = 0;
> do {
> sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
> *buf++ = val;
> len += sz;
> - } while ((sz == 4) && (len < info.len));
> + } while (len < buf_len);
> +
> + /* Read the FCS and discard it */
> + sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
> + /* Update the statistics if part of the FCS was read before */
> + len -= ETH_FCS_LEN - sz;
>
> if (sz < 0) {
> err = sz;
> --
> 2.17.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net-next v3 11/11] net: mscc: ocelot: make use of SerDes PHYs for handling their configuration
From: Quentin Schulz @ 2018-10-01 9:42 UTC (permalink / raw)
To: Florian Fainelli
Cc: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, kishon, andrew, allan.nielsen, linux-mips,
devicetree, linux-kernel, netdev, thomas.petazzoni
In-Reply-To: <0f762d63-a392-d2fe-a121-a013a13a8584@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2479 bytes --]
Hi Florian,
On Sat, Sep 15, 2018 at 02:25:05PM -0700, Florian Fainelli wrote:
>
>
> On 09/14/18 01:16, Quentin Schulz wrote:
> > Previously, the SerDes muxing was hardcoded to a given mode in the MAC
> > controller driver. Now, the SerDes muxing is configured within the
> > Device Tree and is enforced in the MAC controller driver so we can have
> > a lot of different SerDes configurations.
> >
> > Make use of the SerDes PHYs in the MAC controller to set up the SerDes
> > according to the SerDes<->switch port mapping and the communication mode
> > with the Ethernet PHY.
>
> This looks good, just a few comments below:
>
> [snip]
>
> > + err = of_get_phy_mode(portnp);
> > + if (err < 0)
> > + ocelot->ports[port]->phy_mode = PHY_INTERFACE_MODE_NA;
> > + else
> > + ocelot->ports[port]->phy_mode = err;
> > +
> > + switch (ocelot->ports[port]->phy_mode) {
> > + case PHY_INTERFACE_MODE_NA:
> > + continue;
>
> Would not you want to issue a message indicating that the Device Tree
> must be updated here? AFAICT with your patch series, this should no
> longer be a condition that you will hit unless you kept the old DTB
> around, right?
>
It'll occur for internal PHYs. On the PCB123[1], there are four of them,
so we need to be able to give no mode in the DT for those. For the
upcoming PCB120, there'll be 4 external PHYs that require a mode in the
DT and 4 internal PHYs that do not require any mode. I could put a debug
message that says this or that PHY is configured as an internal PHY but
I wouldn't put a message that is printed with the default log level.
So I think we should keep it, shouldn't we?
[1] https://elixir.bootlin.com/linux/latest/source/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
> > + case PHY_INTERFACE_MODE_SGMII:
> > + phy_mode = PHY_MODE_SGMII;
> > + break;
> > + case PHY_INTERFACE_MODE_QSGMII:
> > + phy_mode = PHY_MODE_QSGMII;
> > + break;
> > + default:
> > + dev_err(ocelot->dev,
> > + "invalid phy mode for port%d, (Q)SGMII only\n",
> > + port);
> > + return -EINVAL;
> > + }
> > +
> > + serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> > + if (IS_ERR(serdes)) {
> > + err = PTR_ERR(serdes);
> > + if (err == -EPROBE_DEFER) {
>
> This can be simplified into:
>
> if (err == -EPROBE_DEFER)
> dev_dbg();
> else
> dev_err();
> goto err_probe_ports;
>
Indeed, good catch.
Thanks,
Quentin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 2/7] net: phy: mscc: add support for VSC8584 PHY
From: Quentin Schulz @ 2018-10-01 9:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, f.fainelli, allan.nielsen, linux-mips,
devicetree, linux-kernel, netdev, thomas.petazzoni,
antoine.tenart
In-Reply-To: <20180914172754.GC3811@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]
Hi Andrew,
On Fri, Sep 14, 2018 at 07:27:54PM +0200, Andrew Lunn wrote:
>
> > struct vsc8531_private {
> > int rate_magic;
> > u16 supp_led_modes;
> > @@ -181,6 +354,7 @@ struct vsc8531_private {
> > struct vsc85xx_hw_stat *hw_stats;
> > u64 *stats;
> > int nstats;
> > + bool pkg_init;
>
> > +/* bus->mdio_lock should be locked when using this function */
> > +static int vsc8584_cmd(struct mii_bus *bus, int phy, u16 val)
> > +{
> > + unsigned long deadline;
> > + u16 reg_val;
> > +
> > + __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS,
> > + MSCC_PHY_PAGE_EXTENDED_GPIO);
> > +
> > + __mdiobus_write(bus, phy, MSCC_PHY_PROC_CMD, PROC_CMD_NCOMPLETED | val);
>
> Hi Quentin
>
> All the __mdiobus_write() look a bit ugly.
I agree :)
> Maybe add bus and base_addr
> to the vsc8531_private structure. Then add helpers
> phy_write_base_phy(priv, reg, val) and phy_read_base_phy(priv, reg).
>
ACK.
> You could also add in:
>
> if (unlikely(!mutex_is_locked(&priv->bus->mdio_lock))) {
> dev_err(bus->dev, "MDIO bus lock not held!\n");
> dump_stack();
> }
>
> Having such code in the mv88e6xxx driver has found a few bugs for me.
>
ACK.
Thanks,
Quentin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net v2] r8169: always autoneg on resume
From: Andrew Lunn @ 2018-10-01 2:39 UTC (permalink / raw)
To: Alex Xu; +Cc: Daan Wendelen, netdev, hkallweit1, nic_swsd, davem, linux-kernel
In-Reply-To: <153834665886.421.6376790866713055481@pink.alxu.ca>
On Sun, Sep 30, 2018 at 10:30:58PM +0000, Alex Xu wrote:
> Quoting Daan Wendelen (2018-09-30 22:17:51)
> > Hi Alex,
> >
> > I randomly opened your patch even though I have absolutely no idea what this patch is about, but I
> > found a mistake in one of the comments:
> >
> > On Sun, Sep 30, 2018 at 11:06:39AM -0400, Alex Xu (Hello71) wrote:
> > > This affects at least versions 25 and 33, so assume all cards are broken
> > ...
> > > * 1GBit link after resuming from S3. For whatever reason the PHY on
> > > - * this chip doesn't properly start a renegotiation when soft-reset.
> > > + * these chips doesn't properly start a renegotiation when soft-reset.
> > ~~~~~~~
> > I believe it should be "these chips don't"
> >
> > With kind regards,
> > Daan
>
> The grammar is correct as is. The subject of the sentence is "the PHY",
> which is singular. However, I think it should be "the PHYs" instead,
> assuming that they are different.
I keep reading the patch wrong, so here is the end state:
/* It was reported that several chips end up with 10MBit/Half on a
* 1GBit link after resuming from S3. For whatever reason the PHY on
* these chips doesn't properly start a renegotiation when soft-reset.
* Explicitly requesting a renegotiation fixes this.
*/
I would also say 'don't'. For me the subject is 'PHY on these chips',
which is plural.
However:
For whatever reason the PHY, on these chips, doesn't properly start a
renegotiation when soft-reset.
Now just 'the PHY' is the subject, so singular.
But i'm just a native speaker who never actually learnt the rules of
grammar, it just sounds right or wrong, and i have no idea why.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 3/7] net: phy: mscc: split config_init in two functions for VSC8584
From: Quentin Schulz @ 2018-10-01 9:07 UTC (permalink / raw)
To: Florian Fainelli
Cc: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, andrew, allan.nielsen, linux-mips,
devicetree, linux-kernel, netdev, thomas.petazzoni,
antoine.tenart
In-Reply-To: <bc31175f-52eb-cb7d-669e-192aec79ac5c@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]
Hi Florian,
On Fri, Sep 14, 2018 at 10:57:08AM -0700, Florian Fainelli wrote:
> On 09/14/2018 02:44 AM, Quentin Schulz wrote:
> > Part of the config init is common between the VSC8584 and the VSC8574,
> > so to prepare the upcoming support for VSC8574, separate config_init
> > PHY-specific code to config_pre_init function which is set in the probe
> > function of the PHY and used in config_init.
> >
> > Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> > ---
> > drivers/net/phy/mscc.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
> > index b450489..69cc3cf 100644
> > --- a/drivers/net/phy/mscc.c
> > +++ b/drivers/net/phy/mscc.c
> > @@ -355,6 +355,7 @@ struct vsc8531_private {
> > u64 *stats;
> > int nstats;
> > bool pkg_init;
> > + int (*config_pre_init)(struct mii_bus *bus, int phy);
>
> Is not this overkill given that you have a reference to the phy_device,
> you could check for the for phy_id to know which exact type you have and
> call the appropriate pre_init function?
>
Agreed. It just seemed "cleaner" to me to set config_pre_init in
separate probe functions which are pretty straightforward and short.
Thus not complexifying an already-not-so-straightforward config_init
function.
Anyway, I'll use the phy_id as suggested so that we don't overload the
vsc8531_private structure (since it's also only called once, in
config_init).
Thanks,
Quentin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 5/7] MIPS: mscc: ocelot: add GPIO4 pinmuxing DT node
From: Quentin Schulz @ 2018-10-01 9:01 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexandre Belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, f.fainelli, allan.nielsen, linux-mips,
devicetree, linux-kernel, netdev, thomas.petazzoni,
antoine.tenart
In-Reply-To: <20180914170221.GB3811@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]
Hi Andrew,
On Fri, Sep 14, 2018 at 07:02:21PM +0200, Andrew Lunn wrote:
> On Fri, Sep 14, 2018 at 06:26:38PM +0200, Quentin Schulz wrote:
> > Hi Alexandre,
> >
> > On Fri, Sep 14, 2018 at 04:54:46PM +0200, Alexandre Belloni wrote:
> > > Hi,
> > >
> > > On 14/09/2018 11:44:26+0200, Quentin Schulz wrote:
> > > > In order to use GPIO4 as a GPIO, we need to mux it in this mode so let's
> > > > declare a new pinctrl DT node for it.
> > > >
> > > > Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> > > > ---
> > > > arch/mips/boot/dts/mscc/ocelot.dtsi | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/arch/mips/boot/dts/mscc/ocelot.dtsi b/arch/mips/boot/dts/mscc/ocelot.dtsi
> > > > index 8ce317c..b5c4c74 100644
> > > > --- a/arch/mips/boot/dts/mscc/ocelot.dtsi
> > > > +++ b/arch/mips/boot/dts/mscc/ocelot.dtsi
> > > > @@ -182,6 +182,11 @@
> > > > interrupts = <13>;
> > > > #interrupt-cells = <2>;
> > > >
> > > > + gpio4: gpio4 {
> > > > + pins = "GPIO_4";
> > > > + function = "gpio";
> > > > + };
> > > > +
> > >
> > > For a GPIO, I would do that in the board dts because it is not used
> > > directly in the dtsi.
> > >
> >
> > And the day we've two boards using this pinctrl we move it to a dtsi. Is
> > that the plan?
>
> Hi Quentin
>
> gpio4 appears to be pretty arbitrary. Could a different design use a
> different gpio? It me, this seems like a board property.
>
Right now, I don't see why it couldn't be.
Quentin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 2/5] net: phy: mscc: Add EEE init sequence
From: Quentin Schulz @ 2018-10-01 8:51 UTC (permalink / raw)
To: Florian Fainelli
Cc: davem, andrew, allan.nielsen, linux-kernel, netdev,
thomas.petazzoni, Raju Lakkaraju
In-Reply-To: <1321f1a0-57af-a059-f78a-a699da94c064@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]
Hi Florian,
On Fri, Sep 14, 2018 at 07:21:09PM -0700, Florian Fainelli wrote:
>
>
> On 09/14/18 01:33, Quentin Schulz wrote:
> > From: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> >
> > Microsemi PHYs (VSC 8530/31/40/41) need to update the Energy Efficient
> > Ethernet initialization sequence.
> > In order to avoid certain link state errors that could result in link
> > drops and packet loss, the physical coding sublayer (PCS) must be
> > updated with settings related to EEE in order to improve performance.
> >
> > Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> > Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> > ---
>
> [snip]
>
> > + vsc85xx_tr_write(phydev, 0x0f82, 0x0012b00a);
>
> Can you just make this an array of register + value pair? That would be
Sure, I'll.
> less error prone in case you need to update that sequence in the future.
>
I'm curious about the kind of errors you're worrying about or have
experienced. Do you have any particular example or thought in mind?
> With that:
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks,
Quentin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* OFFIZIELLE GEWINNBENACHRITIGUNG
From: EL GORDO LOTO INTERNATIONAL @ 2018-10-01 17:32 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 555 bytes --]
Sehr geehrter Gewinner
Wir wurden beauftragt, Ihnen zu helfen Ihr Gewinn zu beanspruche in der letzten Sitzung der
European Elgordo Lottery 2017/18. Ihre Informationen wurden aus einer Online-Umfrage und
Zahlungssystemen ausgewählt.
Aus Tausenden von E-Mails und Namen erhalten von den internationalen Umfrage und
Zahlungssystemen, um Teilnahme an der Europäischen Online Elgordo Lotterie 2017/2018, Sie ein
Gewinner in der Zweiter Kategorie sind mit Gewinnnummer (51244), gezogen am Fritag, 22.
Dezember 2017, - Gewinnen betrag ?1.250.000,00
[-- Attachment #2: Gewinner new.pdf --]
[-- Type: application/pdf, Size: 461405 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] rtnetlink: fix rtnl_fdb_dump() for shorter family headers
From: David Ahern @ 2018-10-01 1:06 UTC (permalink / raw)
To: Mauricio Faria de Oliveira, netdev; +Cc: davem
In-Reply-To: <20180928193516.13308-1-mfo@canonical.com>
On 9/28/18 1:35 PM, Mauricio Faria de Oliveira wrote:
> Currently, rtnl_fdb_dump() assumes the family header is 'struct ifinfomsg',
> which is not always true. For example, 'struct ndmsg' is used by iproute2
> as well (in the 'ip neigh' command).
>
> The problem is, the function bails out early if nlmsg_parse() fails, which
> does occur for iproute2 usage of 'struct ndmsg' because the payload length
> is shorter than the family header alone (as 'struct ifinfomsg' is assumed).
>
> This breaks backward compatibility with userspace (different response) and
> is a regression due to commit 0ff50e83b512 ("net: rtnetlink: bail out from
> rtnl_fdb_dump() on parse error").
...
>
> Fixes: 0ff50e83b512 ("net: rtnetlink: bail out from rtnl_fdb_dump() on parse error")
> Fixes: 5e6d24358799 ("bridge: netlink dump interface at par with brctl")
> Reported-by: Aidan Obley <aobley@pivotal.io>
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
> ---
> P.S.: this may be 'net', but labeling as 'net-next' for possible relation to recent thread
> [PATCH RFC net-next 0/5] rtnetlink: Add support for rigid checking of data in dump request
>
> net/core/rtnetlink.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 60c928894a78..9695a27cc9b9 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -3744,16 +3744,17 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
> int err = 0;
> int fidx = 0;
>
> - err = nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb,
> - IFLA_MAX, ifla_policy, NULL);
> - if (err < 0) {
> - return -EINVAL;
> - } else if (err == 0) {
> + /* The family header may _not_ be struct ifinfomsg
> + * (e.g., struct ndmsg). Usage of the ifm pointer
> + * must check payload length (e.g., nlmsg_parse()).
> + */
> + if (nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb,
> + IFLA_MAX, ifla_policy, NULL) == 0) {
> if (tb[IFLA_MASTER])
> br_idx = nla_get_u32(tb[IFLA_MASTER]);
> - }
>
> - brport_idx = ifm->ifi_index;
> + brport_idx = ifm->ifi_index;
> + }
>
> if (br_idx) {
> br_dev = __dev_get_by_index(net, br_idx);
>
I suspect rtnl_fdb_dump is forever stuck with the ifinfomsg struct as
the header if any kernel side filtering is to be done. As for the change
above, I suggest something like this:
/* if header struct is ndmsg, no attributes can be appended */
if (nlmsg_len(nlh) != sizeof(struct ndmsg)) {
current ifinfomsg based code
}
We certainly do not want to ignore parse failures.
^ permalink raw reply
* KASAN: use-after-free Read in xfrm_sk_policy_lookup
From: syzbot @ 2018-10-01 6:55 UTC (permalink / raw)
To: davem, herbert, linux-kernel, netdev, steffen.klassert,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 291d0e5d81e1 Merge tag 'for-linus-20180929' of git://git.k..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=173b614e400000
kernel config: https://syzkaller.appspot.com/x/.config?x=a8212f992609a887
dashboard link: https://syzkaller.appspot.com/bug?extid=fa158127e8a7f651dbbf
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+fa158127e8a7f651dbbf@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in xfrm_sk_policy_lookup+0x61c/0x640
net/xfrm/xfrm_policy.c:1191
Read of size 2 at addr ffff8801995e5d1c by task syz-executor1/9885
CPU: 0 PID: 9885 Comm: syz-executor1 Not tainted 4.19.0-rc5+ #38
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
__asan_report_load2_noabort+0x14/0x20 mm/kasan/report.c:431
xfrm_sk_policy_lookup+0x61c/0x640 net/xfrm/xfrm_policy.c:1191
xfrm_lookup_with_ifid+0x2a1/0x2b80 net/xfrm/xfrm_policy.c:2063
xfrm_lookup net/xfrm/xfrm_policy.c:2200 [inline]
xfrm_lookup_route+0x3c/0x1f0 net/xfrm/xfrm_policy.c:2211
ip6_dst_lookup_flow+0x1c6/0x270 net/ipv6/ip6_output.c:1083
inet6_csk_route_socket+0x8bd/0x1020 net/ipv6/inet6_connection_sock.c:110
inet6_csk_xmit+0x118/0x630 net/ipv6/inet6_connection_sock.c:125
l2tp_xmit_core net/l2tp/l2tp_core.c:1030 [inline]
l2tp_xmit_skb+0x134c/0x1670 net/l2tp/l2tp_core.c:1132
pppol2tp_sendmsg+0x4b1/0x6c0 net/l2tp/l2tp_ppp.c:329
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:631
___sys_sendmsg+0x51d/0x930 net/socket.c:2116
__sys_sendmmsg+0x246/0x6d0 net/socket.c:2211
__do_sys_sendmmsg net/socket.c:2240 [inline]
__se_sys_sendmmsg net/socket.c:2237 [inline]
__x64_sys_sendmmsg+0x9d/0x100 net/socket.c:2237
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457579
Code: 1d b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 eb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f36fada1c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000133
RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 0000000000457579
RDX: 0800000000000059 RSI: 0000000020005fc0 RDI: 0000000000000007
RBP: 000000000072bf00 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000003 R11: 0000000000000246 R12: 00007f36fada26d4
R13: 00000000004c34ed R14: 00000000004d52b0 R15: 00000000ffffffff
Allocated by task 9885:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620
kmalloc include/linux/slab.h:513 [inline]
kzalloc include/linux/slab.h:707 [inline]
xfrm_policy_alloc+0xd9/0x480 net/xfrm/xfrm_policy.c:267
xfrm_compile_policy+0x257/0x430 net/xfrm/xfrm_user.c:2994
xfrm_user_policy+0x3ef/0xb10 net/xfrm/xfrm_state.c:2102
do_ipv6_setsockopt.isra.9+0xfd6/0x45d0 net/ipv6/ipv6_sockglue.c:821
ipv6_setsockopt+0xbd/0x170 net/ipv6/ipv6_sockglue.c:933
udpv6_setsockopt+0x62/0xa0 net/ipv6/udp.c:1482
sock_common_setsockopt+0x9a/0xe0 net/core/sock.c:3038
__sys_setsockopt+0x1ba/0x3c0 net/socket.c:1902
__do_sys_setsockopt net/socket.c:1913 [inline]
__se_sys_setsockopt net/socket.c:1910 [inline]
__x64_sys_setsockopt+0xbe/0x150 net/socket.c:1910
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 32034:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3813
xfrm_policy_destroy_rcu+0x4a/0x60 net/xfrm/xfrm_policy.c:290
__rcu_reclaim kernel/rcu/rcu.h:236 [inline]
rcu_do_batch kernel/rcu/tree.c:2576 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2880 [inline]
__rcu_process_callbacks kernel/rcu/tree.c:2847 [inline]
rcu_process_callbacks+0xf23/0x2670 kernel/rcu/tree.c:2864
__do_softirq+0x30b/0xad8 kernel/softirq.c:292
The buggy address belongs to the object at ffff8801995e5b00
which belongs to the cache kmalloc-1024 of size 1024
The buggy address is located 540 bytes inside of
1024-byte region [ffff8801995e5b00, ffff8801995e5f00)
The buggy address belongs to the page:
page:ffffea0006657900 count:1 mapcount:0 mapping:ffff8801da800ac0 index:0x0
compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffffea000730b108 ffffea00075ef008 ffff8801da800ac0
raw: 0000000000000000 ffff8801995e4000 0000000100000007 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801995e5c00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801995e5c80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801995e5d00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8801995e5d80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801995e5e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: [PATCH] gpiolib: Fix incorrect use of find_next_zero_bit()
From: Marek Szyprowski @ 2018-10-01 6:46 UTC (permalink / raw)
To: Janusz Krzysztofik, Linus Walleij
Cc: Andrew Lunn, Ulf Hansson, Tony Lindgren, Dominik Brodowski,
Yegor Yefremov, netdev, linux-i2c, Peter Meerwald-Stadler, devel,
Florian Fainelli, Peter Rosin, Krzysztof Kozlowski,
Kishon Vijay Abraham I, linux-iio, Peter Korsgaard,
Geert Uytterhoeven, linux-serial, Jiri Slaby, Michael Hennerich,
Uwe Kleine-König, linux-gpio, Russell King,
Lars-Peter Clausen
In-Reply-To: <20180929122022.6825-1-jmkrzyszt@gmail.com>
Hi Janusz,
On 2018-09-29 14:20, Janusz Krzysztofik wrote:
> Commit b17566a6b08b ("gpiolib: Implement fast processing path in
> get/set array"), already fixed to some extent with commit 5d581d7e8cdc
> ("gpiolib: Fix missing updates of bitmap index"), introduced a new mode
> of processing bitmaps where bits applicable for fast bitmap processing
> path are supposed to be skipped while iterating bits which don't apply.
> Unfortunately, find_next_zero_bit() function supposed to skip over
> those fast bits is always called with a 'start' argument equal to an
> index of last zero bit found and returns that index value again an
> again, causing an infinite loop.
>
> Fix it by incrementing the index uncoditionally before
> find_next_zero_bit() is optionally called.
>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> Marek,
>
> Could you please test it on top of next-20180920 with "gpiolib: Fix
> missing updates of bitmap index" and optionally "mmc: pwrseq_simple:
> Fix incorrect handling of GPIO bitmap" also applied?
This patch finally fixes the boot issue on Samsung Chromebook Snow.
Thanks!
>
> Thanks,
> Janusz
>
>
> drivers/gpio/gpiolib.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 6ae13e3e05f1..940b543e966d 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2878,12 +2878,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
> int hwgpio = gpio_chip_hwgpio(desc);
>
> __set_bit(hwgpio, mask);
> + i++;
>
> if (array_info)
> i = find_next_zero_bit(array_info->get_mask,
> array_size, i);
> - else
> - i++;
> } while ((i < array_size) &&
> (desc_array[i]->gdev->chip == chip));
>
> @@ -2903,12 +2902,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
> value = !value;
> __assign_bit(j, value_bitmap, value);
> trace_gpio_value(desc_to_gpio(desc), 1, value);
> + j++;
>
> if (array_info)
> j = find_next_zero_bit(array_info->get_mask, i,
> j);
> - else
> - j++;
> }
>
> if (mask != fastpath)
> @@ -3191,12 +3189,11 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
> __clear_bit(hwgpio, bits);
> count++;
> }
> + i++;
>
> if (array_info)
> i = find_next_zero_bit(array_info->set_mask,
> array_size, i);
> - else
> - i++;
> } while ((i < array_size) &&
> (desc_array[i]->gdev->chip == chip));
> /* push collected bits to outputs */
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH net v2] r8169: always autoneg on resume
From: Daan Wendelen @ 2018-10-01 5:33 UTC (permalink / raw)
To: Alex Xu; +Cc: netdev, hkallweit1, nic_swsd, davem, linux-kernel
In-Reply-To: <153834665886.421.6376790866713055481@pink.alxu.ca>
On Sun, Sep 30, 2018 at 10:30:58PM +0000, Alex Xu wrote:
> Quoting Daan Wendelen (2018-09-30 22:17:51)
> > Hi Alex,
> >
> > I randomly opened your patch even though I have absolutely no idea what this patch is about, but I
> > found a mistake in one of the comments:
> >
> > On Sun, Sep 30, 2018 at 11:06:39AM -0400, Alex Xu (Hello71) wrote:
> > > This affects at least versions 25 and 33, so assume all cards are broken
> > ...
> > > * 1GBit link after resuming from S3. For whatever reason the PHY on
> > > - * this chip doesn't properly start a renegotiation when soft-reset.
> > > + * these chips doesn't properly start a renegotiation when soft-reset.
> > ~~~~~~~
> > I believe it should be "these chips don't"
> >
> > With kind regards,
> > Daan
>
> The grammar is correct as is. The subject of the sentence is "the PHY",
> which is singular.
You are right, I misread the patch. I thought that "these chips" was
the subject, but I believe "the PHY on these chips" is the subject.
> However, I think it should be "the PHYs" instead,
> assuming that they are different.
I can't say.
I say we leave it as it is because the comment will look good enough.
^ permalink raw reply
* Re: [PATCH v3] PCI: Reprogram bridge prefetch registers on resume
From: Daniel Drake @ 2018-10-01 4:57 UTC (permalink / raw)
To: Thomas Martitz
Cc: andy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ, Linux PM, Linux PCI,
Wysocki, Rafael J, Linux Kernel, Keith Busch, netdev,
Bjorn Helgaas, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
Bjorn Helgaas, rchang-eYqpPyKDWXRBDgjK7y7TUQ,
Linux Upstreaming Team, David Miller, Jon Derrick,
Heiner Kallweit
In-Reply-To: <3f8efcd6-7cd2-b41a-d984-28c9e1740d6c-UCKwuKHb1aVAfugRpC6u6w@public.gmane.org>
On Sun, Sep 30, 2018 at 5:07 AM Thomas Martitz <kugel@rockbox.org> wrote:
> The latest iteration does not work on my HP system. The GPU fails to
> power up just like the unpatched kernel.
That's weird, I would not expect a behaviour change in the latest
patch. pci_restore_config_dword() has some debug messages, could you
please make them visible and show logs again?
Also remind us of the PCI device address of the parent bridge (lspci -vt)
Thanks
Daniel
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply
* Re: [PATCH net v2] r8169: always autoneg on resume
From: Daan Wendelen @ 2018-09-30 22:17 UTC (permalink / raw)
To: Alex Xu (Hello71); +Cc: netdev, hkallweit1, nic_swsd, davem, linux-kernel
In-Reply-To: <1538319728.z22jaatdgh.astroid@alex-desktop.none>
Hi Alex,
I randomly opened your patch even though I have absolutely no idea what this patch is about, but I
found a mistake in one of the comments:
On Sun, Sep 30, 2018 at 11:06:39AM -0400, Alex Xu (Hello71) wrote:
> This affects at least versions 25 and 33, so assume all cards are broken
...
> * 1GBit link after resuming from S3. For whatever reason the PHY on
> - * this chip doesn't properly start a renegotiation when soft-reset.
> + * these chips doesn't properly start a renegotiation when soft-reset.
~~~~~~~
I believe it should be "these chips don't"
With kind regards,
Daan
^ permalink raw reply
* Re: [PATCH net] ipv4: fix use-after-free in ip_cmsg_recv_dstaddr()
From: Willem de Bruijn @ 2018-09-30 22:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Network Development, Eric Dumazet, Willem de Bruijn
In-Reply-To: <20180930183339.153681-1-edumazet@google.com>
On Sun, Sep 30, 2018 at 2:37 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Caching ip_hdr(skb) before a call to pskb_may_pull() is buggy,
> do not do it.
>
> Fixes: 2efd4fca703a ("ip: in cmsg IP(V6)_ORIGDSTADDR call pskb_may_pull")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Acked-by: Willem de Bruijn <willemb@google.com>
Argh, indeed. Thanks for fixing this, Eric.
^ permalink raw reply
* [PATCH] dpaa_eth: Remove useless declaration
From: Nathan Chancellor @ 2018-10-01 4:10 UTC (permalink / raw)
To: Madalin Bucur, David S. Miller; +Cc: netdev, linux-kernel, Nathan Chancellor
Clang warns:
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:2734:34: warning:
tentative array definition assumed to have one element
static const struct of_device_id dpaa_match[];
^
1 warning generated.
Turns out that since this driver was introduced in commit 9ad1a3749333
("dpaa_eth: add support for DPAA Ethernet"), this declaration has been
unused. Remove it to silence the warning.
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 84843de25c7b..6e0f47f2c8a3 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2731,8 +2731,6 @@ static int dpaa_ingress_cgr_init(struct dpaa_priv *priv)
return err;
}
-static const struct of_device_id dpaa_match[];
-
static inline u16 dpaa_get_headroom(struct dpaa_buffer_layout *bl)
{
u16 headroom;
--
2.19.0
^ permalink raw reply related
* [PATCH] cxgb4: Use proper enum in cxgb4_dcb_handle_fw_update
From: Nathan Chancellor @ 2018-10-01 3:51 UTC (permalink / raw)
To: Ganesh Goudar, David S. Miller
Cc: netdev, linux-kernel, Nick Desaulniers, Nathan Chancellor
Clang warns when one enumerated type is implicitly converted to another.
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c:303:7: warning: implicit
conversion from enumeration type 'enum cxgb4_dcb_state' to different
enumeration type 'enum cxgb4_dcb_state_input' [-Wenum-conversion]
? CXGB4_DCB_STATE_FW_ALLSYNCED
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c:304:7: warning: implicit
conversion from enumeration type 'enum cxgb4_dcb_state' to different
enumeration type 'enum cxgb4_dcb_state_input' [-Wenum-conversion]
: CXGB4_DCB_STATE_FW_INCOMPLETE);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
Use the equivalent value of the expected type to silence Clang while
resulting in no functional change.
CXGB4_DCB_STATE_FW_INCOMPLETE = CXGB4_DCB_INPUT_FW_INCOMPLETE = 2
CXGB4_DCB_STATE_FW_ALLSYNCED = CXGB4_DCB_INPUT_FW_ALLSYNCED = 3
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
index 6ba3104ff7eb..9bd5f755a0e0 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
@@ -300,8 +300,8 @@ void cxgb4_dcb_handle_fw_update(struct adapter *adap,
enum cxgb4_dcb_state_input input =
((pcmd->u.dcb.control.all_syncd_pkd &
FW_PORT_CMD_ALL_SYNCD_F)
- ? CXGB4_DCB_STATE_FW_ALLSYNCED
- : CXGB4_DCB_STATE_FW_INCOMPLETE);
+ ? CXGB4_DCB_INPUT_FW_ALLSYNCED
+ : CXGB4_DCB_INPUT_FW_INCOMPLETE);
if (dcb->dcb_version != FW_PORT_DCB_VER_UNKNOWN) {
dcb_running_version = FW_PORT_CMD_DCB_VERSION_G(
--
2.19.0
^ permalink raw reply related
* Re: [PATCH net v2] r8169: always autoneg on resume
From: Alex Xu @ 2018-10-01 3:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: Daan Wendelen, netdev, hkallweit1, nic_swsd, davem, linux-kernel
In-Reply-To: <20181001023947.GA4943@lunn.ch>
Quoting Andrew Lunn (2018-10-01 02:39:47)
> I keep reading the patch wrong, so here is the end state:
>
> /* It was reported that several chips end up with 10MBit/Half on a
> * 1GBit link after resuming from S3. For whatever reason the PHY on
> * these chips doesn't properly start a renegotiation when soft-reset.
> * Explicitly requesting a renegotiation fixes this.
> */
>
> I would also say 'don't'. For me the subject is 'PHY on these chips',
> which is plural.
>
> However:
>
> For whatever reason the PHY, on these chips, doesn't properly start a
> renegotiation when soft-reset.
>
> Now just 'the PHY' is the subject, so singular.
>
> But i'm just a native speaker who never actually learnt the rules of
> grammar, it just sounds right or wrong, and i have no idea why.
No. Ending in a plural noun doesn't make a noun phrase plural.
Example 1: The boy with the books is walking.
Example 2: The women on the roof are waving.
In these cases, "with" and "on" introduce prepositional phrases
which are subordinate to the noun phrase. The subjects are "boy" and
"women" respectively; the books are not walking, and the roof is not
waving.
In fact, this can be clearly seen by drawing a parse tree, which should
hopefully be a familiar tool to all programmers.
Furthermore, the addition of commas in these cases is not correct (hey,
another example!); the phrases are essential, so commas may not be used.
Regardless, this is highly off-topic, so I think further replies can go
off-list.
^ permalink raw reply
* Re: [PATCH net-next v6 01/23] asm: simd context helper API
From: Jason A. Donenfeld @ 2018-10-01 1:43 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Joe Perches, Ard Biesheuvel, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Thomas Gleixner, linux-arch
In-Reply-To: <3E9E1888-026E-45C1-8AA7-DADA211EDBDF@amacapital.net>
On Sun, Sep 30, 2018 at 7:35 AM Andy Lutomirski <luto@amacapital.net> wrote:
> >>>>>>> Oh, and another thing (and I'm surprised checkpatch.pl didn't complain
> >>>>>>> about it): the use of typedef in new code is strongly discouraged.
> >>>>>>> This policy predates my involvement, so perhaps Joe can elaborate on
> >>>>>>> the rationale?
> >>>>>>
> >>>>>> In case it matters, the motivation for making this a typedef is I
> >>>>>> could imagine this at some point turning into a more complicated
> >>>>>> struct on certain platforms and that would make refactoring easier. I
> >>>>>> could just make it `struct simd_context` now with 1 member though...
> >>>>>
> >>>>> Yes that makes sense
> >>>>
> >>>> The rationale for it being a typedef or moving to a struct now?
> >>>
> >>> Yes just switch to a struct.
> >>
> >> Okay. No problem with that, but will wait to hear from Joe first.
> >
> > Why do you need to hear from me again?
> >
> > As far as I know, the only info about typedef avoidance are in
> > Documentation/process/coding-style.rst section 5.
> >
> >
>
> I personally prefer it with the typedef. If this were my code, I’d say the coding style is silly for opaque tiny structs like this.
I'll stick with a typedef. Reading the style guide, this clearly falls
into 5.a, 5.b, and maybe 5.c. For 5.a, at some point this will
possibly contain architecture specific blobs. For 5.b, it is just an
enum (integer) right now.
^ permalink raw reply
* Re: [PATCH iproute2-next 00/11] Fix dump requests to use proper header for type
From: Christian Brauner @ 2018-09-30 18:45 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, stephen, David Ahern
In-Reply-To: <20180929175931.18448-1-dsahern@kernel.org>
On Sat, Sep 29, 2018 at 10:59:20AM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
>
> iproute2 currently uses ifinfomsg as the header for all dumps using the
> wilddump headers. This is wrong as each message type actually has its own
> header type. While the kernel has traditionally let it go as it for the
> most part only uses the family entry, the use of kernel side filters is
> increasing to alter what is returned on a request. The kernel side filters
> really need to use the proper header type.
>
> To that end, fix iproute2 to use the proper header struct for the GET type.
Thanks for doing this, David!
Christian
>
> David Ahern (11):
> libnetlink: Convert GETADDR dumps to use rtnl_addrdump_req
> libnetlink: Convert GETROUTE dumps to use rtnl_routedump_req
> libnetlink: Convert GETADDRLABEL dumps to use rtnl_addrlbldump_req
> libnetlink: Convert GETMDB dumps to use rtnl_mdbdump_req
> libnetlink: Convert GETNETCONF dumps to use rtnl_netconfdump_req
> libnetlink: Convert GETRULE dumps to use rtnl_ruledump_req
> libnetlink: Convert GETNEIGH dumps to use rtnl_neighdump_req
> libnetlink: Convert GETNEIGHTBL dumps to use rtnl_neightbldump_req
> libnetlink: Convert GETNSID dumps to use rtnl_nsiddump_req
> libnetlink: Rename rtnl_wilddump_* to rtnl_linkdump_*
> libnetlink: Rename rtnl_wilddump_stats_req_filter to
> rtnl_statsdump_req_filter
>
> bridge/link.c | 4 +-
> bridge/mdb.c | 2 +-
> bridge/vlan.c | 10 +---
> include/libnetlink.h | 30 ++++++++--
> ip/ipaddress.c | 10 ++--
> ip/ipaddrlabel.c | 4 +-
> ip/iplink.c | 4 +-
> ip/iplink_xstats.c | 4 +-
> ip/ipmroute.c | 2 +-
> ip/ipnetconf.c | 3 +-
> ip/ipnetns.c | 2 +-
> ip/ipntable.c | 2 +-
> ip/iproute.c | 4 +-
> ip/iprule.c | 2 +-
> ip/iptoken.c | 2 +-
> ip/iptuntap.c | 2 +-
> ip/rtmon.c | 2 +-
> ip/tunnel.c | 2 +-
> lib/libnetlink.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++++---
> lib/ll_map.c | 2 +-
> misc/arpd.c | 2 +-
> misc/ifstat.c | 6 +-
> 22 files changed, 210 insertions(+), 55 deletions(-)
>
> --
> 2.11.0
>
^ permalink raw reply
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