* Re: [PATCH 10/16] net: phy: adin: add EEE translation layer for Clause 22
From: Ardelean, Alexandru @ 2019-08-06 6:47 UTC (permalink / raw)
To: andrew@lunn.ch
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805221150.GE25700@lunn.ch>
On Tue, 2019-08-06 at 00:11 +0200, Andrew Lunn wrote:
> [External]
>
> > +static int adin_cl22_to_adin_reg(int devad, u16 cl22_regnum)
> > +{
> > + struct clause22_mmd_map *m;
> > + int i;
> > +
> > + if (devad == MDIO_MMD_VEND1)
> > + return cl22_regnum;
> > +
> > + for (i = 0; i < ARRAY_SIZE(clause22_mmd_map); i++) {
> > + m = &clause22_mmd_map[i];
> > + if (m->devad == devad && m->cl22_regnum == cl22_regnum)
> > + return m->adin_regnum;
> > + }
> > +
> > + pr_err("No translation available for devad: %d reg: %04x\n",
> > + devad, cl22_regnum);
>
> phydev_err().
ack
>
> Andrew
^ permalink raw reply
* Re: [PATCH 06/16] net: phy: adin: support PHY mode converters
From: Ardelean, Alexandru @ 2019-08-06 6:47 UTC (permalink / raw)
To: andrew@lunn.ch
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805145105.GN24275@lunn.ch>
On Mon, 2019-08-05 at 16:51 +0200, Andrew Lunn wrote:
> [External]
>
> On Mon, Aug 05, 2019 at 07:54:43PM +0300, Alexandru Ardelean wrote:
> > Sometimes, the connection between a MAC and PHY is done via a
> > mode/interface converter. An example is a GMII-to-RGMII converter, which
> > would mean that the MAC operates in GMII mode while the PHY operates in
> > RGMII. In this case there is a discrepancy between what the MAC expects &
> > what the PHY expects and both need to be configured in their respective
> > modes.
> >
> > Sometimes, this converter is specified via a board/system configuration (in
> > the device-tree for example). But, other times it can be left unspecified.
> > The use of these converters is common in boards that have FPGA on them.
> >
> > This patch also adds support for a `adi,phy-mode-internal` property that
> > can be used in these (implicit convert) cases. The internal PHY mode will
> > be used to specify the correct register settings for the PHY.
> >
> > `fwnode_handle` is used, since this property may be specified via ACPI as
> > well in other setups, but testing has been done in DT context.
>
> Looking at the patch, you seems to assume phy-mode is what the MAC is
> using? That seems rather odd, given the name. It seems like a better
> solution would be to add a mac-mode, which the MAC uses to configure
> its side of the link. The MAC driver would then implement this
> property.
>
actually, that's a pretty good idea;
i guess i was narrow-minded when writing the driver, and got stuck on phy specifics, and forgot about the MAC-side;
[ i also catch these design elements when reviewing, but i also seem to miss them when writing stuff sometimes ]
thanks
> I don't see a need for this. phy-mode indicates what the PHY should
> use. End of story.
>
> Andrew
^ permalink raw reply
* Re: [PATCH 05/16] net: phy: adin: configure RGMII/RMII/MII modes on config
From: Ardelean, Alexandru @ 2019-08-06 6:43 UTC (permalink / raw)
To: andrew@lunn.ch
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805143935.GM24275@lunn.ch>
On Mon, 2019-08-05 at 16:39 +0200, Andrew Lunn wrote:
> [External]
>
> On Mon, Aug 05, 2019 at 07:54:42PM +0300, Alexandru Ardelean wrote:
> > The ADIN1300 chip supports RGMII, RMII & MII modes. Default (if
> > unconfigured) is RGMII.
> > This change adds support for configuring these modes via the device
> > registers.
> >
> > For RGMII with internal delays (modes RGMII_ID,RGMII_TXID, RGMII_RXID),
>
> It would be nice to add the missing space.
>
> > the default delay is 2 ns. This can be configurable and will be done in
> > a subsequent change.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/net/phy/adin.c | 79 +++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 78 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index 3dd9fe50f4c8..dbdb8f60741c 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -33,14 +33,91 @@
> > ADIN1300_INT_HW_IRQ_EN)
> > #define ADIN1300_INT_STATUS_REG 0x0019
> >
> > +#define ADIN1300_GE_RGMII_CFG_REG 0xff23
> > +#define ADIN1300_GE_RGMII_RXID_EN BIT(2)
> > +#define ADIN1300_GE_RGMII_TXID_EN BIT(1)
> > +#define ADIN1300_GE_RGMII_EN BIT(0)
> > +
> > +#define ADIN1300_GE_RMII_CFG_REG 0xff24
> > +#define ADIN1300_GE_RMII_EN BIT(0)
> > +
> > +static int adin_config_rgmii_mode(struct phy_device *phydev,
> > + phy_interface_t intf)
> > +{
> > + int reg;
> > +
> > + reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_RGMII_CFG_REG);
> > + if (reg < 0)
> > + return reg;
> > +
> > + if (!phy_interface_mode_is_rgmii(intf)) {
> > + reg &= ~ADIN1300_GE_RGMII_EN;
> > + goto write;
> > + }
> > +
> > + reg |= ADIN1300_GE_RGMII_EN;
> > +
> > + if (intf == PHY_INTERFACE_MODE_RGMII_ID ||
> > + intf == PHY_INTERFACE_MODE_RGMII_RXID) {
> > + reg |= ADIN1300_GE_RGMII_RXID_EN;
> > + } else {
> > + reg &= ~ADIN1300_GE_RGMII_RXID_EN;
> > + }
> > +
> > + if (intf == PHY_INTERFACE_MODE_RGMII_ID ||
> > + intf == PHY_INTERFACE_MODE_RGMII_TXID) {
> > + reg |= ADIN1300_GE_RGMII_TXID_EN;
> > + } else {
> > + reg &= ~ADIN1300_GE_RGMII_TXID_EN;
> > + }
>
> Nice. Often driver writers forget to clear the delay, they only set
> it. Not so here.
>
> However, is checkpatch happy with this? Each half of the if/else is a
> single statement, so the {} are not needed.
it did not complain;
this whole series is checkpatch friendly [with the version of checkpatch in net-next]
i think it complained about un-balanced if-block; something like:
```
if () {
} else
single-statement
```
but checkpatch is also a moving target;
so ¯\_(ツ)_/¯
>
> > +
> > +write:
> > + return phy_write_mmd(phydev, MDIO_MMD_VEND1,
> > + ADIN1300_GE_RGMII_CFG_REG, reg);
> > +}
> > +
> > +static int adin_config_rmii_mode(struct phy_device *phydev,
> > + phy_interface_t intf)
> > +{
> > + int reg;
> > +
> > + reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_RMII_CFG_REG);
> > + if (reg < 0)
> > + return reg;
> > +
> > + if (intf != PHY_INTERFACE_MODE_RMII) {
> > + reg &= ~ADIN1300_GE_RMII_EN;
> > + goto write;
>
> goto? Really?
yep;
personally, i used to not like it all that much up until a few years, but sometimes it feels it can help with creating
cleaner patches in certain contexts;
i'll re-spin without it;
>
> > + }
> > +
> > + reg |= ADIN1300_GE_RMII_EN;
> > +
> > +write:
> > + return phy_write_mmd(phydev, MDIO_MMD_VEND1,
> > + ADIN1300_GE_RMII_CFG_REG, reg);
> > +}
> > +
> > static int adin_config_init(struct phy_device *phydev)
> > {
> > - int rc;
> > + phy_interface_t interface, rc;
>
> genphy_config_init() does not return a phy_interface_t!
good point;
will check;
>
> >
> > rc = genphy_config_init(phydev);
> > if (rc < 0)
> > return rc;
> >
> > + interface = phydev->interface;
> > +
> > + rc = adin_config_rgmii_mode(phydev, interface);
> > + if (rc < 0)
> > + return rc;
> > +
> > + rc = adin_config_rmii_mode(phydev, interface);
> > + if (rc < 0)
> > + return rc;
> > +
> > + dev_info(&phydev->mdio.dev, "PHY is using mode '%s'\n",
> > + phy_modes(phydev->interface));
>
> phydev_dbg(), or not at all.
ack
>
> Andrew
^ permalink raw reply
* Re: [PATCH iproute2-next] rdma: Add driver QP type string
From: Gal Pressman @ 2019-08-06 6:41 UTC (permalink / raw)
To: David Ahern, Stephen Hemminger; +Cc: netdev, linux-rdma, Leon Romanovsky
In-Reply-To: <fd623a4e-d076-3eea-2d1e-7702812b0dfc@gmail.com>
On 05/08/2019 22:08, David Ahern wrote:
> On 8/4/19 2:07 AM, Gal Pressman wrote:
>> RDMA resource tracker now tracks driver QPs as well, add driver QP type
>> string to qp_types_to_str function.
>
> "now" means which kernel release? Leon: should this be in master or -next?
Now means the patch is merged to RDMA's for-rc branch (5.3).
^ permalink raw reply
* Re: [PATCH 04/16] net: phy: adin: add {write,read}_mmd hooks
From: Ardelean, Alexandru @ 2019-08-06 6:38 UTC (permalink / raw)
To: andrew@lunn.ch
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805142513.GK24275@lunn.ch>
On Mon, 2019-08-05 at 16:25 +0200, Andrew Lunn wrote:
> [External]
>
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index b75c723bda79..3dd9fe50f4c8 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -14,6 +14,9 @@
> > #define PHY_ID_ADIN1200 0x0283bc20
> > #define PHY_ID_ADIN1300 0x0283bc30
> >
> > +#define ADIN1300_MII_EXT_REG_PTR 0x10
> > +#define ADIN1300_MII_EXT_REG_DATA 0x11
> > +
> > #define ADIN1300_INT_MASK_REG 0x0018
>
> Please be consistent with registers. Either use 4 digits, or 2 digits.
ack;
>
> Andrew
^ permalink raw reply
* Re: [PATCH 03/16] net: phy: adin: add support for interrupts
From: Ardelean, Alexandru @ 2019-08-06 6:38 UTC (permalink / raw)
To: devicetree@vger.kernel.org, hkallweit1@gmail.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: f.fainelli@gmail.com, davem@davemloft.net, mark.rutland@arm.com,
robh+dt@kernel.org, andrew@lunn.ch
In-Reply-To: <4f539572-4c59-0450-fcd4-0bbc3eece9c8@gmail.com>
On Mon, 2019-08-05 at 23:02 +0200, Heiner Kallweit wrote:
> [External]
>
> On 05.08.2019 18:54, Alexandru Ardelean wrote:
> > This change adds support for enabling PHY interrupts that can be used by
> > the PHY framework to get signal for link/speed/auto-negotiation changes.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/net/phy/adin.c | 44 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 44 insertions(+)
> >
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index c100a0dd95cd..b75c723bda79 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -14,6 +14,22 @@
> > #define PHY_ID_ADIN1200 0x0283bc20
> > #define PHY_ID_ADIN1300 0x0283bc30
> >
> > +#define ADIN1300_INT_MASK_REG 0x0018
> > +#define ADIN1300_INT_MDIO_SYNC_EN BIT(9)
> > +#define ADIN1300_INT_ANEG_STAT_CHNG_EN BIT(8)
> > +#define ADIN1300_INT_ANEG_PAGE_RX_EN BIT(6)
> > +#define ADIN1300_INT_IDLE_ERR_CNT_EN BIT(5)
> > +#define ADIN1300_INT_MAC_FIFO_OU_EN BIT(4)
> > +#define ADIN1300_INT_RX_STAT_CHNG_EN BIT(3)
> > +#define ADIN1300_INT_LINK_STAT_CHNG_EN BIT(2)
> > +#define ADIN1300_INT_SPEED_CHNG_EN BIT(1)
> > +#define ADIN1300_INT_HW_IRQ_EN BIT(0)
> > +#define ADIN1300_INT_MASK_EN \
> > + (ADIN1300_INT_ANEG_STAT_CHNG_EN | ADIN1300_INT_ANEG_PAGE_RX_EN | \
> > + ADIN1300_INT_LINK_STAT_CHNG_EN | ADIN1300_INT_SPEED_CHNG_EN | \
> > + ADIN1300_INT_HW_IRQ_EN)
> > +#define ADIN1300_INT_STATUS_REG 0x0019
> > +
> > static int adin_config_init(struct phy_device *phydev)
> > {
> > int rc;
> > @@ -25,15 +41,40 @@ static int adin_config_init(struct phy_device *phydev)
> > return 0;
> > }
> >
> > +static int adin_phy_ack_intr(struct phy_device *phydev)
> > +{
> > + int ret;
> > +
> > + /* Clear pending interrupts. */
> > + ret = phy_read(phydev, ADIN1300_INT_STATUS_REG);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int adin_phy_config_intr(struct phy_device *phydev)
> > +{
> > + if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
> > + return phy_set_bits(phydev, ADIN1300_INT_MASK_REG,
> > + ADIN1300_INT_MASK_EN);
> > +
> > + return phy_clear_bits(phydev, ADIN1300_INT_MASK_REG,
> > + ADIN1300_INT_MASK_EN);
> > +}
> > +
> > static struct phy_driver adin_driver[] = {
> > {
> > .phy_id = PHY_ID_ADIN1200,
> > .name = "ADIN1200",
> > .phy_id_mask = 0xfffffff0,
> > .features = PHY_BASIC_FEATURES,
> > + .flags = PHY_HAS_INTERRUPT,
>
> This flag doesn't exist any longer. This indicates that you
> develop against an older kernel version. Please develop
> against net-next. Check up-to-date drivers like the one
> for Realtek PHY's for hints.
ack;
>
> > .config_init = adin_config_init,
> > .config_aneg = genphy_config_aneg,
> > .read_status = genphy_read_status,
> > + .ack_interrupt = adin_phy_ack_intr,
> > + .config_intr = adin_phy_config_intr,
> > .resume = genphy_resume,
> > .suspend = genphy_suspend,
> > },
> > @@ -42,9 +83,12 @@ static struct phy_driver adin_driver[] = {
> > .name = "ADIN1300",
> > .phy_id_mask = 0xfffffff0,
> > .features = PHY_GBIT_FEATURES,
> > + .flags = PHY_HAS_INTERRUPT,
> > .config_init = adin_config_init,
> > .config_aneg = genphy_config_aneg,
> > .read_status = genphy_read_status,
> > + .ack_interrupt = adin_phy_ack_intr,
> > + .config_intr = adin_phy_config_intr,
> > .resume = genphy_resume,
> > .suspend = genphy_suspend,
> > },
> >
^ permalink raw reply
* Re: [PATCH 03/16] net: phy: adin: add support for interrupts
From: Ardelean, Alexandru @ 2019-08-06 6:37 UTC (permalink / raw)
To: andrew@lunn.ch
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805142123.GJ24275@lunn.ch>
On Mon, 2019-08-05 at 16:21 +0200, Andrew Lunn wrote:
> [External]
>
> On Mon, Aug 05, 2019 at 07:54:40PM +0300, Alexandru Ardelean wrote:
> > This change adds support for enabling PHY interrupts that can be used by
> > the PHY framework to get signal for link/speed/auto-negotiation changes.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/net/phy/adin.c | 44 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 44 insertions(+)
> >
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index c100a0dd95cd..b75c723bda79 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -14,6 +14,22 @@
> > #define PHY_ID_ADIN1200 0x0283bc20
> > #define PHY_ID_ADIN1300 0x0283bc30
> >
> > +#define ADIN1300_INT_MASK_REG 0x0018
> > +#define ADIN1300_INT_MDIO_SYNC_EN BIT(9)
> > +#define ADIN1300_INT_ANEG_STAT_CHNG_EN BIT(8)
> > +#define ADIN1300_INT_ANEG_PAGE_RX_EN BIT(6)
> > +#define ADIN1300_INT_IDLE_ERR_CNT_EN BIT(5)
> > +#define ADIN1300_INT_MAC_FIFO_OU_EN BIT(4)
> > +#define ADIN1300_INT_RX_STAT_CHNG_EN BIT(3)
> > +#define ADIN1300_INT_LINK_STAT_CHNG_EN BIT(2)
> > +#define ADIN1300_INT_SPEED_CHNG_EN BIT(1)
> > +#define ADIN1300_INT_HW_IRQ_EN BIT(0)
> > +#define ADIN1300_INT_MASK_EN \
> > + (ADIN1300_INT_ANEG_STAT_CHNG_EN | ADIN1300_INT_ANEG_PAGE_RX_EN | \
> > + ADIN1300_INT_LINK_STAT_CHNG_EN | ADIN1300_INT_SPEED_CHNG_EN | \
> > + ADIN1300_INT_HW_IRQ_EN)
> > +#define ADIN1300_INT_STATUS_REG 0x0019
> > +
> > static int adin_config_init(struct phy_device *phydev)
> > {
> > int rc;
> > @@ -25,15 +41,40 @@ static int adin_config_init(struct phy_device *phydev)
> > return 0;
> > }
> >
> > +static int adin_phy_ack_intr(struct phy_device *phydev)
> > +{
> > + int ret;
> > +
> > + /* Clear pending interrupts. */
> > + ret = phy_read(phydev, ADIN1300_INT_STATUS_REG);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return 0;
>
> Please go through the whole driver and throw out all the needless
ack;
i'll re-visit;
>
> if (ret < 0)
> return ret;
>
> return 0;
>
> Thanks
> Andrew
^ permalink raw reply
* Re: [PATCH 01/16] net: phy: adin: add support for Analog Devices PHYs
From: Ardelean, Alexandru @ 2019-08-06 6:35 UTC (permalink / raw)
To: devicetree@vger.kernel.org, hkallweit1@gmail.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: f.fainelli@gmail.com, davem@davemloft.net, mark.rutland@arm.com,
robh+dt@kernel.org, andrew@lunn.ch
In-Reply-To: <206ec97f-3115-9a2c-91a0-e5f7aec4a39e@gmail.com>
On Mon, 2019-08-05 at 22:54 +0200, Heiner Kallweit wrote:
> [External]
>
> On 05.08.2019 18:54, Alexandru Ardelean wrote:
> > This change adds support for Analog Devices Industrial Ethernet PHYs.
> > Particularly the PHYs this driver adds support for:
> > * ADIN1200 - Robust, Industrial, Low Power 10/100 Ethernet PHY
> > * ADIN1300 - Robust, Industrial, Low Latency 10/100/1000 Gigabit
> > Ethernet PHY
> >
> > The 2 chips are pin & register compatible with one another. The main
> > difference being that ADIN1200 doesn't operate in gigabit mode.
> >
> > The chips can be operated by the Generic PHY driver as well via the
> > standard IEEE PHY registers (0x0000 - 0x000F) which are supported by the
> > kernel as well. This assumes that configuration of the PHY has been done
> > required.
> >
> > Configuration can also be done via registers, which will be implemented by
> > the driver in the next changes.
> >
> > Datasheets:
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1300.pdf
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1200.pdf
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > MAINTAINERS | 7 +++++
> > drivers/net/phy/Kconfig | 9 ++++++
> > drivers/net/phy/Makefile | 1 +
> > drivers/net/phy/adin.c | 59 ++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 76 insertions(+)
> > create mode 100644 drivers/net/phy/adin.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ee663e0e2f2e..faf5723610c8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -938,6 +938,13 @@ S: Supported
> > F: drivers/mux/adgs1408.c
> > F: Documentation/devicetree/bindings/mux/adi,adgs1408.txt
> >
> > +ANALOG DEVICES INC ADIN DRIVER
> > +M: Alexandru Ardelean <alexaundru.ardelean@analog.com>
> > +L: netdev@vger.kernel.org
> > +W: http://ez.analog.com/community/linux-device-drivers
> > +S: Supported
> > +F: drivers/net/phy/adin.c
> > +
> > ANALOG DEVICES INC ADIS DRIVER LIBRARY
> > M: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > S: Supported
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 206d8650ee7f..5966d3413676 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -257,6 +257,15 @@ config SFP
> > depends on HWMON || HWMON=n
> > select MDIO_I2C
> >
> > +config ADIN_PHY
> > + tristate "Analog Devices Industrial Ethernet PHYs"
> > + help
> > + Adds support for the Analog Devices Industrial Ethernet PHYs.
> > + Currently supports the:
> > + - ADIN1200 - Robust,Industrial, Low Power 10/100 Ethernet PHY
> > + - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
> > + Ethernet PHY
> > +
> > config AMD_PHY
> > tristate "AMD PHYs"
> > ---help---
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index ba07c27e4208..a03437e091f3 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -47,6 +47,7 @@ obj-$(CONFIG_SFP) += sfp.o
> > sfp-obj-$(CONFIG_SFP) += sfp-bus.o
> > obj-y += $(sfp-obj-y) $(sfp-obj-m)
> >
> > +obj-$(CONFIG_ADIN_PHY) += adin.o
> > obj-$(CONFIG_AMD_PHY) += amd.o
> > aquantia-objs += aquantia_main.o
> > ifdef CONFIG_HWMON
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > new file mode 100644
> > index 000000000000..6a610d4563c3
> > --- /dev/null
> > +++ b/drivers/net/phy/adin.c
> > @@ -0,0 +1,59 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/**
> > + * Driver for Analog Devices Industrial Ethernet PHYs
> > + *
> > + * Copyright 2019 Analog Devices Inc.
> > + */
> > +#include <linux/kernel.h>
> > +#include <linux/errno.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/mii.h>
> > +#include <linux/phy.h>
> > +
> > +#define PHY_ID_ADIN1200 0x0283bc20
> > +#define PHY_ID_ADIN1300 0x0283bc30
> > +
> > +static int adin_config_init(struct phy_device *phydev)
> > +{
> > + int rc;
> > +
> > + rc = genphy_config_init(phydev);
> > + if (rc < 0)
> > + return rc;
> > +
> > + return 0;
> > +}
> > +
> > +static struct phy_driver adin_driver[] = {
> > + {
> > + .phy_id = PHY_ID_ADIN1200,
>
> You could use PHY_ID_MATCH_MODEL here.
>
> > + .name = "ADIN1200",
> > + .phy_id_mask = 0xfffffff0,
> > + .features = PHY_BASIC_FEATURES,
>
> Setting features is deprecated, instead the get_features callback
> should be implemented if the default genphy_read_abilities needs
> to be extended / replaced. You say that the PHY's work with the
> genphy driver, so I suppose the default feature detection is ok
> in your case. Then you could simply remove setting "features".
ack;
thanks for the info
>
> > + .config_init = adin_config_init,
> > + .config_aneg = genphy_config_aneg,
> > + .read_status = genphy_read_status,
> > + },
> > + {
> > + .phy_id = PHY_ID_ADIN1300,
> > + .name = "ADIN1300",
> > + .phy_id_mask = 0xfffffff0,
> > + .features = PHY_GBIT_FEATURES,
> > + .config_init = adin_config_init,
> > + .config_aneg = genphy_config_aneg,
> > + .read_status = genphy_read_status,
> > + },
> > +};
> > +
> > +module_phy_driver(adin_driver);
> > +
> > +static struct mdio_device_id __maybe_unused adin_tbl[] = {
> > + { PHY_ID_ADIN1200, 0xfffffff0 },
> > + { PHY_ID_ADIN1300, 0xfffffff0 },
>
> PHY_ID_MATCH_MODEL could be used here too.
ack;
will take a look
>
> > + { }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(mdio, adin_tbl);
> > +MODULE_DESCRIPTION("Analog Devices Industrial Ethernet PHY driver");
> > +MODULE_LICENSE("GPL");
> >
^ permalink raw reply
* Re: [PATCH 01/16] net: phy: adin: add support for Analog Devices PHYs
From: Ardelean, Alexandru @ 2019-08-06 6:35 UTC (permalink / raw)
To: andrew@lunn.ch
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805151736.GQ24275@lunn.ch>
On Mon, 2019-08-05 at 17:17 +0200, Andrew Lunn wrote:
> [External]
>
> > +static struct phy_driver adin_driver[] = {
> > + {
> > + .phy_id = PHY_ID_ADIN1200,
> > + .name = "ADIN1200",
> > + .phy_id_mask = 0xfffffff0,
> > + .features = PHY_BASIC_FEATURES,
>
> Do you need this? If the device implements the registers correctly,
> phylib can determine this from the registers.
ack;
will take a look;
>
> > + .config_init = adin_config_init,
> > + .config_aneg = genphy_config_aneg,
> > + .read_status = genphy_read_status,
> > + },
> > + {
> > + .phy_id = PHY_ID_ADIN1300,
> > + .name = "ADIN1300",
> > + .phy_id_mask = 0xfffffff0,
> > + .features = PHY_GBIT_FEATURES,
>
> same here.
ack;
>
> > + .config_init = adin_config_init,
> > + .config_aneg = genphy_config_aneg,
> > + .read_status = genphy_read_status,
> > + },
> > +};
> > +
> > +module_phy_driver(adin_driver);
> > +
> > +static struct mdio_device_id __maybe_unused adin_tbl[] = {
> > + { PHY_ID_ADIN1200, 0xfffffff0 },
> > + { PHY_ID_ADIN1300, 0xfffffff0 },
>
> PHY_ID_MATCH_VENDOR().
ack;
>
> Andrew
^ permalink raw reply
* Re: [PATCH 01/16] net: phy: adin: add support for Analog Devices PHYs
From: Ardelean, Alexandru @ 2019-08-06 6:32 UTC (permalink / raw)
To: andrew@lunn.ch
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805141644.GH24275@lunn.ch>
On Mon, 2019-08-05 at 16:16 +0200, Andrew Lunn wrote:
> [External]
>
> > +static int adin_config_init(struct phy_device *phydev)
> > +{
> > + int rc;
> > +
> > + rc = genphy_config_init(phydev);
> > + if (rc < 0)
> > + return rc;
> > +
> > + return 0;
> > +}
>
> Why not just
>
> return genphy_config_init(phydev);
Because stuff will get added after this return statement in the next patches.
I thought maybe this would be a good idea to keep the git changes minimal, but I can do a direct return and update it in
the next patches when needed.
>
> Andrew
>
^ permalink raw reply
* Re: [PATCH 14/16] net: phy: adin: make sure down-speed auto-neg is enabled
From: Heiner Kallweit @ 2019-08-06 5:52 UTC (permalink / raw)
To: Alexandru Ardelean, netdev, devicetree, linux-kernel
Cc: davem, robh+dt, mark.rutland, f.fainelli, andrew
In-Reply-To: <20190805165453.3989-15-alexandru.ardelean@analog.com>
On 05.08.2019 18:54, Alexandru Ardelean wrote:
> Down-speed auto-negotiation may not always be enabled, in which case the
> PHY won't down-shift to 100 or 10 during auto-negotiation.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> drivers/net/phy/adin.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> index 86848444bd98..a1f3456a8504 100644
> --- a/drivers/net/phy/adin.c
> +++ b/drivers/net/phy/adin.c
> @@ -32,6 +32,13 @@
> #define ADIN1300_NRG_PD_TX_EN BIT(2)
> #define ADIN1300_NRG_PD_STATUS BIT(1)
>
> +#define ADIN1300_PHY_CTRL2 0x0016
> +#define ADIN1300_DOWNSPEED_AN_100_EN BIT(11)
> +#define ADIN1300_DOWNSPEED_AN_10_EN BIT(10)
> +#define ADIN1300_GROUP_MDIO_EN BIT(6)
> +#define ADIN1300_DOWNSPEEDS_EN \
> + (ADIN1300_DOWNSPEED_AN_100_EN | ADIN1300_DOWNSPEED_AN_10_EN)
> +
> #define ADIN1300_INT_MASK_REG 0x0018
> #define ADIN1300_INT_MDIO_SYNC_EN BIT(9)
> #define ADIN1300_INT_ANEG_STAT_CHNG_EN BIT(8)
> @@ -425,6 +432,22 @@ static int adin_config_mdix(struct phy_device *phydev)
> return phy_write(phydev, ADIN1300_PHY_CTRL1, reg);
> }
>
> +static int adin_config_downspeeds(struct phy_device *phydev)
> +{
> + int reg;
> +
> + reg = phy_read(phydev, ADIN1300_PHY_CTRL2);
> + if (reg < 0)
> + return reg;
> +
> + if ((reg & ADIN1300_DOWNSPEEDS_EN) == ADIN1300_DOWNSPEEDS_EN)
> + return 0;
> +
> + reg |= ADIN1300_DOWNSPEEDS_EN;
> +
> + return phy_write(phydev, ADIN1300_PHY_CTRL2, reg);
Using phy_set_bits() would be easier.
> +}
> +
> static int adin_config_aneg(struct phy_device *phydev)
> {
> int ret;
> @@ -433,6 +456,10 @@ static int adin_config_aneg(struct phy_device *phydev)
> if (ret)
> return ret;
>
> + ret = adin_config_downspeeds(phydev);
> + if (ret < 0)
> + return ret;
> +
> return genphy_config_aneg(phydev);
> }
>
>
^ permalink raw reply
* Re: [RFC PATCH 1/2] dt-bindings: net: macb: Add new property for PS SGMII only
From: Harini Katakam @ 2019-08-06 5:47 UTC (permalink / raw)
To: Andrew Lunn
Cc: Harini Katakam, Nicolas Ferre, David Miller, Claudiu Beznea,
Rob Herring, Mark Rutland, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Michal Simek,
devicetree@vger.kernel.org
In-Reply-To: <20190805171639.GV24275@lunn.ch>
Hi Andrew,
On Mon, Aug 5, 2019 at 10:47 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Even with the use of this interrupt, the link status actions (link print and
> > netif ops) will still be required. And also the need for macb_open to
> > proceed without phydev. Could you please let me know if that is acceptable
> > to patch or if there's a cleaner way to
> > report this link status?
>
> It sounds like you need to convert to phylink, so you get full sfp
> support. phylib does not handle hotplug of PHYs.
>
> Please look at the comments Russell gave the last time this was
> attempted.
Yes, I looked at the comments from Russell and wasn't sure if this
case qualified for phylink.
Regards,
Harini
^ permalink raw reply
* RE: hv_netvsc: WARNING: suspicious RCU usage?
From: Dexuan Cui @ 2019-08-06 5:25 UTC (permalink / raw)
To: netdev@vger.kernel.org, Yidong Ren, Haiyang Zhang,
Stephen Hemminger, David S. Miller
Cc: linux-hyperv@vger.kernel.org
In-Reply-To: <PU1P153MB0169B7073A4865D50AED9EE5BFDA0@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Dexuan Cui
> Sent: Monday, August 5, 2019 4:56 PM
> The warning is caused by the rcu_dereference_rtnl() :
>
> 1239 static void netvsc_get_stats64(struct net_device *net,
> 1240 struct rtnl_link_stats64 *t)
> 1241 {
> 1242 struct net_device_context *ndev_ctx = netdev_priv(net);
> 1243 struct netvsc_device *nvdev =
> rcu_dereference_rtnl(ndev_ctx->nvdev);
>
> I think here netvsc_get_stats64() neither holds rcu_read_lock() nor RTNL
>
> IMO it should call rcu_read_lock()/unlock(), or get RTNL to fix the warning?
I just posted a patch on behalf of Stephen:
[PATCH net] hv_netvsc: Fix a warning of suspicious RCU usage
Thanks,
-- Dexuan
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: dump nested registers
From: Michal Kubecek @ 2019-08-06 5:20 UTC (permalink / raw)
To: netdev; +Cc: Vivien Didelot, f.fainelli, andrew, davem, linville, cphealy
In-Reply-To: <20190805105216.GB31482@t480s.localdomain>
On Mon, Aug 05, 2019 at 10:52:16AM -0400, Vivien Didelot wrote:
> Hi Michal!
>
> On Mon, 5 Aug 2019 10:04:48 +0200, Michal Kubecek <mkubecek@suse.cz> wrote:
> > On Fri, Aug 02, 2019 at 03:34:54PM -0400, Vivien Didelot wrote:
> > > Usually kernel drivers set the regs->len value to the same length as
> > > info->regdump_len, which was used for the allocation. In case where
> > > regs->len is smaller than the allocated info->regdump_len length,
> > > we may assume that the dump contains a nested set of registers.
> > >
> > > This becomes handy for kernel drivers to expose registers of an
> > > underlying network conduit unfortunately not exposed to userspace,
> > > as found in network switching equipment for example.
> > >
> > > This patch adds support for recursing into the dump operation if there
> > > is enough room for a nested ethtool_drvinfo structure containing a
> > > valid driver name, followed by a ethtool_regs structure like this:
> > >
> > > 0 regs->len info->regdump_len
> > > v v v
> > > +--------------+-----------------+--------------+-- - --+
> > > | ethtool_regs | ethtool_drvinfo | ethtool_regs | |
> > > +--------------+-----------------+--------------+-- - --+
> > >
> > > Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
> > > ---
> >
> > I'm not sure about this approach. If these additional objects with their
> > own registers are represented by a network device, we can query their
> > registers directly. If they are not (which, IIUC, is the case in your
> > use case), we should use an appropriate interface. AFAIK the CPU ports
> > are already represented in devlink, shouldn't devlink be also used to
> > query their registers?
>
> Yet another interface wasn't that much appropriate for DSA, making the
> stack unnecessarily complex.
AFAICS, there is already devlink support in dsa and CPU ports are
presented as devlink ports. So it wouldn't be a completely new
interface.
> In fact we are already glueing the statistics of the CPU port into the
> master's ethtool ops (both physical ports are wired together).
The ethtool statistics (in general) are one big mess, I don't think it's
an example worth following; rather one showing us what to avoid.
> Adding support for nested registers dump in ethtool makes it simple to
> (pretty) dump CPU port's registers without too much userspace
> addition.
It is indeed convenient for pretty print - but very hard to use for any
automated processing. My point is that CPU port is not represented by
a network device but it is already represented by a devlink port so that
it makes much more sense to tie its register dump to that object than to
add add it to register dump of port's master.
In the future, I would like to transform the ethtool register dump from
current opaque block of data to an actual dump of registers. It is
unfortunate that drivers are already mixing information specific to
a network device with information common for the whole physical device.
Adding more data which is actually related to a different object would
only make things worse.
Michal Kubecek
^ permalink raw reply
* [PATCH net] hv_netvsc: Fix a warning of suspicious RCU usage
From: Dexuan Cui @ 2019-08-06 5:17 UTC (permalink / raw)
To: netdev@vger.kernel.org, David S. Miller, Haiyang Zhang,
Stephen Hemminger
Cc: sashal@kernel.org, KY Srinivasan, Michael Kelley,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, vkuznets,
marcelo.cerri@canonical.com
This fixes a warning of "suspicious rcu_dereference_check() usage"
when nload runs.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 44 +++++++++++++++++++--------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f9209594624b5..25502d335b94f 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1236,25 +1236,10 @@ static void netvsc_get_pcpu_stats(struct net_device *net,
}
}
-static void netvsc_get_stats64(struct net_device *net,
- struct rtnl_link_stats64 *t)
+static void netvsc_get_per_chan_stats(struct netvsc_device *nvdev,
+ struct rtnl_link_stats64 *t)
{
- struct net_device_context *ndev_ctx = netdev_priv(net);
- struct netvsc_device *nvdev = rcu_dereference_rtnl(ndev_ctx->nvdev);
- struct netvsc_vf_pcpu_stats vf_tot;
- int i;
-
- if (!nvdev)
- return;
-
- netdev_stats_to_stats64(t, &net->stats);
-
- netvsc_get_vf_stats(net, &vf_tot);
- t->rx_packets += vf_tot.rx_packets;
- t->tx_packets += vf_tot.tx_packets;
- t->rx_bytes += vf_tot.rx_bytes;
- t->tx_bytes += vf_tot.tx_bytes;
- t->tx_dropped += vf_tot.tx_dropped;
+ u32 i;
for (i = 0; i < nvdev->num_chn; i++) {
const struct netvsc_channel *nvchan = &nvdev->chan_table[i];
@@ -1286,6 +1271,29 @@ static void netvsc_get_stats64(struct net_device *net,
}
}
+static void netvsc_get_stats64(struct net_device *net,
+ struct rtnl_link_stats64 *t)
+{
+ struct net_device_context *ndev_ctx = netdev_priv(net);
+ struct netvsc_device *nvdev;
+ struct netvsc_vf_pcpu_stats vf_tot;
+
+ netdev_stats_to_stats64(t, &net->stats);
+
+ netvsc_get_vf_stats(net, &vf_tot);
+ t->rx_packets += vf_tot.rx_packets;
+ t->tx_packets += vf_tot.tx_packets;
+ t->rx_bytes += vf_tot.rx_bytes;
+ t->tx_bytes += vf_tot.tx_bytes;
+ t->tx_dropped += vf_tot.tx_dropped;
+
+ rcu_read_lock();
+ nvdev = rcu_dereference(ndev_ctx->nvdev);
+ if (nvdev)
+ netvsc_get_per_chan_stats(nvdev, t);
+ rcu_read_unlock();
+}
+
static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
{
struct net_device_context *ndc = netdev_priv(ndev);
^ permalink raw reply related
* Re: [PATCH net-next 06/10] net: stmmac: Implement RSS and enable it in XGMAC core
From: Jakub Kicinski @ 2019-08-06 4:46 UTC (permalink / raw)
To: Jose Abreu
Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue,
David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <e70981c111ac857a0bac77750bd69a3383d99ee0.1565027782.git.joabreu@synopsys.com>
On Mon, 5 Aug 2019 20:01:19 +0200, Jose Abreu wrote:
> Implement the RSS functionality and add the corresponding callbacks in
> XGMAC core.
>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> ---
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: netdev@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
> index c4c45402b8f8..9ff9d9ac1a50 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
> @@ -254,6 +254,34 @@ static void dwxgmac2_clear(struct dma_desc *p)
> p->des3 = 0;
> }
>
> +static int dwxgmac2_get_rx_hash(struct dma_desc *p, u32 *hash,
> + enum pkt_hash_types *type)
> +{
> + unsigned int rdes3 = le32_to_cpu(p->des3);
> + u32 ptype;
> +
> + if (rdes3 & XGMAC_RDES3_RSV) {
> + ptype = (rdes3 & XGMAC_RDES3_L34T) >> XGMAC_RDES3_L34T_SHIFT;
> +
> + switch (ptype) {
> + case 0x1:
> + case 0x2:
> + case 0x9:
> + case 0xA:
nit: it'd be nice to have defines for these constants
> + *type = PKT_HASH_TYPE_L4;
> + break;
> + default:
> + *type = PKT_HASH_TYPE_L3;
> + break;
> + }
> +
> + *hash = le32_to_cpu(p->des1);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> const struct stmmac_desc_ops dwxgmac210_desc_ops = {
> .tx_status = dwxgmac2_get_tx_status,
> .rx_status = dwxgmac2_get_rx_status,
> @@ -4182,7 +4208,7 @@ int stmmac_dvr_probe(struct device *device,
> struct net_device *ndev = NULL;
> struct stmmac_priv *priv;
> u32 queue, maxq;
> - int ret = 0;
> + int i, ret = 0;
>
> ndev = devm_alloc_etherdev_mqs(device, sizeof(struct stmmac_priv),
> MTL_MAX_TX_QUEUES, MTL_MAX_RX_QUEUES);
> @@ -4290,6 +4316,14 @@ int stmmac_dvr_probe(struct device *device,
> #endif
> priv->msg_enable = netif_msg_init(debug, default_msg_level);
>
> + /* Initialize RSS */
> + netdev_rss_key_fill(priv->rss.key, sizeof(priv->rss.key));
> + for (i = 0; i < ARRAY_SIZE(priv->rss.table); i++)
> + priv->rss.table[i] = i % priv->plat->rx_queues_to_use;
ethtool_rxfh_indir_default() ?
> + if (priv->dma_cap.rssen && priv->plat->rss_en)
> + ndev->features |= NETIF_F_RXHASH;
> +
> /* MTU range: 46 - hw-specific max */
> ndev->min_mtu = ETH_ZLEN - ETH_HLEN;
> if ((priv->plat->enh_desc) || (priv->synopsys_id >= DWMAC_CORE_4_00))
^ permalink raw reply
* [RFC PATCH] kbuild: re-implement detection of CONFIG options leaked to user-space
From: Masahiro Yamada @ 2019-08-06 4:37 UTC (permalink / raw)
To: linux-kbuild
Cc: Arnd Bergmann, Sam Ravnborg, Masahiro Yamada, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, bpf,
linux-kernel, netdev
scripts/headers_check.pl can detect references to CONFIG options in
exported headers, but it has been disabled for more than a decade.
Reverting commit 7e3fa5614117 ("kbuild: drop check for CONFIG_ in
headers_check") would emit the following warnings for headers_check
on x86:
usr/include/mtd/ubi-user.h:283: leaks CONFIG_MTD_UBI_BEB_LIMIT to userspace where it is not valid
usr/include/linux/elfcore.h:62: leaks CONFIG_BINFMT_ELF_FDPIC to userspace where it is not valid
usr/include/linux/atmdev.h:104: leaks CONFIG_COMPAT to userspace where it is not valid
usr/include/linux/raw.h:17: leaks CONFIG_MAX_RAW_DEVS to userspace where it is not valid
usr/include/linux/pktcdvd.h:37: leaks CONFIG_CDROM_PKTCDVD_WCACHE to userspace where it is not valid
usr/include/linux/videodev2.h:2465: leaks CONFIG_VIDEO_ADV_DEBUG to userspace where it is not valid
usr/include/linux/bpf.h:249: leaks CONFIG_EFFICIENT_UNALIGNED_ACCESS to userspace where it is not valid
usr/include/linux/bpf.h:819: leaks CONFIG_CGROUP_NET_CLASSID to userspace where it is not valid
usr/include/linux/bpf.h:1011: leaks CONFIG_IP_ROUTE_CLASSID to userspace where it is not valid
usr/include/linux/bpf.h:1742: leaks CONFIG_BPF_KPROBE_OVERRIDE to userspace where it is not valid
usr/include/linux/bpf.h:1747: leaks CONFIG_FUNCTION_ERROR_INJECTION to userspace where it is not valid
usr/include/linux/bpf.h:1936: leaks CONFIG_XFRM to userspace where it is not valid
usr/include/linux/bpf.h:2184: leaks CONFIG_BPF_LIRC_MODE2 to userspace where it is not valid
usr/include/linux/bpf.h:2210: leaks CONFIG_BPF_LIRC_MODE2 to userspace where it is not valid
usr/include/linux/bpf.h:2227: leaks CONFIG_SOCK_CGROUP_DATA to userspace where it is not valid
usr/include/linux/bpf.h:2311: leaks CONFIG_NET to userspace where it is not valid
usr/include/linux/bpf.h:2348: leaks CONFIG_NET to userspace where it is not valid
usr/include/linux/bpf.h:2422: leaks CONFIG_BPF_LIRC_MODE2 to userspace where it is not valid
usr/include/linux/bpf.h:2528: leaks CONFIG_NET to userspace where it is not valid
usr/include/linux/eventpoll.h:82: leaks CONFIG_PM_SLEEP to userspace where it is not valid
usr/include/linux/hw_breakpoint.h:27: leaks CONFIG_HAVE_MIXED_BREAKPOINTS_REGS to userspace where it is not valid
usr/include/linux/cm4000_cs.h:26: leaks CONFIG_COMPAT to userspace where it is not valid
usr/include/linux/pkt_cls.h:301: leaks CONFIG_NET_CLS_ACT to userspace where it is not valid
usr/include/asm-generic/unistd.h:651: leaks CONFIG_MMU to userspace where it is not valid
usr/include/asm-generic/fcntl.h:119: leaks CONFIG_64BIT to userspace where it is not valid
usr/include/asm-generic/bitsperlong.h:9: leaks CONFIG_64BIT to userspace where it is not valid
usr/include/asm/e820.h:14: leaks CONFIG_NODES_SHIFT to userspace where it is not valid
usr/include/asm/e820.h:39: leaks CONFIG_X86_PMEM_LEGACY to userspace where it is not valid
usr/include/asm/e820.h:49: leaks CONFIG_INTEL_TXT to userspace where it is not valid
usr/include/asm/mman.h:7: leaks CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS to userspace where it is not valid
usr/include/asm/auxvec.h:14: leaks CONFIG_IA32_EMULATION to userspace where it is not valid
Most of these are false positives because scripts/headers_check.pl
parses comment lines.
It is also false negative. arch/x86/include/uapi/asm/auxvec.h contains
CONFIG_IA32_EMULATION and CONFIG_X86_64, but the only former is reported.
It would be possible to fix scripts/headers_check.pl, of course.
However, we already have some duplicated checks between headers_check
and CONFIG_UAPI_HEADER_TEST. At this moment of time, there are still
dozens of headers excluded from the header test (usr/include/Makefile),
but we might be able to remove headers_check when the time comes.
I re-implemented it in scripts/headers_install.sh by using sed because
the most of code in scripts/headers_install.sh is written is sed.
This patch works like this:
[1] Run scripts/unifdef first because we need to drop the code
surrounded by #ifdef __KERNEL__ ... #endif
[2] Remove all C style comments. The sed code is somewhat complicated
since we need to deal with both single and multi line comments.
Precisely speaking, a comment block is replaced with a space just
in case.
CONFIG_FOO/* this is a comment */CONFIG_BAR
should be converted into:
CONFIG_FOO CONFIG_BAR
instead of:
CONFIG_FOOCONFIG_BAR
[3] Match CONFIG_... pattern. It correctly matches to all CONFIG options
that appear in a single line.
After this commit, you will see the following warnings, all of which
are real ones.
warning: include/uapi/linux/elfcore.h: leaks CONFIG_BINFMT_ELF_FDPIC to user-space
warning: include/uapi/linux/atmdev.h: leaks CONFIG_COMPAT to user-space
warning: include/uapi/linux/raw.h: leaks CONFIG_MAX_RAW_DEVS to user-space
warning: include/uapi/linux/pktcdvd.h: leaks CONFIG_CDROM_PKTCDVD_WCACHE to user-space
warning: include/uapi/linux/eventpoll.h: leaks CONFIG_PM_SLEEP to user-space
warning: include/uapi/linux/hw_breakpoint.h: leaks CONFIG_HAVE_MIXED_BREAKPOINTS_REGS to user-space
warning: include/uapi/asm-generic/fcntl.h: leaks CONFIG_64BIT to user-space
warning: arch/x86/include/uapi/asm/mman.h: leaks CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS to user-space
warning: arch/x86/include/uapi/asm/auxvec.h: leaks CONFIG_IA32_EMULATION to user-space
warning: arch/x86/include/uapi/asm/auxvec.h: leaks CONFIG_X86_64 to user-space
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
I was playing with sed yesterday, but the resulted code might be unreadable.
Sed scripts tend to be somewhat unreadable.
I just wondered which language is appropriate for this?
Maybe perl, or what else? I am not good at perl, though.
Maybe, it will be better to fix existing warnings
before enabling this check.
If somebody takes a closer look at them, that would be great.
| 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
--git a/scripts/headers_install.sh b/scripts/headers_install.sh
index bbaf29386995..73d95e457090 100755
--- a/scripts/headers_install.sh
+++ b/scripts/headers_install.sh
@@ -41,5 +41,34 @@ sed -E -e '
scripts/unifdef -U__KERNEL__ -D__EXPORTED_HEADERS__ $TMPFILE > $OUTFILE
[ $? -gt 1 ] && exit 1
+# Remove /* ... */ style comments, and find CONFIG_ references in code
+configs=$(sed -e '
+:comment
+ s:/\*[^*][^*]*:/*:
+ s:/\*\*\**\([^/]\):/*\1:
+ t comment
+ s:/\*\*/: :
+ t comment
+ /\/\*/! b check
+ N
+ b comment
+:print
+ P
+ D
+:check
+ s:^[^[:alnum:]_][^[:alnum:]_]*::
+ t check
+ s:^\(CONFIG_[[:alnum:]_]*\):\1\n:
+ t print
+ s:^[[:alnum:]_][[:alnum:]_]*::
+ t check
+ d
+' $OUTFILE)
+
+for c in $configs
+do
+ echo "warning: $INFILE: leaks $c to user-space" >&2
+done
+
rm -f $TMPFILE
trap - EXIT
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 03/10] net: stmmac: Fix issues when number of Queues >= 4
From: Jakub Kicinski @ 2019-08-06 4:34 UTC (permalink / raw)
To: Jose Abreu
Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue,
David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <5e95bb1761f9438361f198d744640685a34790ea.1565027782.git.joabreu@synopsys.com>
On Mon, 5 Aug 2019 20:01:16 +0200, Jose Abreu wrote:
> When queues >= 4 we use different registers but we were not subtracting
> the offset of 4. Fix this.
>
> Found out by Coverity.
>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Should this be a bug fix for the net tree?
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/2] selftests/bpf: more loop tests
From: Yonghong Song @ 2019-08-06 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, davem@davemloft.net
Cc: daniel@iogearbox.net, netdev@vger.kernel.org, bpf@vger.kernel.org,
Kernel Team
In-Reply-To: <20190806021744.2953168-1-ast@kernel.org>
On 8/5/19 7:17 PM, Alexei Starovoitov wrote:
> Add two bounded loop tests.
>
> v1-v2: addressed feedback from Yonghong.
>
> Alexei Starovoitov (2):
> selftests/bpf: add loop test 4
> selftests/bpf: add loop test 5
Looks good to me. Ack for the whole series.
Acked-by: Yonghong Song <yhs@fb.com>
>
> .../bpf/prog_tests/bpf_verif_scale.c | 2 ++
> tools/testing/selftests/bpf/progs/loop4.c | 18 +++++++++++
> tools/testing/selftests/bpf/progs/loop5.c | 32 +++++++++++++++++++
> 3 files changed, 52 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/loop4.c
> create mode 100644 tools/testing/selftests/bpf/progs/loop5.c
>
^ permalink raw reply
* Re: [net-next v2 0/8][pull request] 40GbE Intel Wired LAN Driver Updates 2019-08-05
From: Jakub Kicinski @ 2019-08-06 3:46 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, netdev, nhorman, sassmann
In-Reply-To: <20190805185459.12846-1-jeffrey.t.kirsher@intel.com>
On Mon, 5 Aug 2019 11:54:51 -0700, Jeff Kirsher wrote:
> This series contains updates to i40e driver only.
>
> Dmitrii adds missing statistic counters for VEB and VEB TC's.
>
> Slawomir adds support for logging the "Disable Firmware LLDP" flag
> option and its current status.
>
> Jake fixes an issue where VF's being notified of their link status
> before their queues are enabled which was causing issues. So always
> report link status down when the VF queues are not enabled. Also adds
> future proofing when statistics are added or removed by adding checks to
> ensure the data pointer for the strings lines up with the expected
> statistics count.
>
> Czeslaw fixes the advertised mode reported in ethtool for FEC, where the
> "None BaseR RS" was always being displayed no matter what the mode it
> was in. Also added logging information when the PF is entering or
> leaving "allmulti" (or promiscuous) mode. Fixed up the logging logic
> for VF's when leaving multicast mode to not include unicast as well.
I can understand patch 2 may be useful for troubleshooting since FW
agent is involved. But I can't really say the same for patch 6 :S
If those entered allmutli/left allmutli messages were of value core
should print them instead..
But anyway, that's not a big deal, looks reasonable.
^ permalink raw reply
* Re: [PATCH net-next v6 0/6] flow_offload: add indr-block in nf_table_offload
From: Jakub Kicinski @ 2019-08-06 3:37 UTC (permalink / raw)
To: wenxu; +Cc: jiri, netfilter-devel, netdev
In-Reply-To: <1564925041-23530-1-git-send-email-wenxu@ucloud.cn>
On Sun, 4 Aug 2019 21:23:55 +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> This series patch make nftables offload support the vlan and
> tunnel device offload through indr-block architecture.
>
> The first four patches mv tc indr block to flow offload and
> rename to flow-indr-block.
> Because the new flow-indr-block can't get the tcf_block
> directly. The fifth patch provide a callback list to get
> flow_block of each subsystem immediately when the device
> register and contain a block.
> The last patch make nf_tables_offload support flow-indr-block.
Looks good to me, thanks for the changes!
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* Re: [net-next 0/8][pull request] 100GbE Intel Wired LAN Driver Updates 2019-08-04
From: Jakub Kicinski @ 2019-08-06 3:17 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, netdev, nhorman, sassmann
In-Reply-To: <20190804115926.31944-1-jeffrey.t.kirsher@intel.com>
On Sun, 4 Aug 2019 04:59:18 -0700, Jeff Kirsher wrote:
> This series contains more updates to fm10k from Jake Keller.
>
> Jake removes the unnecessary initialization of some variables to help
> resolve static code checker warnings. Explicitly return success during
> resume, since the value of 'err' is always success. Fixed a issue with
> incrementing a void pointer, which can produce undefined behavior. Used
> the __always_unused macro for function templates that are passed as
> parameters in functions, but are not used. Simplified the code by
> removing an unnecessary macro in determining the value of NON_Q_VECTORS.
> Fixed an issue, using bitwise operations to prevent the low address
> overwriting the high portion of the address.
Looks good. AFAIK void pointer arithmetic is not uncommon in the
kernel, but shouldn't hurt to fix it.
Do you guys have any plans to fix W=1 C=1 build for Intel drivers?
That'd be very nice :)
^ permalink raw reply
* Re: [PATCH v1 1/3] net: hisilicon: make hip04_tx_reclaim non-reentrant
From: Jakub Kicinski @ 2019-08-06 3:05 UTC (permalink / raw)
To: Jiangfeng Xiao
Cc: davem, yisen.zhuang, salil.mehta, netdev, linux-kernel, leeyou.li,
xiaowei774, nixiaoming
In-Reply-To: <c150d41b-6f0e-ad49-e8c2-00896fc9cbe4@huawei.com>
On Tue, 6 Aug 2019 10:00:52 +0800, Jiangfeng Xiao wrote:
> If hip04_tx_reclaim is interrupted while it is running, and then
> __irq_svc->gic_handle_irq->hip04_mac_interrupt->__napi_schedule->hip04_rx_poll->hip04_tx_reclaim
Ah right, obviously you can't do stuff after napi_complete_done(),
that makes sense.
Series looks reasonable.
^ permalink raw reply
* [PATCH 2/2] cxgb4: smt: Use normal int for refcount
From: Chuhong Yuan @ 2019-08-06 2:58 UTC (permalink / raw)
Cc: Vishal Kulkarni, David S . Miller, netdev, linux-kernel,
Chuhong Yuan
All refcount operations are protected by spinlocks now.
Then the atomic counter can be replaced by a normal int.
This patch depends on PATCH 1/2.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/chelsio/cxgb4/smt.c | 14 +++++++-------
drivers/net/ethernet/chelsio/cxgb4/smt.h | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/smt.c b/drivers/net/ethernet/chelsio/cxgb4/smt.c
index d6e84c8b5554..01c65d13fc0e 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/smt.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/smt.c
@@ -57,7 +57,7 @@ struct smt_data *t4_init_smt(void)
s->smtab[i].state = SMT_STATE_UNUSED;
memset(&s->smtab[i].src_mac, 0, ETH_ALEN);
spin_lock_init(&s->smtab[i].lock);
- atomic_set(&s->smtab[i].refcnt, 0);
+ s->smtab[i].refcnt = 0;
}
return s;
}
@@ -68,7 +68,7 @@ static struct smt_entry *find_or_alloc_smte(struct smt_data *s, u8 *smac)
struct smt_entry *e, *end;
for (e = &s->smtab[0], end = &s->smtab[s->smt_size]; e != end; ++e) {
- if (atomic_read(&e->refcnt) == 0) {
+ if (e->refcnt == 0) {
if (!first_free)
first_free = e;
} else {
@@ -97,7 +97,7 @@ static struct smt_entry *find_or_alloc_smte(struct smt_data *s, u8 *smac)
static void t4_smte_free(struct smt_entry *e)
{
- if (atomic_read(&e->refcnt) == 0) { /* hasn't been recycled */
+ if (e->refcnt == 0) { /* hasn't been recycled */
e->state = SMT_STATE_UNUSED;
}
}
@@ -110,7 +110,7 @@ static void t4_smte_free(struct smt_entry *e)
void cxgb4_smt_release(struct smt_entry *e)
{
spin_lock_bh(&e->lock);
- if (atomic_dec_and_test(&e->refcnt))
+ if ((--e->refcnt) == 0)
t4_smte_free(e);
spin_unlock_bh(&e->lock);
}
@@ -215,14 +215,14 @@ static struct smt_entry *t4_smt_alloc_switching(struct adapter *adap, u16 pfvf,
e = find_or_alloc_smte(s, smac);
if (e) {
spin_lock(&e->lock);
- if (!atomic_read(&e->refcnt)) {
- atomic_set(&e->refcnt, 1);
+ if (!e->refcnt) {
+ e->refcnt = 1;
e->state = SMT_STATE_SWITCHING;
e->pfvf = pfvf;
memcpy(e->src_mac, smac, ETH_ALEN);
write_smt_entry(adap, e);
} else {
- atomic_inc(&e->refcnt);
+ ++e->refcnt;
}
spin_unlock(&e->lock);
}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/smt.h b/drivers/net/ethernet/chelsio/cxgb4/smt.h
index d6c2cc271398..1268d6e93a47 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/smt.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/smt.h
@@ -59,7 +59,7 @@ struct smt_entry {
u16 idx;
u16 pfvf;
u8 src_mac[ETH_ALEN];
- atomic_t refcnt;
+ int refcnt;
spinlock_t lock; /* protect smt entry add,removal */
};
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] cxgb4: smt: Add lock for atomic_dec_and_test
From: Chuhong Yuan @ 2019-08-06 2:58 UTC (permalink / raw)
Cc: Vishal Kulkarni, David S . Miller, netdev, linux-kernel,
Chuhong Yuan
The atomic_dec_and_test() is not safe because it is
outside of locks.
Move the locks of t4_smte_free() to its caller,
cxgb4_smt_release() to protect the atomic decrement.
Fixes: 3bdb376e6944 ("cxgb4: introduce SMT ops to prepare for SMAC rewrite support")
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/chelsio/cxgb4/smt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/smt.c b/drivers/net/ethernet/chelsio/cxgb4/smt.c
index eaf1fb74689c..d6e84c8b5554 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/smt.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/smt.c
@@ -97,11 +97,9 @@ static struct smt_entry *find_or_alloc_smte(struct smt_data *s, u8 *smac)
static void t4_smte_free(struct smt_entry *e)
{
- spin_lock_bh(&e->lock);
if (atomic_read(&e->refcnt) == 0) { /* hasn't been recycled */
e->state = SMT_STATE_UNUSED;
}
- spin_unlock_bh(&e->lock);
}
/**
@@ -111,8 +109,10 @@ static void t4_smte_free(struct smt_entry *e)
*/
void cxgb4_smt_release(struct smt_entry *e)
{
+ spin_lock_bh(&e->lock);
if (atomic_dec_and_test(&e->refcnt))
t4_smte_free(e);
+ spin_unlock_bh(&e->lock);
}
EXPORT_SYMBOL(cxgb4_smt_release);
--
2.20.1
^ 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