* Re: [PATCH net-next 0/4] net: ethernet: ti: cpsw: allow vlan h/w timestamping
From: David Miller @ 2018-11-14 0:30 UTC (permalink / raw)
To: ivan.khoronzhuk; +Cc: grygorii.strashko, linux-omap, netdev, linux-kernel
In-Reply-To: <20181112140023.12407-1-ivan.khoronzhuk@linaro.org>
From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Mon, 12 Nov 2018 16:00:19 +0200
> The patchset adds several improvements and allows vlan h/w ts.
>
> Based on net-next/master
Series applied, thanks.
^ permalink raw reply
* [PATCH v2 24/25] ARM: davinci: mityomapl138: use device properties for at24 eeprom
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-mityomapl138.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
index ac1a1699d185..a381b26328d8 100644
--- a/arch/arm/mach-davinci/board-mityomapl138.c
+++ b/arch/arm/mach-davinci/board-mityomapl138.c
@@ -14,13 +14,13 @@
#include <linux/init.h>
#include <linux/console.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/mtd/partitions.h>
#include <linux/notifier.h>
#include <linux/nvmem-consumer.h>
#include <linux/nvmem-provider.h>
#include <linux/regulator/machine.h>
#include <linux/i2c.h>
-#include <linux/platform_data/at24.h>
#include <linux/etherdevice.h>
#include <linux/spi/spi.h>
#include <linux/spi/flash.h>
@@ -192,10 +192,10 @@ static struct nvmem_cell_lookup mityomapl138_nvmem_cell_lookup = {
.con_id = "mac-address",
};
-static struct at24_platform_data mityomapl138_fd_chip = {
- .byte_len = 256,
- .page_size = 8,
- .flags = AT24_FLAG_READONLY | AT24_FLAG_IRUGO,
+static const struct property_entry mityomapl138_fd_chip_properties[] = {
+ PROPERTY_ENTRY_U32("pagesize", 8),
+ PROPERTY_ENTRY_BOOL("read-only"),
+ { }
};
static struct davinci_i2c_platform_data mityomap_i2c_0_pdata = {
@@ -324,7 +324,7 @@ static struct i2c_board_info __initdata mityomap_tps65023_info[] = {
},
{
I2C_BOARD_INFO("24c02", 0x50),
- .platform_data = &mityomapl138_fd_chip,
+ .properties = mityomapl138_fd_chip_properties,
},
};
--
2.19.1
^ permalink raw reply related
* [PATCH v2 22/25] ARM: davinci: remove dead code related to MAC address reading
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
There are no more users of davinci_get_mac_addr(). Remove it.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/common.c | 15 ---------------
include/linux/davinci_emac.h | 1 -
2 files changed, 16 deletions(-)
diff --git a/arch/arm/mach-davinci/common.c b/arch/arm/mach-davinci/common.c
index e1d0f0d841ff..0c638fe15dcb 100644
--- a/arch/arm/mach-davinci/common.c
+++ b/arch/arm/mach-davinci/common.c
@@ -26,21 +26,6 @@ EXPORT_SYMBOL(davinci_soc_info);
void __iomem *davinci_intc_base;
int davinci_intc_type;
-void davinci_get_mac_addr(struct nvmem_device *nvmem, void *context)
-{
- char *mac_addr = davinci_soc_info.emac_pdata->mac_addr;
- off_t offset = (off_t)context;
-
- if (!IS_BUILTIN(CONFIG_NVMEM)) {
- pr_warn("Cannot read MAC addr from EEPROM without CONFIG_NVMEM\n");
- return;
- }
-
- /* Read MAC addr from EEPROM */
- if (nvmem_device_read(nvmem, offset, ETH_ALEN, mac_addr) == ETH_ALEN)
- pr_info("Read MAC addr from EEPROM: %pM\n", mac_addr);
-}
-
static int __init davinci_init_id(struct davinci_soc_info *soc_info)
{
int i;
diff --git a/include/linux/davinci_emac.h b/include/linux/davinci_emac.h
index 05b97144d342..28e6cf1356da 100644
--- a/include/linux/davinci_emac.h
+++ b/include/linux/davinci_emac.h
@@ -46,5 +46,4 @@ enum {
EMAC_VERSION_2, /* DM646x */
};
-void davinci_get_mac_addr(struct nvmem_device *nvmem, void *context);
#endif
--
2.19.1
^ permalink raw reply related
* [PATCH v2 20/25] ARM: davinci: sffsdr: fix the at24 eeprom device name
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
The currently used 24lc64 i2c device name doesn't match against any
of the devices supported by the at24 driver. Change it to the closest
compatible chip.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-sffsdr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-davinci/board-sffsdr.c b/arch/arm/mach-davinci/board-sffsdr.c
index 792bb84d5011..acd9778ffa07 100644
--- a/arch/arm/mach-davinci/board-sffsdr.c
+++ b/arch/arm/mach-davinci/board-sffsdr.c
@@ -100,7 +100,7 @@ static struct at24_platform_data eeprom_info = {
static struct i2c_board_info __initdata i2c_info[] = {
{
- I2C_BOARD_INFO("24lc64", 0x50),
+ I2C_BOARD_INFO("24c64", 0x50),
.platform_data = &eeprom_info,
},
/* Other I2C devices:
--
2.19.1
^ permalink raw reply related
* [PATCH v2 18/25] ARM: davinci: dm644x-evm: use device properties for at24 eeprom
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
arch/arm/mach-davinci/board-dm644x-evm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c
index e1428115067f..efe4e170e87d 100644
--- a/arch/arm/mach-davinci/board-dm644x-evm.c
+++ b/arch/arm/mach-davinci/board-dm644x-evm.c
@@ -16,8 +16,8 @@
#include <linux/gpio/machine.h>
#include <linux/i2c.h>
#include <linux/platform_data/pcf857x.h>
-#include <linux/platform_data/at24.h>
#include <linux/platform_data/gpio-davinci.h>
+#include <linux/property.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/rawnand.h>
#include <linux/mtd/partitions.h>
@@ -532,12 +532,9 @@ static struct nvmem_cell_lookup dm644evm_nvmem_cell_lookup = {
.con_id = "mac-address",
};
-static struct at24_platform_data eeprom_info = {
- .byte_len = (256*1024) / 8,
- .page_size = 64,
- .flags = AT24_FLAG_ADDR16,
- .setup = davinci_get_mac_addr,
- .context = (void *)0x7f00,
+static const struct property_entry eeprom_properties[] = {
+ PROPERTY_ENTRY_U32("pagesize", 64),
+ { }
};
/*
@@ -647,7 +644,7 @@ static struct i2c_board_info __initdata i2c_info[] = {
},
{
I2C_BOARD_INFO("24c256", 0x50),
- .platform_data = &eeprom_info,
+ .properties = eeprom_properties,
},
{
I2C_BOARD_INFO("tlv320aic33", 0x1b),
--
2.19.1
^ permalink raw reply related
* [PATCH v2 13/25] net: davinci_emac: use nvmem_get_mac_address()
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
All DaVinci boards still supported in board files now define nvmem
cells containing the MAC address. We want to stop using the setup
callback from at24 so the MAC address for those users will no longer
be provided over platform data. If we didn't get a valid MAC in pdata,
try nvmem before resorting to a random MAC.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/net/ethernet/ti/davinci_emac.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 9153db120352..840820402cd0 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1912,11 +1912,15 @@ static int davinci_emac_probe(struct platform_device *pdev)
ether_addr_copy(ndev->dev_addr, priv->mac_addr);
if (!is_valid_ether_addr(priv->mac_addr)) {
- /* Use random MAC if none passed */
- eth_hw_addr_random(ndev);
- memcpy(priv->mac_addr, ndev->dev_addr, ndev->addr_len);
- dev_warn(&pdev->dev, "using random MAC addr: %pM\n",
- priv->mac_addr);
+ /* Try nvmem if MAC wasn't passed over pdata or DT. */
+ rc = nvmem_get_mac_address(&pdev->dev, priv->mac_addr);
+ if (rc) {
+ /* Use random MAC if still none obtained. */
+ eth_hw_addr_random(ndev);
+ memcpy(priv->mac_addr, ndev->dev_addr, ndev->addr_len);
+ dev_warn(&pdev->dev, "using random MAC addr: %pM\n",
+ priv->mac_addr);
+ }
}
ndev->netdev_ops = &emac_netdev_ops;
--
2.19.1
^ permalink raw reply related
* [PATCH v2 12/25] of: net: kill of_get_nvmem_mac_address()
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We've switched all users to nvmem_get_mac_address(). Remove the now
dead code.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/of/of_net.c | 39 ---------------------------------------
include/linux/of_net.h | 6 ------
2 files changed, 45 deletions(-)
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index 53189d4022a6..810ab0fbcccb 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -81,42 +81,3 @@ const void *of_get_mac_address(struct device_node *np)
return of_get_mac_addr(np, "address");
}
EXPORT_SYMBOL(of_get_mac_address);
-
-/**
- * Obtain the MAC address from an nvmem provider named 'mac-address' through
- * device tree.
- * On success, copies the new address into memory pointed to by addr and
- * returns 0. Returns a negative error code otherwise.
- * @np: Device tree node containing the nvmem-cells phandle
- * @addr: Pointer to receive the MAC address using ether_addr_copy()
- */
-int of_get_nvmem_mac_address(struct device_node *np, void *addr)
-{
- struct nvmem_cell *cell;
- const void *mac;
- size_t len;
- int ret;
-
- cell = of_nvmem_cell_get(np, "mac-address");
- if (IS_ERR(cell))
- return PTR_ERR(cell);
-
- mac = nvmem_cell_read(cell, &len);
-
- nvmem_cell_put(cell);
-
- if (IS_ERR(mac))
- return PTR_ERR(mac);
-
- if (len < ETH_ALEN || !is_valid_ether_addr(mac)) {
- ret = -EINVAL;
- } else {
- ether_addr_copy(addr, mac);
- ret = 0;
- }
-
- kfree(mac);
-
- return ret;
-}
-EXPORT_SYMBOL(of_get_nvmem_mac_address);
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index 90d81ee9e6a0..9cd72aab76fe 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -13,7 +13,6 @@
struct net_device;
extern int of_get_phy_mode(struct device_node *np);
extern const void *of_get_mac_address(struct device_node *np);
-extern int of_get_nvmem_mac_address(struct device_node *np, void *addr);
extern struct net_device *of_find_net_device_by_node(struct device_node *np);
#else
static inline int of_get_phy_mode(struct device_node *np)
@@ -26,11 +25,6 @@ static inline const void *of_get_mac_address(struct device_node *np)
return NULL;
}
-static inline int of_get_nvmem_mac_address(struct device_node *np, void *addr)
-{
- return -ENODEV;
-}
-
static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
{
return NULL;
--
2.19.1
^ permalink raw reply related
* [PATCH v2 11/25] net: cadence: switch to using nvmem_get_mac_address()
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We now have a generalized helper routine to read the MAC address from
nvmem which takes struct device as argument. The nvmem subsystem will
then try device tree first before all other potential providers.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/net/ethernet/cadence/macb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 1d86b4d5645a..d9a208f7bb40 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4055,7 +4055,7 @@ static int macb_probe(struct platform_device *pdev)
if (mac) {
ether_addr_copy(bp->dev->dev_addr, mac);
} else {
- err = of_get_nvmem_mac_address(np, bp->dev->dev_addr);
+ err = nvmem_get_mac_address(&pdev->dev, bp->dev->dev_addr);
if (err) {
if (err == -EPROBE_DEFER)
goto err_out_free_netdev;
--
2.19.1
^ permalink raw reply related
* [PATCH v2 10/25] net: ethernet: provide nvmem_get_mac_address()
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We already have of_get_nvmem_mac_address() but some non-DT systems want
to read the MAC address from NVMEM too. Implement a generalized routine
that takes struct device as argument.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
include/linux/etherdevice.h | 1 +
net/ethernet/eth.c | 38 +++++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 572e11bb8696..2c0af7b00715 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -32,6 +32,7 @@
struct device;
int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr);
unsigned char *arch_get_platform_mac_address(void);
+int nvmem_get_mac_address(struct device *dev, void *addrbuf);
u32 eth_get_headlen(void *data, unsigned int max_len);
__be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev);
extern const struct header_ops eth_header_ops;
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index fd8faa0dfa61..df38593d1bb4 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -47,6 +47,7 @@
#include <linux/inet.h>
#include <linux/ip.h>
#include <linux/netdevice.h>
+#include <linux/nvmem-consumer.h>
#include <linux/etherdevice.h>
#include <linux/skbuff.h>
#include <linux/errno.h>
@@ -548,3 +549,40 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
return 0;
}
EXPORT_SYMBOL(eth_platform_get_mac_address);
+
+/**
+ * Obtain the MAC address from an nvmem cell named 'mac-address' associated
+ * with given device.
+ *
+ * @dev: Device with which the mac-address cell is associated.
+ * @addrbuf: Buffer to which the MAC address will be copied on success.
+ *
+ * Returns 0 on success or a negative error number on failure.
+ */
+int nvmem_get_mac_address(struct device *dev, void *addrbuf)
+{
+ struct nvmem_cell *cell;
+ const void *mac;
+ size_t len;
+
+ cell = nvmem_cell_get(dev, "mac-address");
+ if (IS_ERR(cell))
+ return PTR_ERR(cell);
+
+ mac = nvmem_cell_read(cell, &len);
+ nvmem_cell_put(cell);
+
+ if (IS_ERR(mac))
+ return PTR_ERR(mac);
+
+ if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
+ kfree(mac);
+ return -EINVAL;
+ }
+
+ ether_addr_copy(addrbuf, mac);
+ kfree(mac);
+
+ return 0;
+}
+EXPORT_SYMBOL(nvmem_get_mac_address);
--
2.19.1
^ permalink raw reply related
* [PATCH v2 08/25] ARM: davinci: dm850-evm: use cell nvmem lookup for mac address
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We now support nvmem lookups and cell definitions for machine code.
Add relevant data structures for the mac-address stored in at24 EEPROM.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-da850-evm.c | 29 +++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index e1a949b47306..bac2162e2153 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -28,6 +28,7 @@
#include <linux/mtd/mtd.h>
#include <linux/mtd/rawnand.h>
#include <linux/mtd/partitions.h>
+#include <linux/nvmem-provider.h>
#include <linux/mtd/physmap.h>
#include <linux/platform_device.h>
#include <linux/platform_data/gpio-davinci.h>
@@ -100,6 +101,31 @@ static struct mtd_partition da850evm_spiflash_part[] = {
},
};
+static struct nvmem_cell_info da850evm_nvmem_cells[] = {
+ {
+ .name = "macaddr",
+ .offset = 0x0,
+ .bytes = ETH_ALEN,
+ }
+};
+
+static struct nvmem_cell_table da850evm_nvmem_cell_table = {
+ /*
+ * The nvmem name differs from the partition name because of the
+ * internal works of the nvmem framework.
+ */
+ .nvmem_name = "MAC-Address0",
+ .cells = da850evm_nvmem_cells,
+ .ncells = ARRAY_SIZE(da850evm_nvmem_cells),
+};
+
+static struct nvmem_cell_lookup da850evm_nvmem_cell_lookup = {
+ .nvmem_name = "MAC-Address0",
+ .cell_name = "macaddr",
+ .dev_id = "davinci_emac.1",
+ .con_id = "mac-address",
+};
+
static struct flash_platform_data da850evm_spiflash_data = {
.name = "m25p80",
.parts = da850evm_spiflash_part,
@@ -1395,6 +1421,9 @@ static __init void da850_evm_init(void)
davinci_serial_init(da8xx_serial_device);
+ nvmem_add_cell_table(&da850evm_nvmem_cell_table);
+ nvmem_add_cell_lookups(&da850evm_nvmem_cell_lookup, 1);
+
i2c_register_board_info(1, da850_evm_i2c_devices,
ARRAY_SIZE(da850_evm_i2c_devices));
--
2.19.1
^ permalink raw reply related
* [PATCH v2 06/25] ARM: davinci: da830-evm: use cell nvmem lookup for mac address
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We now support nvmem lookups and cell definitions for machine code.
Add relevant data structures for the mac-address stored in at24 EEPROM.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 26 +++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 7d8ab36ff83d..e52ec1619b70 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -30,6 +30,7 @@
#include <linux/platform_data/usb-davinci.h>
#include <linux/platform_data/ti-aemif.h>
#include <linux/regulator/machine.h>
+#include <linux/nvmem-provider.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -435,6 +436,27 @@ static inline void da830_evm_init_lcdc(int mux_mode)
static inline void da830_evm_init_lcdc(int mux_mode) { }
#endif
+static struct nvmem_cell_info da830_evm_nvmem_cells[] = {
+ {
+ .name = "macaddr",
+ .offset = 0x7f00,
+ .bytes = ETH_ALEN,
+ }
+};
+
+static struct nvmem_cell_table da830_evm_nvmem_cell_table = {
+ .nvmem_name = "1-00500",
+ .cells = da830_evm_nvmem_cells,
+ .ncells = ARRAY_SIZE(da830_evm_nvmem_cells),
+};
+
+static struct nvmem_cell_lookup da830_evm_nvmem_cell_lookup = {
+ .nvmem_name = "1-00500",
+ .cell_name = "macaddr",
+ .dev_id = "davinci_emac.1",
+ .con_id = "mac-address",
+};
+
static struct at24_platform_data da830_evm_i2c_eeprom_info = {
.byte_len = SZ_256K / 8,
.page_size = 64,
@@ -620,6 +642,10 @@ static __init void da830_evm_init(void)
__func__, ret);
davinci_serial_init(da8xx_serial_device);
+
+ nvmem_add_cell_table(&da830_evm_nvmem_cell_table);
+ nvmem_add_cell_lookups(&da830_evm_nvmem_cell_lookup, 1);
+
i2c_register_board_info(1, da830_evm_i2c_devices,
ARRAY_SIZE(da830_evm_i2c_devices));
--
2.19.1
^ permalink raw reply related
* [PATCH v2 02/25] mtd: add support for reading MTD devices via the nvmem API
From: Bartosz Golaszewski @ 2018-11-13 14:01 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger, Nicolas Ferre,
David S . Miller, Grygorii Strashko, Srinivas Kandagatla,
Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
Wolfram Sang
Cc: linux-kernel, linux-arm-kernel, linux-i2c, linux-mtd, netdev,
linux-omap, devicetree, Alban Bedel, Bartosz Golaszewski
In-Reply-To: <20181113140133.17385-1-brgl@bgdev.pl>
From: Alban Bedel <albeu@free.fr>
Allow drivers that use the nvmem API to read data stored on MTD devices.
For this the mtd devices are registered as read-only NVMEM providers.
We don't support device tree systems for now.
Signed-off-by: Alban Bedel <albeu@free.fr>
[Bartosz:
- include linux/nvmem-provider.h
- set the name of the nvmem provider
- set no_of_node to true in nvmem_config
- don't check the return value of nvmem_unregister() - it cannot fail
- tweaked the commit message]
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/mtd/Kconfig | 1 +
drivers/mtd/mtdcore.c | 56 +++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/mtd.h | 2 ++
3 files changed, 59 insertions(+)
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index c77f537323ec..efbe7a6f1d8f 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -1,5 +1,6 @@
menuconfig MTD
tristate "Memory Technology Device (MTD) support"
+ imply NVMEM
help
Memory Technology Devices are flash, RAM and similar chips, often
used for solid state file systems on embedded devices. This option
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 97ac219c082e..5f1053d995b0 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -41,6 +41,7 @@
#include <linux/reboot.h>
#include <linux/leds.h>
#include <linux/debugfs.h>
+#include <linux/nvmem-provider.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
@@ -488,6 +489,50 @@ int mtd_pairing_groups(struct mtd_info *mtd)
}
EXPORT_SYMBOL_GPL(mtd_pairing_groups);
+static int mtd_nvmem_reg_read(void *priv, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct mtd_info *mtd = priv;
+ size_t retlen;
+ int err;
+
+ err = mtd_read(mtd, offset, bytes, &retlen, val);
+ if (err && err != -EUCLEAN)
+ return err;
+
+ return retlen == bytes ? 0 : -EIO;
+}
+
+static int mtd_nvmem_add(struct mtd_info *mtd)
+{
+ struct nvmem_config config = {};
+
+ config.dev = &mtd->dev;
+ config.name = mtd->name;
+ config.owner = THIS_MODULE;
+ config.reg_read = mtd_nvmem_reg_read;
+ config.size = mtd->size;
+ config.word_size = 1;
+ config.stride = 1;
+ config.read_only = true;
+ config.root_only = true;
+ config.no_of_node = true;
+ config.priv = mtd;
+
+ mtd->nvmem = nvmem_register(&config);
+ if (IS_ERR(mtd->nvmem)) {
+ /* Just ignore if there is no NVMEM support in the kernel */
+ if (PTR_ERR(mtd->nvmem) == -ENOSYS) {
+ mtd->nvmem = NULL;
+ } else {
+ dev_err(&mtd->dev, "Failed to register NVMEM device\n");
+ return PTR_ERR(mtd->nvmem);
+ }
+ }
+
+ return 0;
+}
+
static struct dentry *dfs_dir_mtd;
/**
@@ -570,6 +615,11 @@ int add_mtd_device(struct mtd_info *mtd)
if (error)
goto fail_added;
+ /* Add the nvmem provider */
+ error = mtd_nvmem_add(mtd);
+ if (error)
+ goto fail_nvmem_add;
+
if (!IS_ERR_OR_NULL(dfs_dir_mtd)) {
mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(&mtd->dev), dfs_dir_mtd);
if (IS_ERR_OR_NULL(mtd->dbg.dfs_dir)) {
@@ -595,6 +645,8 @@ int add_mtd_device(struct mtd_info *mtd)
__module_get(THIS_MODULE);
return 0;
+fail_nvmem_add:
+ device_unregister(&mtd->dev);
fail_added:
of_node_put(mtd_get_of_node(mtd));
idr_remove(&mtd_idr, i);
@@ -637,6 +689,10 @@ int del_mtd_device(struct mtd_info *mtd)
mtd->index, mtd->name, mtd->usecount);
ret = -EBUSY;
} else {
+ /* Try to remove the NVMEM provider */
+ if (mtd->nvmem)
+ nvmem_unregister(mtd->nvmem);
+
device_unregister(&mtd->dev);
idr_remove(&mtd_idr, mtd->index);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index cd0be91bdefa..545070c2ee64 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -25,6 +25,7 @@
#include <linux/notifier.h>
#include <linux/device.h>
#include <linux/of.h>
+#include <linux/nvmem-provider.h>
#include <mtd/mtd-abi.h>
@@ -341,6 +342,7 @@ struct mtd_info {
struct device dev;
int usecount;
struct mtd_debug_info dbg;
+ struct nvmem_device *nvmem;
};
int mtd_ooblayout_ecc(struct mtd_info *mtd, int section,
--
2.19.1
^ permalink raw reply related
* Re: [PATCH net-next 16/17] net: sched: conditionally take rtnl lock on rules update path
From: Vlad Buslov @ 2018-11-13 13:58 UTC (permalink / raw)
To: Stefano Brivio
Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
ast@kernel.org, daniel@iogearbox.net
In-Reply-To: <20181113144035.03e3e278@redhat.com>
On Tue 13 Nov 2018 at 13:40, Stefano Brivio <sbrivio@redhat.com> wrote:
> On Tue, 13 Nov 2018 13:25:52 +0000
> Vlad Buslov <vladbu@mellanox.com> wrote:
>
>> On Tue 13 Nov 2018 at 09:40, Stefano Brivio <sbrivio@redhat.com> wrote:
>> > Hi Vlad,
>> >
>> > On Mon, 12 Nov 2018 09:55:45 +0200
>> > Vlad Buslov <vladbu@mellanox.com> wrote:
>> >
>> >> @@ -179,9 +179,25 @@ static void tcf_proto_destroy_work(struct work_struct *work)
>> >> rtnl_unlock();
>> >> }
>> >>
>> >> +/* Helper function to lock rtnl mutex when specified condition is true and mutex
>> >> + * hasn't been locked yet. Will set rtnl_held to 'true' before taking rtnl lock.
>> >> + * Note that this function does nothing if rtnl is already held. This is
>> >> + * intended to be used by cls API rules update API when multiple conditions
>> >> + * could require rtnl lock and its state needs to be tracked to prevent trying
>> >> + * to obtain lock multiple times.
>> >> + */
>> >> +
>> >> +static void tcf_require_rtnl(bool cond, bool *rtnl_held)
>> >> +{
>> >> + if (!*rtnl_held && cond) {
>> >> + *rtnl_held = true;
>> >> + rtnl_lock();
>> >> + }
>> >> +}
>> >
>> > I guess calls to this function are supposed to be serialised. If that's
>> > the case (which is my tentative understanding so far), I would indicate
>> > that in the comment.
>> >
>> > If that's not the case, you would be introducing a race I guess.
>> >
>> > Same applies to tcf_block_release() from 17/17.
>>
>> Hi Stefano,
>>
>> Thank you for reviewing my code!
>>
>> I did not intend for this function to be serialized. First argument to
>> tcf_require_rtnl() is passed by value, and second argument is always a
>> pointer to local stack-allocated value of the caller.
>
> Yes, sorry, I haven't been terribly clear, that's what I meant by
> serialised: it won't be called concurrently with the same *rtnl_held.
>
> Perhaps the risk that somebody uses it that way is close to zero, so
> I'm not even too sure this is worth a comment, but if you can come up
> with a concise way of saying this, that would be nice.
I considered my comment that function "Will set rtnl_held to 'true'
before taking rtnl lock" as a red flag for caller to not pass pointer to
a variable that can be accessed concurrently. I guess I can add
additional sentence to explicitly warn potential users. Or I can just
move rtnl_held assignment in both functions to be performed while
holding rtnl mutex. I implemented it the way I did as an overzealous
optimization, but realistically price of an assignment is negligible in
this case. Suggestions are welcome!
>
>> Same applies to tcf_block_release() - its arguments are Qdisc and block
>> which support concurrency-safe reference counting, and pointer to local
>> variable rtnl_held, which is not accessible to concurrent users.
>
> Same there.
>
>> What is the race in these cases? Am I missing something?
>
> No, no race then. My only concern was:
>
> thread A: thread B:
> - x = false;
> - tcf_require_rtnl(true, &x); - tcf_require_rtnl(true, &x);
> - if (!*x && true) - if (!*x && true)
> - *x = true;
> - rtnl_lock() - *x = true;
> - rtnl_lock()
>
> but this cannot happen as you explained.
^ permalink raw reply
* Re: [PATCH net-next 17/17] net: sched: unlock rules update API
From: Vlad Buslov @ 2018-11-13 13:46 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, ast@kernel.org,
daniel@iogearbox.net
In-Reply-To: <20181112.093014.891880255643252936.davem@davemloft.net>
On Mon 12 Nov 2018 at 17:30, David Miller <davem@davemloft.net> wrote:
> From: Vlad Buslov <vladbu@mellanox.com>
> Date: Mon, 12 Nov 2018 09:55:46 +0200
>
>> Register netlink protocol handlers for message types RTM_NEWTFILTER,
>> RTM_DELTFILTER, RTM_GETTFILTER as unlocked. Set rtnl_held variable that
>> tracks rtnl mutex state to be false by default.
>
> This whole conditional locking mechanism is really not clean and makes
> this code so much harder to understand and audit.
>
> Please improve the code so that this kind of construct is not needed.
>
> Thank you.
Hi David,
I considered several approaches to this problem and decided that this
one is most straightforward to implement. I understand your concern and
agree that this code is not easiest to understand and can suggest
several possible solutions that do not require this kind of elaborate
locking mechanism in cls API, but have their own drawbacks:
1. Convert all qdiscs and classifiers to support unlocked execution,
like we did for actions. However, according to my experience with
converting flower classifier, these require much more code than actions.
I would estimate it to be more work than whole current unlocking effort
(hundred+ patches). Also, authors of some of them might be unhappy with
such intrusive changes. I don't think this approach is realistic.
2. Somehow determine if rtnl is needed at the beginning of cls API rule
update functions. Currently, this is not possible because locking
requirements are determined by qdisc_class_ops and tcf_proto_ops 'flags'
field, which requires code to first do whole ops lookup sequence.
However, instead of class field I can put 'flags' in some kind of hash
table or array that will map qdisc/classifier type string to flags, so
it will be possible to determine locking requirements by just parsing
netlink message and obtaining flags by qdisc/classifier type. I do not
consider it pretty solution either, but maybe you have different
opinion.
3. Anything you can suggest? I might be missing something simple that
you would consider more elegant solution to this problem.
Thanks,
Vlad
^ permalink raw reply
* Re: [PATCH net-next 16/17] net: sched: conditionally take rtnl lock on rules update path
From: Stefano Brivio @ 2018-11-13 13:40 UTC (permalink / raw)
To: Vlad Buslov
Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
ast@kernel.org, daniel@iogearbox.net
In-Reply-To: <vbfin11jeml.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
On Tue, 13 Nov 2018 13:25:52 +0000
Vlad Buslov <vladbu@mellanox.com> wrote:
> On Tue 13 Nov 2018 at 09:40, Stefano Brivio <sbrivio@redhat.com> wrote:
> > Hi Vlad,
> >
> > On Mon, 12 Nov 2018 09:55:45 +0200
> > Vlad Buslov <vladbu@mellanox.com> wrote:
> >
> >> @@ -179,9 +179,25 @@ static void tcf_proto_destroy_work(struct work_struct *work)
> >> rtnl_unlock();
> >> }
> >>
> >> +/* Helper function to lock rtnl mutex when specified condition is true and mutex
> >> + * hasn't been locked yet. Will set rtnl_held to 'true' before taking rtnl lock.
> >> + * Note that this function does nothing if rtnl is already held. This is
> >> + * intended to be used by cls API rules update API when multiple conditions
> >> + * could require rtnl lock and its state needs to be tracked to prevent trying
> >> + * to obtain lock multiple times.
> >> + */
> >> +
> >> +static void tcf_require_rtnl(bool cond, bool *rtnl_held)
> >> +{
> >> + if (!*rtnl_held && cond) {
> >> + *rtnl_held = true;
> >> + rtnl_lock();
> >> + }
> >> +}
> >
> > I guess calls to this function are supposed to be serialised. If that's
> > the case (which is my tentative understanding so far), I would indicate
> > that in the comment.
> >
> > If that's not the case, you would be introducing a race I guess.
> >
> > Same applies to tcf_block_release() from 17/17.
>
> Hi Stefano,
>
> Thank you for reviewing my code!
>
> I did not intend for this function to be serialized. First argument to
> tcf_require_rtnl() is passed by value, and second argument is always a
> pointer to local stack-allocated value of the caller.
Yes, sorry, I haven't been terribly clear, that's what I meant by
serialised: it won't be called concurrently with the same *rtnl_held.
Perhaps the risk that somebody uses it that way is close to zero, so
I'm not even too sure this is worth a comment, but if you can come up
with a concise way of saying this, that would be nice.
> Same applies to tcf_block_release() - its arguments are Qdisc and block
> which support concurrency-safe reference counting, and pointer to local
> variable rtnl_held, which is not accessible to concurrent users.
Same there.
> What is the race in these cases? Am I missing something?
No, no race then. My only concern was:
thread A: thread B:
- x = false;
- tcf_require_rtnl(true, &x); - tcf_require_rtnl(true, &x);
- if (!*x && true) - if (!*x && true)
- *x = true;
- rtnl_lock() - *x = true;
- rtnl_lock()
but this cannot happen as you explained.
--
Stefano
^ permalink raw reply
* Re: [PATCH net-next 02/17] net: sched: protect block state with spinlock
From: Vlad Buslov @ 2018-11-13 13:28 UTC (permalink / raw)
To: Stefano Brivio
Cc: David Miller, netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, ast@kernel.org,
daniel@iogearbox.net
In-Reply-To: <20181113110712.4462807a@redhat.com>
On Tue 13 Nov 2018 at 10:07, Stefano Brivio <sbrivio@redhat.com> wrote:
> Vlad,
>
> On Mon, 12 Nov 2018 09:28:59 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
>
>> From: Vlad Buslov <vladbu@mellanox.com>
>> Date: Mon, 12 Nov 2018 09:55:31 +0200
>>
>> > +#define ASSERT_BLOCK_LOCKED(block) \
>> > + WARN_ONCE(!spin_is_locked(&(block)->lock), \
>> > + "BLOCK: assertion failed at %s (%d)\n", __FILE__, __LINE__)
>>
>> spin_is_locked() is not usable for assertions.
>
> See also b86077207d0c ("igbvf: Replace spin_is_locked() with
> lockdep").
Stefano,
Thanks for the tip. I will check it out.
Vlad.
^ permalink raw reply
* Re: [PATCH net-next 16/17] net: sched: conditionally take rtnl lock on rules update path
From: Vlad Buslov @ 2018-11-13 13:25 UTC (permalink / raw)
To: Stefano Brivio
Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
ast@kernel.org, daniel@iogearbox.net
In-Reply-To: <20181113104016.76d12436@redhat.com>
On Tue 13 Nov 2018 at 09:40, Stefano Brivio <sbrivio@redhat.com> wrote:
> Hi Vlad,
>
> On Mon, 12 Nov 2018 09:55:45 +0200
> Vlad Buslov <vladbu@mellanox.com> wrote:
>
>> @@ -179,9 +179,25 @@ static void tcf_proto_destroy_work(struct work_struct *work)
>> rtnl_unlock();
>> }
>>
>> +/* Helper function to lock rtnl mutex when specified condition is true and mutex
>> + * hasn't been locked yet. Will set rtnl_held to 'true' before taking rtnl lock.
>> + * Note that this function does nothing if rtnl is already held. This is
>> + * intended to be used by cls API rules update API when multiple conditions
>> + * could require rtnl lock and its state needs to be tracked to prevent trying
>> + * to obtain lock multiple times.
>> + */
>> +
>> +static void tcf_require_rtnl(bool cond, bool *rtnl_held)
>> +{
>> + if (!*rtnl_held && cond) {
>> + *rtnl_held = true;
>> + rtnl_lock();
>> + }
>> +}
>
> I guess calls to this function are supposed to be serialised. If that's
> the case (which is my tentative understanding so far), I would indicate
> that in the comment.
>
> If that's not the case, you would be introducing a race I guess.
>
> Same applies to tcf_block_release() from 17/17.
Hi Stefano,
Thank you for reviewing my code!
I did not intend for this function to be serialized. First argument to
tcf_require_rtnl() is passed by value, and second argument is always a
pointer to local stack-allocated value of the caller. Same applies to
tcf_block_release() - its arguments are Qdisc and block which support
concurrency-safe reference counting, and pointer to local variable
rtnl_held, which is not accessible to concurrent users.
What is the race in these cases? Am I missing something?
Vlad
^ permalink raw reply
* Re: [PATCH net-next 01/17] net: sched: refactor mini_qdisc_pair_swap() to use workqueue
From: Vlad Buslov @ 2018-11-13 13:13 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, ast@kernel.org,
daniel@iogearbox.net
In-Reply-To: <20181112.092809.744587974364715455.davem@davemloft.net>
On Mon 12 Nov 2018 at 17:28, David Miller <davem@davemloft.net> wrote:
> From: Vlad Buslov <vladbu@mellanox.com>
> Date: Mon, 12 Nov 2018 09:55:30 +0200
>
>> +void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
>> + struct tcf_proto *tp_head)
>> +{
>> + xchg(&miniqp->tp_head, tp_head);
>
> If you are not checking the return value of xchg(), then this is
> simply a store with optionally a memory barrier of some sort
> either before or after.
That was my intention. What would be a better way to atomically
reset a pointer? Should I just change this line to explicit
assignment+barrier?
^ permalink raw reply
* [iproute PATCH] ip-route: Fix nexthop encap parsing
From: Phil Sutter @ 2018-11-13 12:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
When parsing nexthop parameters, a buffer of 4k bytes is provided. Yet,
in lwt_parse_encap() and some functions called by it, buffer size was
assumed to be 1k despite the actual size was provided. This led to
spurious buffer size errors if the buffer was filled by previous nexthop
parameters to exceed that 1k boundary.
Fixes: 1e5293056a02c ("lwtunnel: Add encapsulation support to ip route")
Fixes: 5866bddd9aa9e ("ila: Add support for ILA lwtunnels")
Fixes: ed67f83806538 ("ila: Support for checksum neutral translation")
Fixes: 86905c8f057c0 ("ila: support for configuring identifier and hook types")
Fixes: b15f440e78373 ("lwt: BPF support for LWT")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/iproute_lwtunnel.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 8f49701509d20..85ab13cb31746 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -860,7 +860,7 @@ static int parse_encap_ila(struct rtattr *rta, size_t len,
argc--; argv++;
- if (rta_addattr64(rta, 1024, ILA_ATTR_LOCATOR, locator))
+ if (rta_addattr64(rta, len, ILA_ATTR_LOCATOR, locator))
return -1;
while (argc > 0) {
@@ -874,7 +874,7 @@ static int parse_encap_ila(struct rtattr *rta, size_t len,
invarg("\"csum-mode\" value is invalid\n",
*argv);
- ret = rta_addattr8(rta, 1024, ILA_ATTR_CSUM_MODE,
+ ret = rta_addattr8(rta, len, ILA_ATTR_CSUM_MODE,
(__u8)csum_mode);
argc--; argv++;
@@ -888,7 +888,7 @@ static int parse_encap_ila(struct rtattr *rta, size_t len,
invarg("\"ident-type\" value is invalid\n",
*argv);
- ret = rta_addattr8(rta, 1024, ILA_ATTR_IDENT_TYPE,
+ ret = rta_addattr8(rta, len, ILA_ATTR_IDENT_TYPE,
(__u8)ident_type);
argc--; argv++;
@@ -902,7 +902,7 @@ static int parse_encap_ila(struct rtattr *rta, size_t len,
invarg("\"hook-type\" value is invalid\n",
*argv);
- ret = rta_addattr8(rta, 1024, ILA_ATTR_HOOK_TYPE,
+ ret = rta_addattr8(rta, len, ILA_ATTR_HOOK_TYPE,
(__u8)hook_type);
argc--; argv++;
@@ -1034,7 +1034,7 @@ static int parse_encap_bpf(struct rtattr *rta, size_t len, int *argcp,
if (get_unsigned(&headroom, *argv, 0) || headroom == 0)
invarg("headroom is invalid\n", *argv);
if (!headroom_set)
- rta_addattr32(rta, 1024, LWT_BPF_XMIT_HEADROOM,
+ rta_addattr32(rta, len, LWT_BPF_XMIT_HEADROOM,
headroom);
headroom_set = 1;
} else if (strcmp(*argv, "help") == 0) {
@@ -1075,7 +1075,7 @@ int lwt_parse_encap(struct rtattr *rta, size_t len, int *argcp, char ***argvp)
exit(-1);
}
- nest = rta_nest(rta, 1024, RTA_ENCAP);
+ nest = rta_nest(rta, len, RTA_ENCAP);
switch (type) {
case LWTUNNEL_ENCAP_MPLS:
ret = parse_encap_mpls(rta, len, &argc, &argv);
@@ -1108,7 +1108,7 @@ int lwt_parse_encap(struct rtattr *rta, size_t len, int *argcp, char ***argvp)
rta_nest_end(rta, nest);
- ret = rta_addattr16(rta, 1024, RTA_ENCAP_TYPE, type);
+ ret = rta_addattr16(rta, len, RTA_ENCAP_TYPE, type);
*argcp = argc;
*argvp = argv;
--
2.19.0
^ permalink raw reply related
* Re: [iproute PATCH] man: ip-route.8: Document nexthop limit
From: Phil Sutter @ 2018-11-13 12:37 UTC (permalink / raw)
To: David Ahern; +Cc: Stephen Hemminger, netdev
In-Reply-To: <dd513bca-acff-bcbd-cf81-6a3969a0cb9f@gmail.com>
Hi David,
On Mon, Nov 12, 2018 at 04:37:48PM -0800, David Ahern wrote:
> On 11/12/18 2:21 PM, Phil Sutter wrote:
> > diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
> > index a33ce1f0f4006..383178c11331e 100644
> > --- a/man/man8/ip-route.8.in
> > +++ b/man/man8/ip-route.8.in
> > @@ -589,6 +589,13 @@ argument lists:
> > route reflecting its relative bandwidth or quality.
> > .in -8
> >
> > +The internal buffer used in iproute2 limits the maximum number of nexthops to
> > +be specified in one go. If only a gateway address is given, the current buffer
> > +size allows for 144 IPv6 nexthops and 253 IPv4 ones. If more are required, they
> > +may be added to the existing route using
> > +.B "ip route append"
> > +command.
> > +
>
> That is not true for IPv4. 'ip ro append' adds a new route after the
> existing route - an entry that can not be hit unless all of the nexthops
> in the first route are down. 'ip ro prepend' adds a new entry before the
> existing one meaning it takes precedence over the existing entries.
Oh, thanks for clarifying. I'll follow-up with a fixed version.
> For IPv6, 'append' and 'prepend' both add new nexthops to the existing
> entry.
'ip route prepend' is not even documented. :(
Thanks, Phil
^ permalink raw reply
* Re: VETH & AF_PACKET problem
From: Anand H. Krishnan @ 2018-11-13 11:24 UTC (permalink / raw)
To: netdev
In-Reply-To: <CACeb84v7xj1PNGJj=Wbq_Nvq1QK32u6kD2=4oreUT0+VU2jbbQ@mail.gmail.com>
[bump]
Hello,
Any ideas? AF_PACKET with veth seems to be incompatible. I am using DPDK to bind
af_packet with one end of VETH pair and the other end of the pair is
in a container.
Thanks,
Anand
On Mon, Nov 12, 2018 at 12:42 PM Anand H. Krishnan
<anandhkrishnan@gmail.com> wrote:
>
> Hello,
>
> We are seeing a problem with AF_PACKET when used along with the
> veth interfaces. SCP complains that message authentication code is
> incorrect.
>
> I was browsing the code and I see that veth_xmit calls ____dev_forward_skb
> which does a skb_scrub_packet, which in turn calls the skb destructor function.
>
> In the case of packets coming from the AF_PACKET socket, the destructor
> function seems to set all the mmap-ed pages to be available for user space to
> copy any new packet it wants. Isn't this a problem?
>
> skb_orphan_frags, called by ____dev_forward_skb, seems to do the right thing,
> but it probably does not get called for packets from AF_PACKET socket, since the
> skb is not a zero copy skb (SKBTX_DEV_ZEROCOPY is not set).
>
> Did I miss something basic here?
>
> (Please cc me, since I am not part of this list)
>
> Thanks,
> Anand
^ permalink raw reply
* Re: [PATCH v1] tg3: optionally get mac address from devicetree
From: Andrew Lunn @ 2018-11-13 21:09 UTC (permalink / raw)
To: thesven73
Cc: svendev, siva.kallam, prashant, mchan, davem, linux-kernel,
netdev, arnd
In-Reply-To: <20181113161508.7427-1-TheSven73@googlemail.com>
> +static int tg3_of_get_macaddr(struct tg3 *tp)
> {
> - struct net_device *dev = tp->dev;
> - struct pci_dev *pdev = tp->pdev;
> - struct device_node *dp = pci_device_to_OF_node(pdev);
> - const unsigned char *addr;
> - int len;
> + struct device_node *np = pci_device_to_OF_node(tp->pdev);
> + const void *mac;
>
> - addr = of_get_property(dp, "local-mac-address", &len);
> - if (addr && len == ETH_ALEN) {
> - memcpy(dev->dev_addr, addr, ETH_ALEN);
> - return 0;
> - }
> - return -ENODEV;
> + if (!np)
> + return -ENODEV;
> + mac = of_get_mac_address(np);
> + if (!mac || !is_valid_ether_addr(mac))
> + return -EINVAL;
Hi Sven
If i'm reading of_get_mac_address() correctly, there is no need to
call is_valid_ether_addr() afterwards. It does it already.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 02/17] net: sched: protect block state with spinlock
From: Stefano Brivio @ 2018-11-13 10:07 UTC (permalink / raw)
To: vladbu; +Cc: David Miller, netdev, jhs, xiyou.wangcong, jiri, ast, daniel
In-Reply-To: <20181112.092859.498970939054805019.davem@davemloft.net>
Vlad,
On Mon, 12 Nov 2018 09:28:59 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
> From: Vlad Buslov <vladbu@mellanox.com>
> Date: Mon, 12 Nov 2018 09:55:31 +0200
>
> > +#define ASSERT_BLOCK_LOCKED(block) \
> > + WARN_ONCE(!spin_is_locked(&(block)->lock), \
> > + "BLOCK: assertion failed at %s (%d)\n", __FILE__, __LINE__)
>
> spin_is_locked() is not usable for assertions.
See also b86077207d0c ("igbvf: Replace spin_is_locked() with lockdep").
--
Stefano
^ permalink raw reply
* Re: [PATCH net-next 16/17] net: sched: conditionally take rtnl lock on rules update path
From: Stefano Brivio @ 2018-11-13 9:40 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem, ast, daniel
In-Reply-To: <1542009346-23780-17-git-send-email-vladbu@mellanox.com>
Hi Vlad,
On Mon, 12 Nov 2018 09:55:45 +0200
Vlad Buslov <vladbu@mellanox.com> wrote:
> @@ -179,9 +179,25 @@ static void tcf_proto_destroy_work(struct work_struct *work)
> rtnl_unlock();
> }
>
> +/* Helper function to lock rtnl mutex when specified condition is true and mutex
> + * hasn't been locked yet. Will set rtnl_held to 'true' before taking rtnl lock.
> + * Note that this function does nothing if rtnl is already held. This is
> + * intended to be used by cls API rules update API when multiple conditions
> + * could require rtnl lock and its state needs to be tracked to prevent trying
> + * to obtain lock multiple times.
> + */
> +
> +static void tcf_require_rtnl(bool cond, bool *rtnl_held)
> +{
> + if (!*rtnl_held && cond) {
> + *rtnl_held = true;
> + rtnl_lock();
> + }
> +}
I guess calls to this function are supposed to be serialised. If that's
the case (which is my tentative understanding so far), I would indicate
that in the comment.
If that's not the case, you would be introducing a race I guess.
Same applies to tcf_block_release() from 17/17.
--
Stefano
^ permalink raw reply
* RE: [PATCH net-next v3 6/6] net/ncsi: Configure multi-package, multi-channel modes with failover
From: Justin.Lee1 @ 2018-11-13 18:00 UTC (permalink / raw)
To: sam, netdev; +Cc: davem, linux-kernel, openbmc
In-Reply-To: <cfeab378a2b226c1c0ba62912d18c5a254de8daa.camel@mendozajonas.com>
Hi Samuel,
I have tested your new patch. The failover function works as expected.
Thanks,
Justin
> On Fri, 2018-11-09 at 21:58 +0000, Justin.Lee1@Dell.com wrote:
> > Hi Samuel,
> >
> > After running more testing, I notice that the extra patch causing failover function
> > to fail. Also, occasionally, I will see two channels having TX enabled if I
> > send netlink command back-to-back.
> >
> > cat /sys/kernel/debug/ncsi_protocol/ncsi_device_
> > IFIDX IFNAME NAME PID CID RX TX MP MC WP WC PC CS PS LS RU CR NQ HA
> > =====================================================================
> > 2 eth2 ncsi0 000 000 1 1 1 1 1 1 1 2 1 1 1 1 0 1
> > 2 eth2 ncsi1 000 001 1 1 1 1 1 1 0 2 1 1 1 1 0 1
> > 2 eth2 ncsi2 001 000 0 0 1 1 0 0 0 1 0 1 1 1 0 1
> > 2 eth2 ncsi3 001 001 0 0 1 1 0 0 0 1 0 1 1 1 0 1
> > =====================================================================
> > MP: Multi-mode Package WP: Whitelist Package
> > MC: Multi-mode Channel WC: Whitelist Channel
> > PC: Primary Channel CS: Channel State IA/A/IV 1/2/3
> > PS: Poll Status LS: Link Status
> > RU: Running CR: Carrier OK
> > NQ: Queue Stopped HA: Hardware Arbitration
> >
> > Thanks,
> > Justin
> >
> >
> > > From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> > > From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> > > Date: Fri, 9 Nov 2018 13:11:03 +1100
> > > Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC
> > >
> > > ---
> > > net/ncsi/ncsi-aen.c | 8 +++++---
> > > net/ncsi/ncsi-manage.c | 19 +++++++++++++++----
> > > 2 files changed, 20 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
> > > index 39c2e9eea2ba..034cb1dc5566 100644
> > > --- a/net/ncsi/ncsi-aen.c
> > > +++ b/net/ncsi/ncsi-aen.c
> > > @@ -93,14 +93,16 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
> > > if ((had_link == has_link) || chained)
> > > return 0;
> > >
> > > - if (!ndp->multi_package && !nc->package->multi_channel) {
> > > - if (had_link)
> > > - ndp->flags |= NCSI_DEV_RESHUFFLE;
> > > + if (!ndp->multi_package && !nc->package->multi_channel && had_link) {
> > > + ndp->flags |= NCSI_DEV_RESHUFFLE;
> > > ncsi_stop_channel_monitor(nc);
> > > spin_lock_irqsave(&ndp->lock, flags);
> > > list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> > > spin_unlock_irqrestore(&ndp->lock, flags);
> > > return ncsi_process_next_channel(ndp);
> > > + } else {
> > > + /* Configured channel came up */
> > > + return 0;
> >
> > It is always going to else statement if multi_package and/or mutlit_channel is enabled.
> >
> > npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - pkg 0 ch 0 state down
> > npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - had_link 1, has_link 0, chained 0
> >
> > These codes have no chance to run.
> > if (had_link) {
> > ncm = &nc->modes[NCSI_MODE_TX_ENABLE];
> > if (ncsi_channel_is_last(ndp, nc)) {
> > /* No channels left, reconfigure */
> > return ncsi_reset_dev(&ndp->ndev);
> > } else if (ncm->enable) {
> > /* Need to failover Tx channel */
> > ncsi_update_tx_channel(ndp, nc->package, nc, NULL);
> > }
> >
> > > }
> > >
>
> Oops, wrote that patch a little fast it seems. This may also affect the
> double-Tx above since ncsi_update_tx_channel() won't be reached.
> I've attached a fixed up version below.
>
> For the failover issue you're seeing in your previous email the issue is
> likely in the ncsi_dev_state_suspend_gls state. This should send a GLS
> command to all available channels, but it only does it for the current
> package. Since not all packages are necessarily enabled in single-channel
> mode I'll need to have a think about the neatest way to handle that, but
> it's a separate issue from this patch.
>
> Cheers,
> Sam
>
>
> From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> Date: Fri, 9 Nov 2018 13:11:03 +1100
> Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC
>
> ---
> net/ncsi/ncsi-aen.c | 16 ++++++++++------
> net/ncsi/ncsi-manage.c | 19 +++++++++++++++----
> 2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
> index 39c2e9eea2ba..76559d0eeea8 100644
> --- a/net/ncsi/ncsi-aen.c
> +++ b/net/ncsi/ncsi-aen.c
> @@ -94,13 +94,17 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
> return 0;
>
> if (!ndp->multi_package && !nc->package->multi_channel) {
> - if (had_link)
> + if (had_link) {
> ndp->flags |= NCSI_DEV_RESHUFFLE;
> - ncsi_stop_channel_monitor(nc);
> - spin_lock_irqsave(&ndp->lock, flags);
> - list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> - spin_unlock_irqrestore(&ndp->lock, flags);
> - return ncsi_process_next_channel(ndp);
> + ncsi_stop_channel_monitor(nc);
> + spin_lock_irqsave(&ndp->lock, flags);
> + list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> + spin_unlock_irqrestore(&ndp->lock, flags);
> + return ncsi_process_next_channel(ndp);
> + } else {
> + /* Configured channel came up */
> + return 0;
> + }
> }
>
> if (had_link) {
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index fa3c2144f5ba..92e59f07f9a7 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -1063,17 +1063,17 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> case ncsi_dev_state_config_done:
> netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
> nc->id);
> + spin_lock_irqsave(&nc->lock, flags);
> + nc->state = NCSI_CHANNEL_ACTIVE;
> +
> if (ndp->flags & NCSI_DEV_RESET) {
> /* A reset event happened during config, start it now */
> - spin_lock_irqsave(&nc->lock, flags);
> nc->reconfigure_needed = false;
> spin_unlock_irqrestore(&nc->lock, flags);
> - nd->state = ncsi_dev_state_functional;
> ncsi_reset_dev(nd);
> break;
> }
>
> - spin_lock_irqsave(&nc->lock, flags);
> if (nc->reconfigure_needed) {
> /* This channel's configuration has been updated
> * part-way during the config state - start the
> @@ -1092,7 +1092,6 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> break;
> }
>
> - nc->state = NCSI_CHANNEL_ACTIVE;
> if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
> hot_nc = nc;
> } else {
> @@ -1803,6 +1802,18 @@ int ncsi_reset_dev(struct ncsi_dev *nd)
> spin_unlock_irqrestore(&ndp->lock, flags);
> return 0;
> }
> + } else {
> + switch (nd->state) {
> + case ncsi_dev_state_suspend_done:
> + case ncsi_dev_state_config_done:
> + case ncsi_dev_state_functional:
> + /* Ok */
> + break;
> + default:
> + /* Current reset operation happening */
> + spin_unlock_irqrestore(&ndp->lock, flags);
> + return 0;
> + }
> }
>
> if (!list_empty(&ndp->channel_queue)) {
> --
> 2.19.1
^ 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