* Re: [PATCH v2] net: dsa: sja1105: prevent leaking memory
From: Vladimir Oltean @ 2019-09-18 20:00 UTC (permalink / raw)
To: Navid Emamdoost, andrew
Cc: emamd001, smccaman, kjlu, Vivien Didelot, Florian Fainelli,
David S. Miller, linux-kernel, netdev
In-Reply-To: <20190918180439.12441-1-navid.emamdoost@gmail.com>
Hi Navid,
Thanks for the patch.
On 9/18/19 9:04 PM, Navid Emamdoost wrote:
> In sja1105_static_config_upload, in two cases memory is leaked: when
> static_config_buf_prepare_for_upload fails and when sja1105_inhibit_tx
> fails. In both cases config_buf should be released.
>
> Fixes: 8aa9ebccae876 (avoid leaking config_buf)
> Fixes: 1a4c69406cc1c (avoid leaking config_buf)
>
You're not supposed to add a short description of the patch here, but
rather the commit message of the patch you're fixing.
Add this to your ~/.gitconfig:
[pretty]
fixes = Fixes: %h (\"%s\")
And then run:
git show --pretty=fixes 8aa9ebccae87621d997707e4f25e53fddd7e30e4
Fixes: 8aa9ebccae87 ("net: dsa: Introduce driver for NXP SJA1105 5-port
L2 switch")
git show --pretty=fixes 1a4c69406cc1c3c42bb7391c8eb544e93fe9b320
Fixes: 1a4c69406cc1 ("net: dsa: sja1105: Prevent PHY jabbering during
switch reset")
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> drivers/net/dsa/sja1105/sja1105_spi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
> index 84dc603138cf..58dd37ecde17 100644
> --- a/drivers/net/dsa/sja1105/sja1105_spi.c
> +++ b/drivers/net/dsa/sja1105/sja1105_spi.c
> @@ -409,7 +409,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
> rc = static_config_buf_prepare_for_upload(priv, config_buf, buf_len);
> if (rc < 0) {
> dev_err(dev, "Invalid config, cannot upload\n");
> - return -EINVAL;
> + rc = -EINVAL;
> + goto out;
> }
> /* Prevent PHY jabbering during switch reset by inhibiting
> * Tx on all ports and waiting for current packet to drain.
> @@ -418,7 +419,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
> rc = sja1105_inhibit_tx(priv, port_bitmap, true);
> if (rc < 0) {
> dev_err(dev, "Failed to inhibit Tx on ports\n");
> - return -ENXIO;
> + rc = -ENXIO;
> + goto out;
> }
> /* Wait for an eventual egress packet to finish transmission
> * (reach IFG). It is guaranteed that a second one will not
>
Regards,
-Vladimir
^ permalink raw reply
* Re: [patch iproute2-next v2] devlink: add reload failed indication
From: David Ahern @ 2019-09-18 20:01 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, stephen, idosch, jakub.kicinski, tariqt, mlxsw
In-Reply-To: <20190918073738.GA2543@nanopsycho>
On 9/18/19 1:37 AM, Jiri Pirko wrote:
> Wed, Sep 18, 2019 at 01:46:13AM CEST, dsahern@gmail.com wrote:
>> On 9/17/19 12:36 PM, Jiri Pirko wrote:
>>> Tue, Sep 17, 2019 at 06:46:31PM CEST, dsahern@gmail.com wrote:
>>>> On 9/16/19 3:44 AM, Jiri Pirko wrote:
>>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>>
>>>>> Add indication about previous failed devlink reload.
>>>>>
>>>>> Example outputs:
>>>>>
>>>>> $ devlink dev
>>>>> netdevsim/netdevsim10: reload_failed true
>>>>
>>>> odd output to user. Why not just "reload failed"?
>>>
>>> Well it is common to have "name value". The extra space would seem
>>> confusing for the reader..
>>> Also it is common to have "_" instead of space for the output in cases
>>> like this.
>>>
>>
>> I am not understanding your point.
>>
>> "reload failed" is still a name/value pair. It is short and to the point
>> as to what it indicates. There is no need for the name in the uapi (ie.,
>> the name of the netlink attribute) to be dumped here.
>
> Ah, got it. Well it is a bool value, that means it is "true" or "false".
> In json output, it is True of False. App processing json would have to
> handle this case in a special way.
>
Technically it is a u8. But really I do not understand why it is
RELOAD_FAILED and not RELOAD_STATUS which is more generic and re-usable.
e.g,. 'none', 'failed', 'success'.
^ permalink raw reply
* [PATCH] net: remove netx ethernet driver
From: Arnd Bergmann @ 2019-09-18 20:21 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, Uwe Kleine-König, Sascha Hauer, linux-kernel,
netdev
The ARM netx platform got removed in 5.3, so this driver
is now useless.
Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This fell through the cracks somewhere, I meant to send it as a separate
patch for net-next in v5.4, but only sent it as part of a series that
I mostly merged through the soc tree.
Thanks Uwe for the reminder!
---
drivers/net/ethernet/Kconfig | 11 -
drivers/net/ethernet/Makefile | 1 -
drivers/net/ethernet/netx-eth.c | 497 -------------------------
include/linux/platform_data/eth-netx.h | 13 -
4 files changed, 522 deletions(-)
delete mode 100644 drivers/net/ethernet/netx-eth.c
delete mode 100644 include/linux/platform_data/eth-netx.h
diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index 1e2de9d062bf..e8e9c166185d 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -140,17 +140,6 @@ source "drivers/net/ethernet/neterion/Kconfig"
source "drivers/net/ethernet/netronome/Kconfig"
source "drivers/net/ethernet/ni/Kconfig"
source "drivers/net/ethernet/8390/Kconfig"
-
-config NET_NETX
- tristate "NetX Ethernet support"
- select MII
- depends on ARCH_NETX
- ---help---
- This is support for the Hilscher netX builtin Ethernet ports
-
- To compile this driver as a module, choose M here. The module
- will be called netx-eth.
-
source "drivers/net/ethernet/nvidia/Kconfig"
source "drivers/net/ethernet/nxp/Kconfig"
source "drivers/net/ethernet/oki-semi/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 77f9838a76c9..05abebc17804 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -64,7 +64,6 @@ obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/
obj-$(CONFIG_NET_VENDOR_NETERION) += neterion/
obj-$(CONFIG_NET_VENDOR_NETRONOME) += netronome/
obj-$(CONFIG_NET_VENDOR_NI) += ni/
-obj-$(CONFIG_NET_NETX) += netx-eth.o
obj-$(CONFIG_NET_VENDOR_NVIDIA) += nvidia/
obj-$(CONFIG_LPC_ENET) += nxp/
obj-$(CONFIG_NET_VENDOR_OKI) += oki-semi/
diff --git a/drivers/net/ethernet/netx-eth.c b/drivers/net/ethernet/netx-eth.c
deleted file mode 100644
index cf6e7eb1b1e1..000000000000
--- a/drivers/net/ethernet/netx-eth.c
+++ /dev/null
@@ -1,497 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * drivers/net/ethernet/netx-eth.c
- *
- * Copyright (c) 2005 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
- */
-
-#include <linux/init.h>
-#include <linux/interrupt.h>
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/delay.h>
-
-#include <linux/netdevice.h>
-#include <linux/platform_device.h>
-#include <linux/etherdevice.h>
-#include <linux/skbuff.h>
-#include <linux/mii.h>
-
-#include <asm/io.h>
-#include <mach/hardware.h>
-#include <mach/netx-regs.h>
-#include <mach/pfifo.h>
-#include <mach/xc.h>
-#include <linux/platform_data/eth-netx.h>
-
-/* XC Fifo Offsets */
-#define EMPTY_PTR_FIFO(xcno) (0 + ((xcno) << 3)) /* Index of the empty pointer FIFO */
-#define IND_FIFO_PORT_HI(xcno) (1 + ((xcno) << 3)) /* Index of the FIFO where received */
- /* Data packages are indicated by XC */
-#define IND_FIFO_PORT_LO(xcno) (2 + ((xcno) << 3)) /* Index of the FIFO where received */
- /* Data packages are indicated by XC */
-#define REQ_FIFO_PORT_HI(xcno) (3 + ((xcno) << 3)) /* Index of the FIFO where Data packages */
- /* have to be indicated by ARM which */
- /* shall be sent */
-#define REQ_FIFO_PORT_LO(xcno) (4 + ((xcno) << 3)) /* Index of the FIFO where Data packages */
- /* have to be indicated by ARM which shall */
- /* be sent */
-#define CON_FIFO_PORT_HI(xcno) (5 + ((xcno) << 3)) /* Index of the FIFO where sent Data packages */
- /* are confirmed */
-#define CON_FIFO_PORT_LO(xcno) (6 + ((xcno) << 3)) /* Index of the FIFO where sent Data */
- /* packages are confirmed */
-#define PFIFO_MASK(xcno) (0x7f << (xcno*8))
-
-#define FIFO_PTR_FRAMELEN_SHIFT 0
-#define FIFO_PTR_FRAMELEN_MASK (0x7ff << 0)
-#define FIFO_PTR_FRAMELEN(len) (((len) << 0) & FIFO_PTR_FRAMELEN_MASK)
-#define FIFO_PTR_TIMETRIG (1<<11)
-#define FIFO_PTR_MULTI_REQ
-#define FIFO_PTR_ORIGIN (1<<14)
-#define FIFO_PTR_VLAN (1<<15)
-#define FIFO_PTR_FRAMENO_SHIFT 16
-#define FIFO_PTR_FRAMENO_MASK (0x3f << 16)
-#define FIFO_PTR_FRAMENO(no) (((no) << 16) & FIFO_PTR_FRAMENO_MASK)
-#define FIFO_PTR_SEGMENT_SHIFT 22
-#define FIFO_PTR_SEGMENT_MASK (0xf << 22)
-#define FIFO_PTR_SEGMENT(seg) (((seg) & 0xf) << 22)
-#define FIFO_PTR_ERROR_SHIFT 28
-#define FIFO_PTR_ERROR_MASK (0xf << 28)
-
-#define ISR_LINK_STATUS_CHANGE (1<<4)
-#define ISR_IND_LO (1<<3)
-#define ISR_CON_LO (1<<2)
-#define ISR_IND_HI (1<<1)
-#define ISR_CON_HI (1<<0)
-
-#define ETH_MAC_LOCAL_CONFIG 0x1560
-#define ETH_MAC_4321 0x1564
-#define ETH_MAC_65 0x1568
-
-#define MAC_TRAFFIC_CLASS_ARRANGEMENT_SHIFT 16
-#define MAC_TRAFFIC_CLASS_ARRANGEMENT_MASK (0xf<<MAC_TRAFFIC_CLASS_ARRANGEMENT_SHIFT)
-#define MAC_TRAFFIC_CLASS_ARRANGEMENT(x) (((x)<<MAC_TRAFFIC_CLASS_ARRANGEMENT_SHIFT) & MAC_TRAFFIC_CLASS_ARRANGEMENT_MASK)
-#define LOCAL_CONFIG_LINK_STATUS_IRQ_EN (1<<24)
-#define LOCAL_CONFIG_CON_LO_IRQ_EN (1<<23)
-#define LOCAL_CONFIG_CON_HI_IRQ_EN (1<<22)
-#define LOCAL_CONFIG_IND_LO_IRQ_EN (1<<21)
-#define LOCAL_CONFIG_IND_HI_IRQ_EN (1<<20)
-
-#define CARDNAME "netx-eth"
-
-/* LSB must be zero */
-#define INTERNAL_PHY_ADR 0x1c
-
-struct netx_eth_priv {
- void __iomem *sram_base, *xpec_base, *xmac_base;
- int id;
- struct mii_if_info mii;
- u32 msg_enable;
- struct xc *xc;
- spinlock_t lock;
-};
-
-static void netx_eth_set_multicast_list(struct net_device *ndev)
-{
- /* implement me */
-}
-
-static int
-netx_eth_hard_start_xmit(struct sk_buff *skb, struct net_device *ndev)
-{
- struct netx_eth_priv *priv = netdev_priv(ndev);
- unsigned char *buf = skb->data;
- unsigned int len = skb->len;
-
- spin_lock_irq(&priv->lock);
- memcpy_toio(priv->sram_base + 1560, (void *)buf, len);
- if (len < 60) {
- memset_io(priv->sram_base + 1560 + len, 0, 60 - len);
- len = 60;
- }
-
- pfifo_push(REQ_FIFO_PORT_LO(priv->id),
- FIFO_PTR_SEGMENT(priv->id) |
- FIFO_PTR_FRAMENO(1) |
- FIFO_PTR_FRAMELEN(len));
-
- ndev->stats.tx_packets++;
- ndev->stats.tx_bytes += skb->len;
-
- netif_stop_queue(ndev);
- spin_unlock_irq(&priv->lock);
- dev_kfree_skb(skb);
-
- return NETDEV_TX_OK;
-}
-
-static void netx_eth_receive(struct net_device *ndev)
-{
- struct netx_eth_priv *priv = netdev_priv(ndev);
- unsigned int val, frameno, seg, len;
- unsigned char *data;
- struct sk_buff *skb;
-
- val = pfifo_pop(IND_FIFO_PORT_LO(priv->id));
-
- frameno = (val & FIFO_PTR_FRAMENO_MASK) >> FIFO_PTR_FRAMENO_SHIFT;
- seg = (val & FIFO_PTR_SEGMENT_MASK) >> FIFO_PTR_SEGMENT_SHIFT;
- len = (val & FIFO_PTR_FRAMELEN_MASK) >> FIFO_PTR_FRAMELEN_SHIFT;
-
- skb = netdev_alloc_skb(ndev, len);
- if (unlikely(skb == NULL)) {
- ndev->stats.rx_dropped++;
- return;
- }
-
- data = skb_put(skb, len);
-
- memcpy_fromio(data, priv->sram_base + frameno * 1560, len);
-
- pfifo_push(EMPTY_PTR_FIFO(priv->id),
- FIFO_PTR_SEGMENT(seg) | FIFO_PTR_FRAMENO(frameno));
-
- skb->protocol = eth_type_trans(skb, ndev);
- netif_rx(skb);
- ndev->stats.rx_packets++;
- ndev->stats.rx_bytes += len;
-}
-
-static irqreturn_t
-netx_eth_interrupt(int irq, void *dev_id)
-{
- struct net_device *ndev = dev_id;
- struct netx_eth_priv *priv = netdev_priv(ndev);
- int status;
- unsigned long flags;
-
- spin_lock_irqsave(&priv->lock, flags);
-
- status = readl(NETX_PFIFO_XPEC_ISR(priv->id));
- while (status) {
- int fill_level;
- writel(status, NETX_PFIFO_XPEC_ISR(priv->id));
-
- if ((status & ISR_CON_HI) || (status & ISR_IND_HI))
- printk("%s: unexpected status: 0x%08x\n",
- __func__, status);
-
- fill_level =
- readl(NETX_PFIFO_FILL_LEVEL(IND_FIFO_PORT_LO(priv->id)));
- while (fill_level--)
- netx_eth_receive(ndev);
-
- if (status & ISR_CON_LO)
- netif_wake_queue(ndev);
-
- if (status & ISR_LINK_STATUS_CHANGE)
- mii_check_media(&priv->mii, netif_msg_link(priv), 1);
-
- status = readl(NETX_PFIFO_XPEC_ISR(priv->id));
- }
- spin_unlock_irqrestore(&priv->lock, flags);
- return IRQ_HANDLED;
-}
-
-static int netx_eth_open(struct net_device *ndev)
-{
- struct netx_eth_priv *priv = netdev_priv(ndev);
-
- if (request_irq
- (ndev->irq, netx_eth_interrupt, IRQF_SHARED, ndev->name, ndev))
- return -EAGAIN;
-
- writel(ndev->dev_addr[0] |
- ndev->dev_addr[1]<<8 |
- ndev->dev_addr[2]<<16 |
- ndev->dev_addr[3]<<24,
- priv->xpec_base + NETX_XPEC_RAM_START_OFS + ETH_MAC_4321);
- writel(ndev->dev_addr[4] |
- ndev->dev_addr[5]<<8,
- priv->xpec_base + NETX_XPEC_RAM_START_OFS + ETH_MAC_65);
-
- writel(LOCAL_CONFIG_LINK_STATUS_IRQ_EN |
- LOCAL_CONFIG_CON_LO_IRQ_EN |
- LOCAL_CONFIG_CON_HI_IRQ_EN |
- LOCAL_CONFIG_IND_LO_IRQ_EN |
- LOCAL_CONFIG_IND_HI_IRQ_EN,
- priv->xpec_base + NETX_XPEC_RAM_START_OFS +
- ETH_MAC_LOCAL_CONFIG);
-
- mii_check_media(&priv->mii, netif_msg_link(priv), 1);
- netif_start_queue(ndev);
-
- return 0;
-}
-
-static int netx_eth_close(struct net_device *ndev)
-{
- struct netx_eth_priv *priv = netdev_priv(ndev);
-
- netif_stop_queue(ndev);
-
- writel(0,
- priv->xpec_base + NETX_XPEC_RAM_START_OFS + ETH_MAC_LOCAL_CONFIG);
-
- free_irq(ndev->irq, ndev);
-
- return 0;
-}
-
-static void netx_eth_timeout(struct net_device *ndev)
-{
- struct netx_eth_priv *priv = netdev_priv(ndev);
- int i;
-
- printk(KERN_ERR "%s: transmit timed out, resetting\n", ndev->name);
-
- spin_lock_irq(&priv->lock);
-
- xc_reset(priv->xc);
- xc_start(priv->xc);
-
- for (i=2; i<=18; i++)
- pfifo_push(EMPTY_PTR_FIFO(priv->id),
- FIFO_PTR_FRAMENO(i) | FIFO_PTR_SEGMENT(priv->id));
-
- spin_unlock_irq(&priv->lock);
-
- netif_wake_queue(ndev);
-}
-
-static int
-netx_eth_phy_read(struct net_device *ndev, int phy_id, int reg)
-{
- unsigned int val;
-
- val = MIIMU_SNRDY | MIIMU_PREAMBLE | MIIMU_PHYADDR(phy_id) |
- MIIMU_REGADDR(reg) | MIIMU_PHY_NRES;
-
- writel(val, NETX_MIIMU);
- while (readl(NETX_MIIMU) & MIIMU_SNRDY);
-
- return readl(NETX_MIIMU) >> 16;
-
-}
-
-static void
-netx_eth_phy_write(struct net_device *ndev, int phy_id, int reg, int value)
-{
- unsigned int val;
-
- val = MIIMU_SNRDY | MIIMU_PREAMBLE | MIIMU_PHYADDR(phy_id) |
- MIIMU_REGADDR(reg) | MIIMU_PHY_NRES | MIIMU_OPMODE_WRITE |
- MIIMU_DATA(value);
-
- writel(val, NETX_MIIMU);
- while (readl(NETX_MIIMU) & MIIMU_SNRDY);
-}
-
-static const struct net_device_ops netx_eth_netdev_ops = {
- .ndo_open = netx_eth_open,
- .ndo_stop = netx_eth_close,
- .ndo_start_xmit = netx_eth_hard_start_xmit,
- .ndo_tx_timeout = netx_eth_timeout,
- .ndo_set_rx_mode = netx_eth_set_multicast_list,
- .ndo_validate_addr = eth_validate_addr,
- .ndo_set_mac_address = eth_mac_addr,
-};
-
-static int netx_eth_enable(struct net_device *ndev)
-{
- struct netx_eth_priv *priv = netdev_priv(ndev);
- unsigned int mac4321, mac65;
- int running, i, ret;
- bool inv_mac_addr = false;
-
- ndev->netdev_ops = &netx_eth_netdev_ops;
- ndev->watchdog_timeo = msecs_to_jiffies(5000);
-
- priv->msg_enable = NETIF_MSG_LINK;
- priv->mii.phy_id_mask = 0x1f;
- priv->mii.reg_num_mask = 0x1f;
- priv->mii.force_media = 0;
- priv->mii.full_duplex = 0;
- priv->mii.dev = ndev;
- priv->mii.mdio_read = netx_eth_phy_read;
- priv->mii.mdio_write = netx_eth_phy_write;
- priv->mii.phy_id = INTERNAL_PHY_ADR + priv->id;
-
- running = xc_running(priv->xc);
- xc_stop(priv->xc);
-
- /* if the xc engine is already running, assume the bootloader has
- * loaded the firmware for us
- */
- if (running) {
- /* get Node Address from hardware */
- mac4321 = readl(priv->xpec_base +
- NETX_XPEC_RAM_START_OFS + ETH_MAC_4321);
- mac65 = readl(priv->xpec_base +
- NETX_XPEC_RAM_START_OFS + ETH_MAC_65);
-
- ndev->dev_addr[0] = mac4321 & 0xff;
- ndev->dev_addr[1] = (mac4321 >> 8) & 0xff;
- ndev->dev_addr[2] = (mac4321 >> 16) & 0xff;
- ndev->dev_addr[3] = (mac4321 >> 24) & 0xff;
- ndev->dev_addr[4] = mac65 & 0xff;
- ndev->dev_addr[5] = (mac65 >> 8) & 0xff;
- } else {
- if (xc_request_firmware(priv->xc)) {
- printk(CARDNAME ": requesting firmware failed\n");
- return -ENODEV;
- }
- }
-
- xc_reset(priv->xc);
- xc_start(priv->xc);
-
- if (!is_valid_ether_addr(ndev->dev_addr))
- inv_mac_addr = true;
-
- for (i=2; i<=18; i++)
- pfifo_push(EMPTY_PTR_FIFO(priv->id),
- FIFO_PTR_FRAMENO(i) | FIFO_PTR_SEGMENT(priv->id));
-
- ret = register_netdev(ndev);
- if (inv_mac_addr)
- printk("%s: Invalid ethernet MAC address. Please set using ip\n",
- ndev->name);
-
- return ret;
-}
-
-static int netx_eth_drv_probe(struct platform_device *pdev)
-{
- struct netx_eth_priv *priv;
- struct net_device *ndev;
- struct netxeth_platform_data *pdata;
- int ret;
-
- ndev = alloc_etherdev(sizeof (struct netx_eth_priv));
- if (!ndev) {
- ret = -ENOMEM;
- goto exit;
- }
- SET_NETDEV_DEV(ndev, &pdev->dev);
-
- platform_set_drvdata(pdev, ndev);
-
- priv = netdev_priv(ndev);
-
- pdata = dev_get_platdata(&pdev->dev);
- priv->xc = request_xc(pdata->xcno, &pdev->dev);
- if (!priv->xc) {
- dev_err(&pdev->dev, "unable to request xc engine\n");
- ret = -ENODEV;
- goto exit_free_netdev;
- }
-
- ndev->irq = priv->xc->irq;
- priv->id = pdev->id;
- priv->xpec_base = priv->xc->xpec_base;
- priv->xmac_base = priv->xc->xmac_base;
- priv->sram_base = priv->xc->sram_base;
-
- spin_lock_init(&priv->lock);
-
- ret = pfifo_request(PFIFO_MASK(priv->id));
- if (ret) {
- printk("unable to request PFIFO\n");
- goto exit_free_xc;
- }
-
- ret = netx_eth_enable(ndev);
- if (ret)
- goto exit_free_pfifo;
-
- return 0;
-exit_free_pfifo:
- pfifo_free(PFIFO_MASK(priv->id));
-exit_free_xc:
- free_xc(priv->xc);
-exit_free_netdev:
- free_netdev(ndev);
-exit:
- return ret;
-}
-
-static int netx_eth_drv_remove(struct platform_device *pdev)
-{
- struct net_device *ndev = platform_get_drvdata(pdev);
- struct netx_eth_priv *priv = netdev_priv(ndev);
-
- unregister_netdev(ndev);
- xc_stop(priv->xc);
- free_xc(priv->xc);
- free_netdev(ndev);
- pfifo_free(PFIFO_MASK(priv->id));
-
- return 0;
-}
-
-static int netx_eth_drv_suspend(struct platform_device *pdev, pm_message_t state)
-{
- dev_err(&pdev->dev, "suspend not implemented\n");
- return 0;
-}
-
-static int netx_eth_drv_resume(struct platform_device *pdev)
-{
- dev_err(&pdev->dev, "resume not implemented\n");
- return 0;
-}
-
-static struct platform_driver netx_eth_driver = {
- .probe = netx_eth_drv_probe,
- .remove = netx_eth_drv_remove,
- .suspend = netx_eth_drv_suspend,
- .resume = netx_eth_drv_resume,
- .driver = {
- .name = CARDNAME,
- },
-};
-
-static int __init netx_eth_init(void)
-{
- unsigned int phy_control, val;
-
- printk("NetX Ethernet driver\n");
-
- phy_control = PHY_CONTROL_PHY_ADDRESS(INTERNAL_PHY_ADR>>1) |
- PHY_CONTROL_PHY1_MODE(PHY_MODE_ALL) |
- PHY_CONTROL_PHY1_AUTOMDIX |
- PHY_CONTROL_PHY1_EN |
- PHY_CONTROL_PHY0_MODE(PHY_MODE_ALL) |
- PHY_CONTROL_PHY0_AUTOMDIX |
- PHY_CONTROL_PHY0_EN |
- PHY_CONTROL_CLK_XLATIN;
-
- val = readl(NETX_SYSTEM_IOC_ACCESS_KEY);
- writel(val, NETX_SYSTEM_IOC_ACCESS_KEY);
-
- writel(phy_control | PHY_CONTROL_RESET, NETX_SYSTEM_PHY_CONTROL);
- udelay(100);
-
- val = readl(NETX_SYSTEM_IOC_ACCESS_KEY);
- writel(val, NETX_SYSTEM_IOC_ACCESS_KEY);
-
- writel(phy_control, NETX_SYSTEM_PHY_CONTROL);
-
- return platform_driver_register(&netx_eth_driver);
-}
-
-static void __exit netx_eth_cleanup(void)
-{
- platform_driver_unregister(&netx_eth_driver);
-}
-
-module_init(netx_eth_init);
-module_exit(netx_eth_cleanup);
-
-MODULE_AUTHOR("Sascha Hauer, Pengutronix");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:" CARDNAME);
-MODULE_FIRMWARE("xc0.bin");
-MODULE_FIRMWARE("xc1.bin");
-MODULE_FIRMWARE("xc2.bin");
diff --git a/include/linux/platform_data/eth-netx.h b/include/linux/platform_data/eth-netx.h
deleted file mode 100644
index a3a6322668d8..000000000000
--- a/include/linux/platform_data/eth-netx.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (c) 2005 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
- */
-
-#ifndef __ETH_NETX_H
-#define __ETH_NETX_H
-
-struct netxeth_platform_data {
- unsigned int xcno; /* number of xmac/xpec engine this eth uses */
-};
-
-#endif
--
2.20.0
^ permalink raw reply related
* Re: [patch iproute2-next v2] devlink: add reload failed indication
From: Jiri Pirko @ 2019-09-18 20:23 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, stephen, idosch, jakub.kicinski, tariqt, mlxsw
In-Reply-To: <13688c37-3f27-bdb4-973b-dd73031fa230@gmail.com>
Wed, Sep 18, 2019 at 10:01:31PM CEST, dsahern@gmail.com wrote:
>On 9/18/19 1:37 AM, Jiri Pirko wrote:
>> Wed, Sep 18, 2019 at 01:46:13AM CEST, dsahern@gmail.com wrote:
>>> On 9/17/19 12:36 PM, Jiri Pirko wrote:
>>>> Tue, Sep 17, 2019 at 06:46:31PM CEST, dsahern@gmail.com wrote:
>>>>> On 9/16/19 3:44 AM, Jiri Pirko wrote:
>>>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>>>
>>>>>> Add indication about previous failed devlink reload.
>>>>>>
>>>>>> Example outputs:
>>>>>>
>>>>>> $ devlink dev
>>>>>> netdevsim/netdevsim10: reload_failed true
>>>>>
>>>>> odd output to user. Why not just "reload failed"?
>>>>
>>>> Well it is common to have "name value". The extra space would seem
>>>> confusing for the reader..
>>>> Also it is common to have "_" instead of space for the output in cases
>>>> like this.
>>>>
>>>
>>> I am not understanding your point.
>>>
>>> "reload failed" is still a name/value pair. It is short and to the point
>>> as to what it indicates. There is no need for the name in the uapi (ie.,
>>> the name of the netlink attribute) to be dumped here.
>>
>> Ah, got it. Well it is a bool value, that means it is "true" or "false".
>> In json output, it is True of False. App processing json would have to
>> handle this case in a special way.
>>
>
>Technically it is a u8. But really I do not understand why it is
>RELOAD_FAILED and not RELOAD_STATUS which is more generic and re-usable.
>e.g,. 'none', 'failed', 'success'.
I was thinking about that. But I was not able to figure out any other
possible values. So it is bool. For indication of some other status,
there would have to be independent bool/othertype anyway.
^ permalink raw reply
* [PATCH 2/2] ptp: Add a ptp clock driver for IDT ClockMatrix.
From: vincent.cheng.xh @ 2019-09-18 20:06 UTC (permalink / raw)
To: robh+dt, mark.rutland, richardcochran
Cc: devicetree, netdev, linux-kernel, Vincent Cheng
In-Reply-To: <1568837198-27211-1-git-send-email-vincent.cheng.xh@renesas.com>
From: Vincent Cheng <vincent.cheng.xh@renesas.com>
The IDT ClockMatrix (TM) family includes integrated devices that provide
eight PLL channels. Each PLL channel can be independently configured as a
frequency synthesizer, jitter attenuator, digitally controlled
oscillator (DCO), or a digital phase lock loop (DPLL). Typically
these devices are used as timing references and clock sources for PTP
applications. This patch adds support for the device.
Co-developed-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Vincent Cheng <vincent.cheng.xh@renesas.com>
---
drivers/ptp/Kconfig | 12 +
drivers/ptp/Makefile | 1 +
drivers/ptp/idt8a340_reg.h | 659 ++++++++++++++++++++
drivers/ptp/ptp_clockmatrix.c | 1384 +++++++++++++++++++++++++++++++++++++++++
drivers/ptp/ptp_clockmatrix.h | 123 ++++
5 files changed, 2179 insertions(+)
create mode 100644 drivers/ptp/idt8a340_reg.h
create mode 100644 drivers/ptp/ptp_clockmatrix.c
create mode 100644 drivers/ptp/ptp_clockmatrix.h
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 960961f..16c7c90 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -119,4 +119,16 @@ config PTP_1588_CLOCK_KVM
To compile this driver as a module, choose M here: the module
will be called ptp_kvm.
+config PTP_1588_CLOCK_IDTCM
+ tristate "IDT CLOCKMATRIX as PTP clock"
+ select PTP_1588_CLOCK
+ default n
+ help
+ This driver adds support for using IDT CLOCKMATRIX(TM) as a PTP
+ clock. This clock is only useful if your time stamping MAC
+ is connected to the IDT chip.
+
+ To compile this driver as a module, choose M here: the module
+ will be called ptp_clockmatrix.
+
endmenu
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index 677d1d1..9791ba9 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_PTP_1588_CLOCK_KVM) += ptp_kvm.o
obj-$(CONFIG_PTP_1588_CLOCK_QORIQ) += ptp-qoriq.o
ptp-qoriq-y += ptp_qoriq.o
ptp-qoriq-$(CONFIG_DEBUG_FS) += ptp_qoriq_debugfs.o
+obj-$(CONFIG_PTP_1588_CLOCK_IDTCM) += ptp_clockmatrix.o
diff --git a/drivers/ptp/idt8a340_reg.h b/drivers/ptp/idt8a340_reg.h
new file mode 100644
index 0000000..9263bc3
--- /dev/null
+++ b/drivers/ptp/idt8a340_reg.h
@@ -0,0 +1,659 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* idt8a340_reg.h
+ *
+ * Originally generated by regen.tcl on Thu Feb 14 19:23:44 PST 2019
+ * https://github.com/richardcochran/regen
+ *
+ * Hand modified to include some HW registers.
+ * Based on 4.8.0, SCSR rev C commit a03c7ae5
+ */
+#ifndef HAVE_IDT8A340_REG
+#define HAVE_IDT8A340_REG
+
+#define PAGE_ADDR_BASE 0x0000
+#define PAGE_ADDR 0x00fc
+
+#define HW_REVISION 0x8180
+#define REV_ID 0x007a
+
+#define HW_DPLL_0 (0x8a00)
+#define HW_DPLL_1 (0x8b00)
+#define HW_DPLL_2 (0x8c00)
+#define HW_DPLL_3 (0x8d00)
+
+#define HW_DPLL_TOD_SW_TRIG_ADDR__0 (0x080)
+#define HW_DPLL_TOD_CTRL_1 (0x089)
+#define HW_DPLL_TOD_CTRL_2 (0x08A)
+#define HW_DPLL_TOD_OVR__0 (0x098)
+#define HW_DPLL_TOD_OUT_0__0 (0x0B0)
+
+#define HW_Q0_Q1_CH_SYNC_CTRL_0 (0xa740)
+#define HW_Q0_Q1_CH_SYNC_CTRL_1 (0xa741)
+#define HW_Q2_Q3_CH_SYNC_CTRL_0 (0xa742)
+#define HW_Q2_Q3_CH_SYNC_CTRL_1 (0xa743)
+#define HW_Q4_Q5_CH_SYNC_CTRL_0 (0xa744)
+#define HW_Q4_Q5_CH_SYNC_CTRL_1 (0xa745)
+#define HW_Q6_Q7_CH_SYNC_CTRL_0 (0xa746)
+#define HW_Q6_Q7_CH_SYNC_CTRL_1 (0xa747)
+#define HW_Q8_CH_SYNC_CTRL_0 (0xa748)
+#define HW_Q8_CH_SYNC_CTRL_1 (0xa749)
+#define HW_Q9_CH_SYNC_CTRL_0 (0xa74a)
+#define HW_Q9_CH_SYNC_CTRL_1 (0xa74b)
+#define HW_Q10_CH_SYNC_CTRL_0 (0xa74c)
+#define HW_Q10_CH_SYNC_CTRL_1 (0xa74d)
+#define HW_Q11_CH_SYNC_CTRL_0 (0xa74e)
+#define HW_Q11_CH_SYNC_CTRL_1 (0xa74f)
+
+#define SYNC_SOURCE_DPLL0_TOD_PPS 0x14
+#define SYNC_SOURCE_DPLL1_TOD_PPS 0x15
+#define SYNC_SOURCE_DPLL2_TOD_PPS 0x16
+#define SYNC_SOURCE_DPLL3_TOD_PPS 0x17
+
+#define SYNCTRL1_MASTER_SYNC_RST BIT(7)
+#define SYNCTRL1_MASTER_SYNC_TRIG BIT(5)
+#define SYNCTRL1_TOD_SYNC_TRIG BIT(4)
+#define SYNCTRL1_FBDIV_FRAME_SYNC_TRIG BIT(3)
+#define SYNCTRL1_FBDIV_SYNC_TRIG BIT(2)
+#define SYNCTRL1_Q1_DIV_SYNC_TRIG BIT(1)
+#define SYNCTRL1_Q0_DIV_SYNC_TRIG BIT(0)
+
+#define RESET_CTRL 0xc000
+#define SM_RESET 0x0012
+#define SM_RESET_CMD 0x5A
+
+#define GENERAL_STATUS 0xc014
+#define HW_REV_ID 0x000A
+#define BOND_ID 0x000B
+#define HW_CSR_ID 0x000C
+#define HW_IRQ_ID 0x000E
+
+#define MAJ_REL 0x0010
+#define MIN_REL 0x0011
+#define HOTFIX_REL 0x0012
+
+#define PIPELINE_ID 0x0014
+#define BUILD_ID 0x0018
+
+#define JTAG_DEVICE_ID 0x001c
+#define PRODUCT_ID 0x001e
+
+#define STATUS 0xc03c
+#define USER_GPIO0_TO_7_STATUS 0x008a
+#define USER_GPIO8_TO_15_STATUS 0x008b
+
+#define GPIO_USER_CONTROL 0xc160
+#define GPIO0_TO_7_OUT 0x0000
+#define GPIO8_TO_15_OUT 0x0001
+
+#define STICKY_STATUS_CLEAR 0xc164
+
+#define GPIO_TOD_NOTIFICATION_CLEAR 0xc16c
+
+#define ALERT_CFG 0xc188
+
+#define SYS_DPLL_XO 0xc194
+
+#define SYS_APLL 0xc19c
+
+#define INPUT_0 0xc1b0
+
+#define INPUT_1 0xc1c0
+
+#define INPUT_2 0xc1d0
+
+#define INPUT_3 0xc200
+
+#define INPUT_4 0xc210
+
+#define INPUT_5 0xc220
+
+#define INPUT_6 0xc230
+
+#define INPUT_7 0xc240
+
+#define INPUT_8 0xc250
+
+#define INPUT_9 0xc260
+
+#define INPUT_10 0xc280
+
+#define INPUT_11 0xc290
+
+#define INPUT_12 0xc2a0
+
+#define INPUT_13 0xc2b0
+
+#define INPUT_14 0xc2c0
+
+#define INPUT_15 0xc2d0
+
+#define REF_MON_0 0xc2e0
+
+#define REF_MON_1 0xc2ec
+
+#define REF_MON_2 0xc300
+
+#define REF_MON_3 0xc30c
+
+#define REF_MON_4 0xc318
+
+#define REF_MON_5 0xc324
+
+#define REF_MON_6 0xc330
+
+#define REF_MON_7 0xc33c
+
+#define REF_MON_8 0xc348
+
+#define REF_MON_9 0xc354
+
+#define REF_MON_10 0xc360
+
+#define REF_MON_11 0xc36c
+
+#define REF_MON_12 0xc380
+
+#define REF_MON_13 0xc38c
+
+#define REF_MON_14 0xc398
+
+#define REF_MON_15 0xc3a4
+
+#define DPLL_0 0xc3b0
+#define DPLL_CTRL_REG_0 0x0002
+#define DPLL_CTRL_REG_1 0x0003
+#define DPLL_CTRL_REG_2 0x0004
+#define DPLL_TOD_SYNC_CFG 0x0031
+#define DPLL_COMBO_SLAVE_CFG_0 0x0032
+#define DPLL_COMBO_SLAVE_CFG_1 0x0033
+#define DPLL_SLAVE_REF_CFG 0x0034
+#define DPLL_REF_MODE 0x0035
+#define DPLL_PHASE_MEASUREMENT_CFG 0x0036
+#define DPLL_MODE 0x0037
+
+#define DPLL_1 0xc400
+
+#define DPLL_2 0xc438
+
+#define DPLL_3 0xc480
+
+#define DPLL_4 0xc4b8
+
+#define DPLL_5 0xc500
+
+#define DPLL_6 0xc538
+
+#define DPLL_7 0xc580
+
+#define SYS_DPLL 0xc5b8
+
+#define DPLL_CTRL_0 0xc600
+#define DPLL_CTRL_DPLL_MANU_REF_CFG 0x0001
+
+#define DPLL_CTRL_1 0xc63c
+
+#define DPLL_CTRL_2 0xc680
+
+#define DPLL_CTRL_3 0xc6bc
+
+#define DPLL_CTRL_4 0xc700
+
+#define DPLL_CTRL_5 0xc73c
+
+#define DPLL_CTRL_6 0xc780
+
+#define DPLL_CTRL_7 0xc7bc
+
+#define SYS_DPLL_CTRL 0xc800
+
+#define DPLL_PHASE_0 0xc818
+
+/* Signed 42-bit FFO in units of 2^(-53) */
+#define DPLL_WR_PHASE 0x0000
+
+#define DPLL_PHASE_1 0xc81c
+
+#define DPLL_PHASE_2 0xc820
+
+#define DPLL_PHASE_3 0xc824
+
+#define DPLL_PHASE_4 0xc828
+
+#define DPLL_PHASE_5 0xc82c
+
+#define DPLL_PHASE_6 0xc830
+
+#define DPLL_PHASE_7 0xc834
+
+#define DPLL_FREQ_0 0xc838
+
+/* Signed 42-bit FFO in units of 2^(-53) */
+#define DPLL_WR_FREQ 0x0000
+
+#define DPLL_FREQ_1 0xc840
+
+#define DPLL_FREQ_2 0xc848
+
+#define DPLL_FREQ_3 0xc850
+
+#define DPLL_FREQ_4 0xc858
+
+#define DPLL_FREQ_5 0xc860
+
+#define DPLL_FREQ_6 0xc868
+
+#define DPLL_FREQ_7 0xc870
+
+#define DPLL_PHASE_PULL_IN_0 0xc880
+#define PULL_IN_OFFSET 0x0000 /* Signed 32 bit */
+#define PULL_IN_SLOPE_LIMIT 0x0004 /* Unsigned 24 bit */
+#define PULL_IN_CTRL 0x0007
+
+#define DPLL_PHASE_PULL_IN_1 0xc888
+
+#define DPLL_PHASE_PULL_IN_2 0xc890
+
+#define DPLL_PHASE_PULL_IN_3 0xc898
+
+#define DPLL_PHASE_PULL_IN_4 0xc8a0
+
+#define DPLL_PHASE_PULL_IN_5 0xc8a8
+
+#define DPLL_PHASE_PULL_IN_6 0xc8b0
+
+#define DPLL_PHASE_PULL_IN_7 0xc8b8
+
+#define GPIO_CFG 0xc8c0
+#define GPIO_CFG_GBL 0x0000
+
+#define GPIO_0 0xc8c2
+#define GPIO_DCO_INC_DEC 0x0000
+#define GPIO_OUT_CTRL_0 0x0001
+#define GPIO_OUT_CTRL_1 0x0002
+#define GPIO_TOD_TRIG 0x0003
+#define GPIO_DPLL_INDICATOR 0x0004
+#define GPIO_LOS_INDICATOR 0x0005
+#define GPIO_REF_INPUT_DSQ_0 0x0006
+#define GPIO_REF_INPUT_DSQ_1 0x0007
+#define GPIO_REF_INPUT_DSQ_2 0x0008
+#define GPIO_REF_INPUT_DSQ_3 0x0009
+#define GPIO_MAN_CLK_SEL_0 0x000a
+#define GPIO_MAN_CLK_SEL_1 0x000b
+#define GPIO_MAN_CLK_SEL_2 0x000c
+#define GPIO_SLAVE 0x000d
+#define GPIO_ALERT_OUT_CFG 0x000e
+#define GPIO_TOD_NOTIFICATION_CFG 0x000f
+#define GPIO_CTRL 0x0010
+
+#define GPIO_1 0xc8d4
+
+#define GPIO_2 0xc8e6
+
+#define GPIO_3 0xc900
+
+#define GPIO_4 0xc912
+
+#define GPIO_5 0xc924
+
+#define GPIO_6 0xc936
+
+#define GPIO_7 0xc948
+
+#define GPIO_8 0xc95a
+
+#define GPIO_9 0xc980
+
+#define GPIO_10 0xc992
+
+#define GPIO_11 0xc9a4
+
+#define GPIO_12 0xc9b6
+
+#define GPIO_13 0xc9c8
+
+#define GPIO_14 0xc9da
+
+#define GPIO_15 0xca00
+
+#define OUT_DIV_MUX 0xca12
+
+#define OUTPUT_0 0xca14
+/* FOD frequency output divider value */
+#define OUT_DIV 0x0000
+#define OUT_DUTY_CYCLE_HIGH 0x0004
+#define OUT_CTRL_0 0x0008
+#define OUT_CTRL_1 0x0009
+/* Phase adjustment in FOD cycles */
+#define OUT_PHASE_ADJ 0x000c
+
+#define OUTPUT_1 0xca24
+
+#define OUTPUT_2 0xca34
+
+#define OUTPUT_3 0xca44
+
+#define OUTPUT_4 0xca54
+
+#define OUTPUT_5 0xca64
+
+#define OUTPUT_6 0xca80
+
+#define OUTPUT_7 0xca90
+
+#define OUTPUT_8 0xcaa0
+
+#define OUTPUT_9 0xcab0
+
+#define OUTPUT_10 0xcac0
+
+#define OUTPUT_11 0xcad0
+
+#define SERIAL 0xcae0
+
+#define PWM_ENCODER_0 0xcb00
+
+#define PWM_ENCODER_1 0xcb08
+
+#define PWM_ENCODER_2 0xcb10
+
+#define PWM_ENCODER_3 0xcb18
+
+#define PWM_ENCODER_4 0xcb20
+
+#define PWM_ENCODER_5 0xcb28
+
+#define PWM_ENCODER_6 0xcb30
+
+#define PWM_ENCODER_7 0xcb38
+
+#define PWM_DECODER_0 0xcb40
+
+#define PWM_DECODER_1 0xcb48
+
+#define PWM_DECODER_2 0xcb50
+
+#define PWM_DECODER_3 0xcb58
+
+#define PWM_DECODER_4 0xcb60
+
+#define PWM_DECODER_5 0xcb68
+
+#define PWM_DECODER_6 0xcb70
+
+#define PWM_DECODER_7 0xcb80
+
+#define PWM_DECODER_8 0xcb88
+
+#define PWM_DECODER_9 0xcb90
+
+#define PWM_DECODER_10 0xcb98
+
+#define PWM_DECODER_11 0xcba0
+
+#define PWM_DECODER_12 0xcba8
+
+#define PWM_DECODER_13 0xcbb0
+
+#define PWM_DECODER_14 0xcbb8
+
+#define PWM_DECODER_15 0xcbc0
+
+#define PWM_USER_DATA 0xcbc8
+
+#define TOD_0 0xcbcc
+
+/* Enable TOD counter, output channel sync and even-PPS mode */
+#define TOD_CFG 0x0000
+
+#define TOD_1 0xcbce
+
+#define TOD_2 0xcbd0
+
+#define TOD_3 0xcbd2
+
+
+#define TOD_WRITE_0 0xcc00
+/* 8-bit subns, 32-bit ns, 48-bit seconds */
+#define TOD_WRITE 0x0000
+/* Counter increments after TOD write is completed */
+#define TOD_WRITE_COUNTER 0x000c
+/* TOD write trigger configuration */
+#define TOD_WRITE_SELECT_CFG_0 0x000d
+/* TOD write trigger selection */
+#define TOD_WRITE_CMD 0x000f
+
+#define TOD_WRITE_1 0xcc10
+
+#define TOD_WRITE_2 0xcc20
+
+#define TOD_WRITE_3 0xcc30
+
+#define TOD_READ_PRIMARY_0 0xcc40
+/* 8-bit subns, 32-bit ns, 48-bit seconds */
+#define TOD_READ_PRIMARY 0x0000
+/* Counter increments after TOD write is completed */
+#define TOD_READ_PRIMARY_COUNTER 0x000b
+/* Read trigger configuration */
+#define TOD_READ_PRIMARY_SEL_CFG_0 0x000c
+/* Read trigger selection */
+#define TOD_READ_PRIMARY_CMD 0x000e
+
+#define TOD_READ_PRIMARY_1 0xcc50
+
+#define TOD_READ_PRIMARY_2 0xcc60
+
+#define TOD_READ_PRIMARY_3 0xcc80
+
+#define TOD_READ_SECONDARY_0 0xcc90
+
+#define TOD_READ_SECONDARY_1 0xcca0
+
+#define TOD_READ_SECONDARY_2 0xccb0
+
+#define TOD_READ_SECONDARY_3 0xccc0
+
+#define OUTPUT_TDC_CFG 0xccd0
+
+#define OUTPUT_TDC_0 0xcd00
+
+#define OUTPUT_TDC_1 0xcd08
+
+#define OUTPUT_TDC_2 0xcd10
+
+#define OUTPUT_TDC_3 0xcd18
+
+#define INPUT_TDC 0xcd20
+
+#define SCRATCH 0xcf50
+
+#define EEPROM 0xcf68
+
+#define OTP 0xcf70
+
+#define BYTE 0xcf80
+
+/* Bit definitions for the MAJ_REL register */
+#define MAJOR_SHIFT (1)
+#define MAJOR_MASK (0x7f)
+#define PR_BUILD BIT(0)
+
+/* Bit definitions for the USER_GPIO0_TO_7_STATUS register */
+#define GPIO0_LEVEL BIT(0)
+#define GPIO1_LEVEL BIT(1)
+#define GPIO2_LEVEL BIT(2)
+#define GPIO3_LEVEL BIT(3)
+#define GPIO4_LEVEL BIT(4)
+#define GPIO5_LEVEL BIT(5)
+#define GPIO6_LEVEL BIT(6)
+#define GPIO7_LEVEL BIT(7)
+
+/* Bit definitions for the USER_GPIO8_TO_15_STATUS register */
+#define GPIO8_LEVEL BIT(0)
+#define GPIO9_LEVEL BIT(1)
+#define GPIO10_LEVEL BIT(2)
+#define GPIO11_LEVEL BIT(3)
+#define GPIO12_LEVEL BIT(4)
+#define GPIO13_LEVEL BIT(5)
+#define GPIO14_LEVEL BIT(6)
+#define GPIO15_LEVEL BIT(7)
+
+/* Bit definitions for the GPIO0_TO_7_OUT register */
+#define GPIO0_DRIVE_LEVEL BIT(0)
+#define GPIO1_DRIVE_LEVEL BIT(1)
+#define GPIO2_DRIVE_LEVEL BIT(2)
+#define GPIO3_DRIVE_LEVEL BIT(3)
+#define GPIO4_DRIVE_LEVEL BIT(4)
+#define GPIO5_DRIVE_LEVEL BIT(5)
+#define GPIO6_DRIVE_LEVEL BIT(6)
+#define GPIO7_DRIVE_LEVEL BIT(7)
+
+/* Bit definitions for the GPIO8_TO_15_OUT register */
+#define GPIO8_DRIVE_LEVEL BIT(0)
+#define GPIO9_DRIVE_LEVEL BIT(1)
+#define GPIO10_DRIVE_LEVEL BIT(2)
+#define GPIO11_DRIVE_LEVEL BIT(3)
+#define GPIO12_DRIVE_LEVEL BIT(4)
+#define GPIO13_DRIVE_LEVEL BIT(5)
+#define GPIO14_DRIVE_LEVEL BIT(6)
+#define GPIO15_DRIVE_LEVEL BIT(7)
+
+/* Bit definitions for the DPLL_TOD_SYNC_CFG register */
+#define TOD_SYNC_SOURCE_SHIFT (1)
+#define TOD_SYNC_SOURCE_MASK (0x3)
+#define TOD_SYNC_EN BIT(0)
+
+/* Bit definitions for the DPLL_MODE register */
+#define WRITE_TIMER_MODE BIT(6)
+#define PLL_MODE_SHIFT (3)
+#define PLL_MODE_MASK (0x7)
+#define STATE_MODE_SHIFT (0)
+#define STATE_MODE_MASK (0x7)
+
+/* Bit definitions for the GPIO_CFG_GBL register */
+#define SUPPLY_MODE_SHIFT (0)
+#define SUPPLY_MODE_MASK (0x3)
+
+/* Bit definitions for the GPIO_DCO_INC_DEC register */
+#define INCDEC_DPLL_INDEX_SHIFT (0)
+#define INCDEC_DPLL_INDEX_MASK (0x7)
+
+/* Bit definitions for the GPIO_OUT_CTRL_0 register */
+#define CTRL_OUT_0 BIT(0)
+#define CTRL_OUT_1 BIT(1)
+#define CTRL_OUT_2 BIT(2)
+#define CTRL_OUT_3 BIT(3)
+#define CTRL_OUT_4 BIT(4)
+#define CTRL_OUT_5 BIT(5)
+#define CTRL_OUT_6 BIT(6)
+#define CTRL_OUT_7 BIT(7)
+
+/* Bit definitions for the GPIO_OUT_CTRL_1 register */
+#define CTRL_OUT_8 BIT(0)
+#define CTRL_OUT_9 BIT(1)
+#define CTRL_OUT_10 BIT(2)
+#define CTRL_OUT_11 BIT(3)
+#define CTRL_OUT_12 BIT(4)
+#define CTRL_OUT_13 BIT(5)
+#define CTRL_OUT_14 BIT(6)
+#define CTRL_OUT_15 BIT(7)
+
+/* Bit definitions for the GPIO_TOD_TRIG register */
+#define TOD_TRIG_0 BIT(0)
+#define TOD_TRIG_1 BIT(1)
+#define TOD_TRIG_2 BIT(2)
+#define TOD_TRIG_3 BIT(3)
+
+/* Bit definitions for the GPIO_DPLL_INDICATOR register */
+#define IND_DPLL_INDEX_SHIFT (0)
+#define IND_DPLL_INDEX_MASK (0x7)
+
+/* Bit definitions for the GPIO_LOS_INDICATOR register */
+#define REFMON_INDEX_SHIFT (0)
+#define REFMON_INDEX_MASK (0xf)
+/* Active level of LOS indicator, 0=low 1=high */
+#define ACTIVE_LEVEL BIT(4)
+
+/* Bit definitions for the GPIO_REF_INPUT_DSQ_0 register */
+#define DSQ_INP_0 BIT(0)
+#define DSQ_INP_1 BIT(1)
+#define DSQ_INP_2 BIT(2)
+#define DSQ_INP_3 BIT(3)
+#define DSQ_INP_4 BIT(4)
+#define DSQ_INP_5 BIT(5)
+#define DSQ_INP_6 BIT(6)
+#define DSQ_INP_7 BIT(7)
+
+/* Bit definitions for the GPIO_REF_INPUT_DSQ_1 register */
+#define DSQ_INP_8 BIT(0)
+#define DSQ_INP_9 BIT(1)
+#define DSQ_INP_10 BIT(2)
+#define DSQ_INP_11 BIT(3)
+#define DSQ_INP_12 BIT(4)
+#define DSQ_INP_13 BIT(5)
+#define DSQ_INP_14 BIT(6)
+#define DSQ_INP_15 BIT(7)
+
+/* Bit definitions for the GPIO_REF_INPUT_DSQ_2 register */
+#define DSQ_DPLL_0 BIT(0)
+#define DSQ_DPLL_1 BIT(1)
+#define DSQ_DPLL_2 BIT(2)
+#define DSQ_DPLL_3 BIT(3)
+#define DSQ_DPLL_4 BIT(4)
+#define DSQ_DPLL_5 BIT(5)
+#define DSQ_DPLL_6 BIT(6)
+#define DSQ_DPLL_7 BIT(7)
+
+/* Bit definitions for the GPIO_REF_INPUT_DSQ_3 register */
+#define DSQ_DPLL_SYS BIT(0)
+#define GPIO_DSQ_LEVEL BIT(1)
+
+/* Bit definitions for the GPIO_TOD_NOTIFICATION_CFG register */
+#define DPLL_TOD_SHIFT (0)
+#define DPLL_TOD_MASK (0x3)
+#define TOD_READ_SECONDARY BIT(2)
+#define GPIO_ASSERT_LEVEL BIT(3)
+
+/* Bit definitions for the GPIO_CTRL register */
+#define GPIO_FUNCTION_EN BIT(0)
+#define GPIO_CMOS_OD_MODE BIT(1)
+#define GPIO_CONTROL_DIR BIT(2)
+#define GPIO_PU_PD_MODE BIT(3)
+#define GPIO_FUNCTION_SHIFT (4)
+#define GPIO_FUNCTION_MASK (0xf)
+
+/* Bit definitions for the OUT_CTRL_1 register */
+#define OUT_SYNC_DISABLE BIT(7)
+#define SQUELCH_VALUE BIT(6)
+#define SQUELCH_DISABLE BIT(5)
+#define PAD_VDDO_SHIFT (2)
+#define PAD_VDDO_MASK (0x7)
+#define PAD_CMOSDRV_SHIFT (0)
+#define PAD_CMOSDRV_MASK (0x3)
+
+/* Bit definitions for the TOD_CFG register */
+#define TOD_EVEN_PPS_MODE BIT(2)
+#define TOD_OUT_SYNC_ENABLE BIT(1)
+#define TOD_ENABLE BIT(0)
+
+/* Bit definitions for the TOD_WRITE_SELECT_CFG_0 register */
+#define WR_PWM_DECODER_INDEX_SHIFT (4)
+#define WR_PWM_DECODER_INDEX_MASK (0xf)
+#define WR_REF_INDEX_SHIFT (0)
+#define WR_REF_INDEX_MASK (0xf)
+
+/* Bit definitions for the TOD_WRITE_CMD register */
+#define TOD_WRITE_SELECTION_SHIFT (0)
+#define TOD_WRITE_SELECTION_MASK (0xf)
+
+/* Bit definitions for the TOD_READ_PRIMARY_SEL_CFG_0 register */
+#define RD_PWM_DECODER_INDEX_SHIFT (4)
+#define RD_PWM_DECODER_INDEX_MASK (0xf)
+#define RD_REF_INDEX_SHIFT (0)
+#define RD_REF_INDEX_MASK (0xf)
+
+/* Bit definitions for the TOD_READ_PRIMARY_CMD register */
+#define TOD_READ_TRIGGER_MODE BIT(4)
+#define TOD_READ_TRIGGER_SHIFT (0)
+#define TOD_READ_TRIGGER_MASK (0xf)
+
+#endif
diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c
new file mode 100644
index 0000000..9abf4b1
--- /dev/null
+++ b/drivers/ptp/ptp_clockmatrix.c
@@ -0,0 +1,1384 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * PTP hardware clock driver for the IDT ClockMatrix(TM) family of timing and
+ * synchronization devices.
+ *
+ * Copyright (C) 2019 Integrated Device Technology, Inc., a Renesas Company.
+ */
+#define pr_fmt(fmt) "IDT_CM: " fmt
+
+#include <linux/firmware.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/ptp_clock_kernel.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/timekeeping.h>
+
+#include "ptp_private.h"
+#include "ptp_clockmatrix.h"
+
+MODULE_DESCRIPTION("Driver for IDT ClockMatrix(TM) family");
+MODULE_AUTHOR("Richard Cochran <richardcochran@gmail.com>");
+MODULE_AUTHOR("IDT support-1588 <IDT-support-1588@lm.renesas.com>");
+MODULE_VERSION("1.0");
+MODULE_LICENSE("GPL");
+
+#define SETTIME_CORRECTION (0)
+
+static s32 char_array_to_timespec(u8 *buf,
+ u8 count,
+ struct timespec64 *ts)
+{
+ u8 i;
+ u64 nsec;
+ time64_t sec;
+
+ if (count < TOD_BYTE_COUNT)
+ return 1;
+
+ /* Sub-nanoseconds are in buf[0]. */
+ nsec = buf[4];
+ for (i = 0; i < 3; i++) {
+ nsec <<= 8;
+ nsec |= buf[3 - i];
+ }
+
+ sec = buf[10];
+ for (i = 0; i < 5; i++) {
+ sec <<= 8;
+ sec |= buf[9 - i];
+ }
+
+ ts->tv_sec = sec;
+ ts->tv_nsec = nsec;
+
+ return 0;
+}
+
+static s32 timespec_to_char_array(struct timespec64 const *ts,
+ u8 *buf,
+ u8 count)
+{
+ u8 i;
+ s32 nsec;
+ time64_t sec;
+
+ if (count < TOD_BYTE_COUNT)
+ return 1;
+
+ nsec = ts->tv_nsec;
+ sec = ts->tv_sec;
+
+ /* Sub-nanoseconds are in buf[0]. */
+ buf[0] = 0;
+ for (i = 1; i < 5; i++) {
+ buf[i] = nsec & 0xff;
+ nsec >>= 8;
+ }
+
+ for (i = 5; i < TOD_BYTE_COUNT; i++) {
+
+ buf[i] = sec & 0xff;
+ sec >>= 8;
+ }
+
+ return 0;
+}
+
+static s32 idtcm_xfer(struct idtcm *idtcm,
+ u8 regaddr,
+ u8 *buf,
+ u16 count,
+ bool write)
+{
+ struct i2c_client *client = idtcm->client;
+ struct i2c_msg msg[2];
+ s32 cnt;
+
+ msg[0].addr = client->addr;
+ msg[0].flags = 0;
+ msg[0].len = 1;
+ msg[0].buf = ®addr;
+
+ msg[1].addr = client->addr;
+ msg[1].flags = write ? 0 : I2C_M_RD;
+ msg[1].len = count;
+ msg[1].buf = buf;
+
+ cnt = i2c_transfer(client->adapter, msg, 2);
+
+ if (cnt < 0) {
+ pr_err("i2c_transfer returned %d\n", cnt);
+ return cnt;
+ } else if (cnt != 2) {
+ pr_err("i2c_transfer sent only %d of %d messages\n", cnt, 2);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static s32 idtcm_page_offset(struct idtcm *idtcm, u8 val)
+{
+ u8 buf[4];
+ s32 err;
+
+ if (idtcm->page_offset == val)
+ return 0;
+
+ buf[0] = 0x0;
+ buf[1] = val;
+ buf[2] = 0x10;
+ buf[3] = 0x20;
+
+ err = idtcm_xfer(idtcm, PAGE_ADDR, buf, sizeof(buf), 1);
+
+ if (err)
+ pr_err("failed to set page offset\n");
+ else
+ idtcm->page_offset = val;
+
+ return err;
+}
+
+static s32 idtcm_rdwr(struct idtcm *idtcm,
+ u16 regaddr,
+ u8 *buf,
+ u16 count,
+ bool write)
+{
+ u8 hi;
+ u8 lo;
+ s32 err;
+
+ hi = (regaddr >> 8) & 0xff;
+ lo = regaddr & 0xff;
+
+ err = idtcm_page_offset(idtcm, hi);
+
+ if (err)
+ goto out;
+
+ err = idtcm_xfer(idtcm, lo, buf, count, write);
+out:
+ return err;
+}
+
+static s32 idtcm_read(struct idtcm *idtcm,
+ u16 module,
+ u16 regaddr,
+ u8 *buf,
+ u16 count)
+{
+ return idtcm->_idtcm_rdwr(idtcm, module + regaddr,
+ buf, count, false);
+}
+
+static s32 idtcm_write(struct idtcm *idtcm,
+ u16 module,
+ u16 regaddr,
+ u8 *buf,
+ u16 count)
+{
+ return idtcm->_idtcm_rdwr(idtcm, module + regaddr, buf, count, true);
+}
+
+static s32 idtcm_set_phase_pull_in_offset(struct idtcm_channel *channel,
+ s32 offset_ns)
+{
+ s32 err;
+ s32 i;
+ struct idtcm *idtcm = channel->idtcm;
+
+ u8 buf[4];
+
+ for (i = 0; i < 4; i++) {
+ buf[i] = 0xff & (offset_ns);
+ offset_ns >>= 8;
+ }
+
+ err = idtcm_write(idtcm, channel->dpll_phase_pull_in, PULL_IN_OFFSET,
+ buf, sizeof(buf));
+
+ return err;
+}
+
+static s32 idtcm_set_phase_pull_in_slope_limit(struct idtcm_channel *channel,
+ u32 max_ffo_ppb)
+{
+ s32 err;
+ u8 i;
+ struct idtcm *idtcm = channel->idtcm;
+
+ u8 buf[3];
+
+ if (max_ffo_ppb & 0xff000000)
+ max_ffo_ppb = 0;
+
+ for (i = 0; i < 3; i++) {
+ buf[i] = 0xff & (max_ffo_ppb);
+ max_ffo_ppb >>= 8;
+ }
+
+ err = idtcm_write(idtcm, channel->dpll_phase_pull_in,
+ PULL_IN_SLOPE_LIMIT, buf, sizeof(buf));
+
+ return err;
+}
+
+static s32 idtcm_start_phase_pull_in(struct idtcm_channel *channel)
+{
+ s32 err;
+ struct idtcm *idtcm = channel->idtcm;
+
+ u8 buf;
+
+ err = idtcm_read(idtcm, channel->dpll_phase_pull_in, PULL_IN_CTRL,
+ &buf, sizeof(buf));
+
+ if (err)
+ return err;
+
+ if (buf == 0) {
+ buf = 0x01;
+ err = idtcm_write(idtcm, channel->dpll_phase_pull_in,
+ PULL_IN_CTRL, &buf, sizeof(buf));
+ } else {
+ err = -EBUSY;
+ }
+
+ return err;
+}
+
+static s32 idtcm_do_phase_pull_in(struct idtcm_channel *channel,
+ s32 offset_ns,
+ u32 max_ffo_ppb)
+{
+ s32 err;
+
+ err = idtcm_set_phase_pull_in_offset(channel, -(s32)offset_ns);
+
+ if (err)
+ return err;
+
+ err = idtcm_set_phase_pull_in_slope_limit(channel, max_ffo_ppb);
+
+ if (err)
+ return err;
+
+ err = idtcm_start_phase_pull_in(channel);
+
+ return err;
+}
+
+static s32 _idtcm_adjtime(struct idtcm_channel *channel, s64 delta)
+{
+ s32 err;
+ struct idtcm *idtcm = channel->idtcm;
+ struct timespec64 ts;
+ s64 now;
+
+ if (abs(delta) < PHASE_PULL_IN_THRESHOLD_NS) {
+
+ err = idtcm_do_phase_pull_in(channel, delta, 0);
+
+ } else {
+
+ idtcm->calculate_overhead_flag = 1;
+
+ err = idtcm->_idtcm_gettime(channel, &ts);
+
+ if (err)
+ return err;
+
+ now = timespec64_to_ns(&ts);
+ now += delta;
+
+ ts = ns_to_timespec64(now);
+
+ err = idtcm->_idtcm_settime(channel, &ts,
+ HW_TOD_WR_TRIG_SEL_MSB);
+ }
+
+ return err;
+}
+
+static s32 idtcm_state_machine_reset(struct idtcm *idtcm)
+{
+ s32 err;
+ u8 byte = SM_RESET_CMD;
+
+ err = idtcm_write(idtcm, RESET_CTRL, SM_RESET, &byte, sizeof(byte));
+
+ if (!err) {
+ /* delay */
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule_timeout(_msecs_to_jiffies(POST_SM_RESET_DELAY_MS));
+ }
+
+ return err;
+}
+
+static s32 idtcm_read_hw_rev_id(struct idtcm *idtcm, u8 *hw_rev_id)
+{
+ return idtcm_read(idtcm,
+ GENERAL_STATUS,
+ HW_REV_ID,
+ hw_rev_id,
+ sizeof(u8));
+}
+
+static s32 idtcm_read_bond_id(struct idtcm *idtcm, u8 *bond_id)
+{
+ return idtcm_read(idtcm,
+ GENERAL_STATUS,
+ BOND_ID,
+ bond_id,
+ sizeof(u8));
+}
+
+static s32 idtcm_read_hw_csr_id(struct idtcm *idtcm, u16 *hw_csr_id)
+{
+ s32 err;
+ u8 buf[2] = {0};
+
+ err = idtcm_read(idtcm, GENERAL_STATUS, HW_CSR_ID, buf, sizeof(buf));
+
+ *hw_csr_id = (buf[1] << 8) | buf[0];
+
+ return err;
+}
+
+static s32 idtcm_read_hw_irq_id(struct idtcm *idtcm, u16 *hw_irq_id)
+{
+ s32 err;
+ u8 buf[2] = {0};
+
+ err = idtcm_read(idtcm, GENERAL_STATUS, HW_IRQ_ID, buf, sizeof(buf));
+
+ *hw_irq_id = (buf[1] << 8) | buf[0];
+
+ return err;
+}
+
+static s32 idtcm_read_product_id(struct idtcm *idtcm, u16 *product_id)
+{
+ s32 err;
+ u8 buf[2] = {0};
+
+ err = idtcm_read(idtcm, GENERAL_STATUS, PRODUCT_ID, buf, sizeof(buf));
+
+ *product_id = (buf[1] << 8) | buf[0];
+
+ return err;
+}
+
+static s32 idtcm_read_major_release(struct idtcm *idtcm, u8 *major)
+{
+ s32 err;
+ u8 buf;
+
+ err = idtcm_read(idtcm, GENERAL_STATUS, MAJ_REL, &buf, sizeof(buf));
+
+ *major = buf >> 1;
+
+ return err;
+}
+
+static s32 idtcm_read_minor_release(struct idtcm *idtcm, u8 *minor)
+{
+ return idtcm_read(idtcm, GENERAL_STATUS, MIN_REL, minor, sizeof(u8));
+}
+
+static s32 idtcm_read_hotfix_release(struct idtcm *idtcm, u8 *hotfix)
+{
+ return idtcm_read(idtcm,
+ GENERAL_STATUS,
+ HOTFIX_REL,
+ hotfix,
+ sizeof(u8));
+}
+
+static s32 idtcm_read_pipeline(struct idtcm *idtcm, u32 *pipeline)
+{
+ s32 err;
+ u8 buf[4];
+
+ err = idtcm_read(idtcm,
+ GENERAL_STATUS,
+ PIPELINE_ID,
+ &buf[0],
+ sizeof(buf));
+
+ *pipeline = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0];
+
+ return err;
+}
+
+static s32 process_pll_mask(u32 addr, u8 val, u8 *mask)
+{
+ s32 err = 0;
+
+ if (addr == PLL_MASK_ADDR) {
+ if ((val & 0xf0) || !(val & 0xf)) {
+ pr_err("bad firmware, PLL mask 0x%hhx invalid\n", val);
+ err = -EINVAL;
+ }
+ *mask = val;
+ }
+
+ return err;
+}
+
+static s32 set_pll_output_mask(struct idtcm *idtcm, u16 addr, u8 val)
+{
+ s32 err = 0;
+
+ switch (addr) {
+ case OUTPUT_MASK_PLL0_ADDR:
+ SET_U16_LSB(idtcm->channel[0].output_mask, val);
+ break;
+ case OUTPUT_MASK_PLL0_ADDR + 1:
+ SET_U16_MSB(idtcm->channel[0].output_mask, val);
+ break;
+ case OUTPUT_MASK_PLL1_ADDR:
+ SET_U16_LSB(idtcm->channel[1].output_mask, val);
+ break;
+ case OUTPUT_MASK_PLL1_ADDR + 1:
+ SET_U16_MSB(idtcm->channel[1].output_mask, val);
+ break;
+ case OUTPUT_MASK_PLL2_ADDR:
+ SET_U16_LSB(idtcm->channel[2].output_mask, val);
+ break;
+ case OUTPUT_MASK_PLL2_ADDR + 1:
+ SET_U16_MSB(idtcm->channel[2].output_mask, val);
+ break;
+ case OUTPUT_MASK_PLL3_ADDR:
+ SET_U16_LSB(idtcm->channel[3].output_mask, val);
+ break;
+ case OUTPUT_MASK_PLL3_ADDR + 1:
+ SET_U16_MSB(idtcm->channel[3].output_mask, val);
+ break;
+ default:
+ err = -1;
+ break;
+ }
+
+ return err;
+}
+
+static s32 check_and_set_masks(struct idtcm *idtcm,
+ u16 regaddr,
+ u8 val)
+{
+ s32 err = 0;
+
+ if (set_pll_output_mask(idtcm, regaddr, val)) {
+ /* Not an output mask, check for pll mask */
+ err = process_pll_mask(regaddr, val, &idtcm->pll_mask);
+ }
+
+ return err;
+}
+
+static void display_pll_and_output_masks(struct idtcm *idtcm)
+{
+ u8 i;
+ u8 mask;
+
+ pr_info("pllmask = 0x%02x\n", idtcm->pll_mask);
+
+ for (i = 0; i < MAX_PHC_PLL; i++) {
+ mask = 1 << i;
+
+ if (mask & idtcm->pll_mask)
+ pr_info("PLL%d output_mask = 0x%04x\n",
+ i, idtcm->channel[i].output_mask);
+ }
+}
+
+static s32 idtcm_load_firmware(struct idtcm *idtcm,
+ struct device *dev)
+{
+ const struct firmware *fw;
+ struct idtcm_fwrc *rec;
+ u32 regaddr;
+ s32 err;
+ s32 len;
+ u8 val;
+ u8 loaddr;
+
+ pr_info("requesting firmware '%s'\n", FW_FILENAME);
+
+ err = request_firmware(&fw, FW_FILENAME, dev);
+
+ if (err)
+ return err;
+
+ pr_info("firmware size %zu bytes\n", fw->size);
+
+ rec = (struct idtcm_fwrc *) fw->data;
+
+ if (fw->size > 0)
+ idtcm_state_machine_reset(idtcm);
+
+ for (len = fw->size; len > 0; len -= sizeof(*rec)) {
+
+ if (rec->reserved) {
+ pr_err("bad firmware, reserved field non-zero\n");
+ err = -EINVAL;
+ } else {
+ regaddr = rec->hiaddr << 8;
+ regaddr |= rec->loaddr;
+
+ val = rec->value;
+ loaddr = rec->loaddr;
+
+ rec++;
+
+ err = check_and_set_masks(idtcm, regaddr, val);
+ }
+
+ if (err == 0) {
+ /* Top (status registers) and bottom are read-only */
+ if ((regaddr < GPIO_USER_CONTROL)
+ || (regaddr >= SCRATCH))
+ continue;
+
+ /* Page size 128, last 4 bytes of page skipped */
+ if (((loaddr > 0x7b) && (loaddr <= 0x7f))
+ || ((loaddr > 0xfb) && (loaddr <= 0xff)))
+ continue;
+
+ err = idtcm_write(idtcm, regaddr, 0,
+ &val, sizeof(val));
+ }
+
+ if (err)
+ goto out;
+ }
+
+ display_pll_and_output_masks(idtcm);
+
+out:
+ release_firmware(fw);
+ return err;
+}
+
+static s32 idtcm_pps_enable(struct idtcm_channel *channel, bool enable)
+{
+ struct idtcm *idtcm = channel->idtcm;
+ u32 module;
+ u8 val;
+ s32 err;
+
+ /*
+ * This assumes that the 1-PPS is on the second of the two
+ * output. But is this always true?
+ */
+ switch (channel->dpll_n) {
+ case DPLL_0:
+ module = OUTPUT_1;
+ break;
+ case DPLL_1:
+ module = OUTPUT_3;
+ break;
+ case DPLL_2:
+ module = OUTPUT_5;
+ break;
+ case DPLL_3:
+ module = OUTPUT_7;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ err = idtcm_read(idtcm, module, OUT_CTRL_1, &val, sizeof(val));
+
+ if (err)
+ return err;
+
+ if (enable)
+ val |= SQUELCH_DISABLE;
+ else
+ val &= ~SQUELCH_DISABLE;
+
+ err = idtcm_write(idtcm, module, OUT_CTRL_1, &val, sizeof(val));
+
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static s32 sync_pll_output(struct idtcm *idtcm,
+ u8 pll,
+ u8 sync_src,
+ u8 qn,
+ u8 qn_plus_1)
+{
+ s32 err;
+ u8 val;
+ u16 sync_ctrl0;
+ u16 sync_ctrl1;
+
+ if ((qn == 0) && (qn_plus_1 == 0))
+ return 0;
+
+ switch (pll) {
+ case 0:
+ sync_ctrl0 = HW_Q0_Q1_CH_SYNC_CTRL_0;
+ sync_ctrl1 = HW_Q0_Q1_CH_SYNC_CTRL_1;
+ break;
+ case 1:
+ sync_ctrl0 = HW_Q2_Q3_CH_SYNC_CTRL_0;
+ sync_ctrl1 = HW_Q2_Q3_CH_SYNC_CTRL_1;
+ break;
+ case 2:
+ sync_ctrl0 = HW_Q4_Q5_CH_SYNC_CTRL_0;
+ sync_ctrl1 = HW_Q4_Q5_CH_SYNC_CTRL_1;
+ break;
+ case 3:
+ sync_ctrl0 = HW_Q6_Q7_CH_SYNC_CTRL_0;
+ sync_ctrl1 = HW_Q6_Q7_CH_SYNC_CTRL_1;
+ break;
+ case 4:
+ sync_ctrl0 = HW_Q8_CH_SYNC_CTRL_0;
+ sync_ctrl1 = HW_Q8_CH_SYNC_CTRL_1;
+ break;
+ case 5:
+ sync_ctrl0 = HW_Q9_CH_SYNC_CTRL_0;
+ sync_ctrl1 = HW_Q9_CH_SYNC_CTRL_1;
+ break;
+ case 6:
+ sync_ctrl0 = HW_Q10_CH_SYNC_CTRL_0;
+ sync_ctrl1 = HW_Q10_CH_SYNC_CTRL_1;
+ break;
+ case 7:
+ sync_ctrl0 = HW_Q11_CH_SYNC_CTRL_0;
+ sync_ctrl1 = HW_Q11_CH_SYNC_CTRL_1;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ val = SYNCTRL1_MASTER_SYNC_RST;
+
+ /* Place master sync in reset */
+ err = idtcm_write(idtcm, 0, sync_ctrl1, &val, sizeof(val));
+ if (err)
+ return err;
+
+ err = idtcm_write(idtcm, 0, sync_ctrl0, &sync_src, sizeof(sync_src));
+ if (err)
+ return err;
+
+ /* Set sync trigger mask */
+ val |= SYNCTRL1_FBDIV_FRAME_SYNC_TRIG | SYNCTRL1_FBDIV_SYNC_TRIG;
+
+ if (qn)
+ val |= SYNCTRL1_Q0_DIV_SYNC_TRIG;
+
+ if (qn_plus_1)
+ val |= SYNCTRL1_Q1_DIV_SYNC_TRIG;
+
+ err = idtcm_write(idtcm, 0, sync_ctrl1, &val, sizeof(val));
+ if (err)
+ return err;
+
+ /* Place master sync out of reset */
+ val &= ~(SYNCTRL1_MASTER_SYNC_RST);
+ err = idtcm_write(idtcm, 0, sync_ctrl1, &val, sizeof(val));
+
+ return err;
+}
+
+static s32 idtcm_sync_pps_output(struct idtcm_channel *channel)
+{
+ struct idtcm *idtcm = channel->idtcm;
+
+ u8 pll;
+ u8 sync_src;
+ u8 qn;
+ u8 qn_plus_1;
+ s32 err = 0;
+
+ u16 output_mask = channel->output_mask;
+
+ switch (channel->dpll_n) {
+ case DPLL_0:
+ sync_src = SYNC_SOURCE_DPLL0_TOD_PPS;
+ break;
+ case DPLL_1:
+ sync_src = SYNC_SOURCE_DPLL1_TOD_PPS;
+ break;
+ case DPLL_2:
+ sync_src = SYNC_SOURCE_DPLL2_TOD_PPS;
+ break;
+ case DPLL_3:
+ sync_src = SYNC_SOURCE_DPLL3_TOD_PPS;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ for (pll = 0; pll < 8; pll++) {
+
+ qn = output_mask & 0x1;
+ output_mask = output_mask >> 1;
+
+ if (pll < 4) {
+ /* First 4 pll has 2 outputs */
+ qn_plus_1 = output_mask & 0x1;
+ output_mask = output_mask >> 1;
+ } else {
+ qn_plus_1 = 0;
+ }
+
+ if ((qn != 0) || (qn_plus_1 != 0))
+ err = idtcm->_sync_pll_output(idtcm, pll, sync_src,
+ qn, qn_plus_1);
+
+ if (err)
+ return err;
+ }
+
+ return err;
+}
+
+static s32 idtcm_set_pll_mode(struct idtcm_channel *channel,
+ enum pll_mode pll_mode)
+{
+ struct idtcm *idtcm = channel->idtcm;
+ s32 err;
+ u8 dpll_mode;
+
+ err = idtcm_read(idtcm, channel->dpll_n, DPLL_MODE,
+ &dpll_mode, sizeof(dpll_mode));
+ if (err)
+ return err;
+
+ dpll_mode &= ~(PLL_MODE_MASK << PLL_MODE_SHIFT);
+
+ dpll_mode |= (pll_mode << PLL_MODE_SHIFT);
+
+ channel->pll_mode = pll_mode;
+
+ err = idtcm_write(idtcm, channel->dpll_n, DPLL_MODE,
+ &dpll_mode, sizeof(dpll_mode));
+ if (err)
+ return err;
+
+ return 0;
+}
+
+/* PTP Hardware Clock interface */
+static s32 idtcm_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
+{
+ struct idtcm_channel *channel =
+ container_of(ptp, struct idtcm_channel, caps);
+ struct idtcm *idtcm = channel->idtcm;
+ u8 i;
+ bool neg_adj = 0;
+ s32 err;
+ u8 buf[6] = {0};
+ s64 fcw;
+
+ if (channel->pll_mode != PLL_MODE_WRITE_FREQUENCY) {
+ err = idtcm_set_pll_mode(channel, PLL_MODE_WRITE_FREQUENCY);
+ if (err)
+ return err;
+ }
+
+ /*
+ * Frequency Control Word unit is: 1.11 * 10^-10 ppm
+ *
+ * adjfreq:
+ * ppb * 10^9
+ * FCW = ----------
+ * 111
+ *
+ * adjfine:
+ * ppm_16 * 5^12
+ * FCW = -------------
+ * 111 * 2^4
+ */
+ if (ppb < 0) {
+ neg_adj = 1;
+ ppb = -ppb;
+ }
+
+ /* 2 ^ -53 = 1.1102230246251565404236316680908e-16 */
+ fcw = ppb * 1000000000000ULL;
+
+ fcw = div_u64(fcw, 111022);
+
+ if (neg_adj)
+ fcw = -fcw;
+
+ for (i = 0; i < 6; i++) {
+ buf[i] = fcw & 0xff;
+ fcw >>= 8;
+ }
+
+ mutex_lock(&idtcm->reg_lock);
+
+ err = idtcm_write(idtcm, channel->dpll_freq, DPLL_WR_FREQ,
+ buf, sizeof(buf));
+
+ mutex_unlock(&idtcm->reg_lock);
+ return err;
+}
+
+static s32 _idtcm_gettime(struct idtcm_channel *channel,
+ struct timespec64 *ts)
+{
+ struct idtcm *idtcm = channel->idtcm;
+ u8 buf[TOD_BYTE_COUNT];
+ u8 trigger;
+ s32 err;
+
+ err = idtcm_read(idtcm, channel->tod_read_primary,
+ TOD_READ_PRIMARY_CMD, &trigger, sizeof(trigger));
+ if (err)
+ return err;
+
+ trigger &= ~(TOD_READ_TRIGGER_MASK << TOD_READ_TRIGGER_SHIFT);
+ trigger |= (1 << TOD_READ_TRIGGER_SHIFT);
+ trigger |= TOD_READ_TRIGGER_MODE;
+
+ err = idtcm_write(idtcm, channel->tod_read_primary,
+ TOD_READ_PRIMARY_CMD, &trigger, sizeof(trigger));
+
+ if (err)
+ return err;
+
+ if (idtcm->calculate_overhead_flag)
+ idtcm->start_time = ktime_get_raw();
+
+ err = idtcm_read(idtcm, channel->tod_read_primary,
+ TOD_READ_PRIMARY, buf, sizeof(buf));
+
+ if (err)
+ return err;
+
+ err = char_array_to_timespec(buf, sizeof(buf), ts);
+
+ return err;
+}
+
+static s32 idtcm_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
+{
+ struct idtcm_channel *channel =
+ container_of(ptp, struct idtcm_channel, caps);
+ struct idtcm *idtcm = channel->idtcm;
+ s32 err;
+
+ mutex_lock(&idtcm->reg_lock);
+
+ err = idtcm->_idtcm_gettime(channel, ts);
+
+ mutex_unlock(&idtcm->reg_lock);
+
+ return err;
+}
+
+static s32 _idtcm_set_dpll_tod(struct idtcm_channel *channel,
+ struct timespec64 const *ts,
+ enum hw_tod_write_trig_sel wr_trig)
+{
+ struct idtcm *idtcm = channel->idtcm;
+
+ u8 buf[TOD_BYTE_COUNT];
+ u8 cmd;
+ s32 err;
+ struct timespec64 local_ts = *ts;
+ s64 total_overhead_ns;
+
+ /* Configure HW TOD write trigger. */
+ err = idtcm_read(idtcm, channel->hw_dpll_n, HW_DPLL_TOD_CTRL_1,
+ &cmd, sizeof(cmd));
+
+ if (err)
+ return err;
+
+ cmd &= ~(0x0f);
+ cmd |= wr_trig | 0x08;
+
+ err = idtcm_write(idtcm, channel->hw_dpll_n, HW_DPLL_TOD_CTRL_1,
+ &cmd, sizeof(cmd));
+
+ if (err)
+ return err;
+
+ if (wr_trig != HW_TOD_WR_TRIG_SEL_MSB) {
+
+ err = timespec_to_char_array(&local_ts, buf, sizeof(buf));
+
+ if (err)
+ return err;
+
+ err = idtcm_write(idtcm, channel->hw_dpll_n,
+ HW_DPLL_TOD_OVR__0, buf, sizeof(buf));
+
+ if (err)
+ return err;
+ }
+
+ /* ARM HW TOD write trigger. */
+ cmd &= ~(0x08);
+
+ err = idtcm_write(idtcm, channel->hw_dpll_n, HW_DPLL_TOD_CTRL_1,
+ &cmd, sizeof(cmd));
+
+ if (wr_trig == HW_TOD_WR_TRIG_SEL_MSB) {
+
+ if (idtcm->calculate_overhead_flag) {
+ total_overhead_ns = ktime_to_ns(ktime_get_raw()
+ - idtcm->start_time)
+ + idtcm->tod_write_overhead_ns
+ + SETTIME_CORRECTION;
+
+ timespec64_add_ns(&local_ts, total_overhead_ns);
+
+ idtcm->calculate_overhead_flag = 0;
+ }
+
+ err = timespec_to_char_array(&local_ts, buf, sizeof(buf));
+
+ if (err)
+ return err;
+
+ err = idtcm_write(idtcm, channel->hw_dpll_n,
+ HW_DPLL_TOD_OVR__0, buf, sizeof(buf));
+ }
+
+ return err;
+}
+
+static s32 _idtcm_settime(struct idtcm_channel *channel,
+ struct timespec64 const *ts,
+ enum hw_tod_write_trig_sel wr_trig)
+{
+ struct idtcm *idtcm = channel->idtcm;
+ s32 retval;
+ s32 err;
+ s32 i;
+ u8 trig_sel;
+
+ err = _idtcm_set_dpll_tod(channel, ts, wr_trig);
+
+ if (err)
+ return err;
+
+ /* Wait for the operation to complete. */
+ for (i = 0; i < 10000; i++) {
+ err = idtcm_read(idtcm, channel->hw_dpll_n,
+ HW_DPLL_TOD_CTRL_1, &trig_sel,
+ sizeof(trig_sel));
+
+ if (err)
+ return err;
+
+ if (trig_sel == 0x4a)
+ break;
+
+ err = 1;
+ }
+
+ if (err)
+ return err;
+
+ retval = idtcm_sync_pps_output(channel);
+
+ return retval;
+}
+
+static s32 idtcm_settime(struct ptp_clock_info *ptp,
+ const struct timespec64 *ts)
+{
+ struct idtcm_channel *channel =
+ container_of(ptp, struct idtcm_channel, caps);
+ struct idtcm *idtcm = channel->idtcm;
+ s32 err;
+
+ mutex_lock(&idtcm->reg_lock);
+
+ err = idtcm->_idtcm_settime(channel, ts, HW_TOD_WR_TRIG_SEL_MSB);
+
+ mutex_unlock(&idtcm->reg_lock);
+
+ return err;
+}
+
+static s32 idtcm_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+ struct idtcm_channel *channel =
+ container_of(ptp, struct idtcm_channel, caps);
+ struct idtcm *idtcm = channel->idtcm;
+ s32 err;
+
+ mutex_lock(&idtcm->reg_lock);
+
+ err = _idtcm_adjtime(channel, delta);
+
+ mutex_unlock(&idtcm->reg_lock);
+
+ return err;
+}
+
+static s32 idtcm_enable(struct ptp_clock_info *ptp,
+ struct ptp_clock_request *rq, s32 on)
+{
+ struct idtcm_channel *channel =
+ container_of(ptp, struct idtcm_channel, caps);
+
+ switch (rq->type) {
+ case PTP_CLK_REQ_PEROUT:
+ if (!on)
+ return idtcm_pps_enable(channel, false);
+
+ /* Only accept a 1-PPS aligned to the second. */
+ if (rq->perout.start.nsec || rq->perout.period.sec != 1 ||
+ rq->perout.period.nsec)
+ return -ERANGE;
+
+ return idtcm_pps_enable(channel, true);
+ default:
+ break;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static s32 idtcm_enable_tod(struct idtcm_channel *channel)
+{
+ struct idtcm *idtcm = channel->idtcm;
+ struct timespec64 ts = {0, 0};
+ u8 cfg;
+ s32 err;
+
+ err = idtcm_pps_enable(channel, false);
+ if (err)
+ return err;
+
+ /*
+ * Start the TOD clock ticking.
+ */
+ err = idtcm_read(idtcm, channel->tod_n, TOD_CFG, &cfg, sizeof(cfg));
+ if (err)
+ return err;
+
+ cfg |= TOD_ENABLE;
+
+ err = idtcm_write(idtcm, channel->tod_n, TOD_CFG, &cfg, sizeof(cfg));
+ if (err)
+ return err;
+
+ return idtcm->_idtcm_settime(channel, &ts, HW_TOD_WR_TRIG_SEL_MSB);
+}
+
+static void set_default_function_pointers(struct idtcm *idtcm)
+{
+ idtcm->_idtcm_gettime = _idtcm_gettime;
+ idtcm->_idtcm_settime = _idtcm_settime;
+ idtcm->_idtcm_rdwr = idtcm_rdwr;
+ idtcm->_sync_pll_output = sync_pll_output;
+}
+
+static void idtcm_display_version_info(struct idtcm *idtcm)
+{
+ u8 major;
+ u8 minor;
+ u8 hotfix;
+ u32 pipeline;
+ u16 product_id;
+ u16 csr_id;
+ u16 irq_id;
+ u8 hw_rev_id;
+ u8 bond_id;
+
+ idtcm_read_major_release(idtcm, &major);
+ idtcm_read_minor_release(idtcm, &minor);
+ idtcm_read_hotfix_release(idtcm, &hotfix);
+ idtcm_read_pipeline(idtcm, &pipeline);
+
+ idtcm_read_product_id(idtcm, &product_id);
+ idtcm_read_hw_rev_id(idtcm, &hw_rev_id);
+ idtcm_read_bond_id(idtcm, &bond_id);
+ idtcm_read_hw_csr_id(idtcm, &csr_id);
+ idtcm_read_hw_irq_id(idtcm, &irq_id);
+
+ pr_info("Version: %d.%d.%d, Pipeline %u\t"
+ "0x%04x, Rev %d, Bond %d, CSR %d, IRQ %d\n",
+ major, minor, hotfix, pipeline,
+ product_id, hw_rev_id, bond_id, csr_id, irq_id);
+}
+
+static struct ptp_clock_info idtcm_caps = {
+ .owner = THIS_MODULE,
+ .max_adj = 244000,
+ .n_per_out = 1,
+ .adjfreq = &idtcm_adjfreq,
+ .adjtime = &idtcm_adjtime,
+ .gettime64 = &idtcm_gettime,
+ .settime64 = &idtcm_settime,
+ .enable = &idtcm_enable,
+};
+
+static s32 idtcm_enable_channel(struct idtcm *idtcm, u32 index)
+{
+ struct idtcm_channel *channel;
+ s32 err;
+
+ if (!(index < MAX_PHC_PLL))
+ return -EINVAL;
+
+ channel = &idtcm->channel[index];
+
+ switch (index) {
+ case 0:
+ channel->dpll_freq = DPLL_FREQ_0;
+ channel->dpll_n = DPLL_0;
+ channel->tod_read_primary = TOD_READ_PRIMARY_0;
+ channel->tod_write = TOD_WRITE_0;
+ channel->tod_n = TOD_0;
+ channel->hw_dpll_n = HW_DPLL_0;
+ channel->dpll_phase = DPLL_PHASE_0;
+ channel->dpll_ctrl_n = DPLL_CTRL_0;
+ channel->dpll_phase_pull_in = DPLL_PHASE_PULL_IN_0;
+ break;
+ case 1:
+ channel->dpll_freq = DPLL_FREQ_1;
+ channel->dpll_n = DPLL_1;
+ channel->tod_read_primary = TOD_READ_PRIMARY_1;
+ channel->tod_write = TOD_WRITE_1;
+ channel->tod_n = TOD_1;
+ channel->hw_dpll_n = HW_DPLL_1;
+ channel->dpll_phase = DPLL_PHASE_1;
+ channel->dpll_ctrl_n = DPLL_CTRL_1;
+ channel->dpll_phase_pull_in = DPLL_PHASE_PULL_IN_1;
+ break;
+ case 2:
+ channel->dpll_freq = DPLL_FREQ_2;
+ channel->dpll_n = DPLL_2;
+ channel->tod_read_primary = TOD_READ_PRIMARY_2;
+ channel->tod_write = TOD_WRITE_2;
+ channel->tod_n = TOD_2;
+ channel->hw_dpll_n = HW_DPLL_2;
+ channel->dpll_phase = DPLL_PHASE_2;
+ channel->dpll_ctrl_n = DPLL_CTRL_2;
+ channel->dpll_phase_pull_in = DPLL_PHASE_PULL_IN_2;
+ break;
+ case 3:
+ channel->dpll_freq = DPLL_FREQ_3;
+ channel->dpll_n = DPLL_3;
+ channel->tod_read_primary = TOD_READ_PRIMARY_3;
+ channel->tod_write = TOD_WRITE_3;
+ channel->tod_n = TOD_3;
+ channel->hw_dpll_n = HW_DPLL_3;
+ channel->dpll_phase = DPLL_PHASE_3;
+ channel->dpll_ctrl_n = DPLL_CTRL_3;
+ channel->dpll_phase_pull_in = DPLL_PHASE_PULL_IN_3;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ channel->idtcm = idtcm;
+
+ channel->caps = idtcm_caps;
+ snprintf(channel->caps.name, sizeof(channel->caps.name),
+ "IDT CM PLL%u", index);
+
+ err = idtcm_set_pll_mode(channel, PLL_MODE_WRITE_FREQUENCY);
+ if (err)
+ return err;
+
+ err = idtcm_enable_tod(channel);
+ if (err)
+ return err;
+
+ channel->ptp_clock = ptp_clock_register(&channel->caps, NULL);
+
+ if (IS_ERR(channel->ptp_clock)) {
+ err = PTR_ERR(channel->ptp_clock);
+ channel->ptp_clock = NULL;
+ return err;
+ }
+
+ if (!channel->ptp_clock)
+ return -ENOTSUPP;
+
+ pr_info("PLL%d registered as ptp%d ***\n",
+ index, channel->ptp_clock->index);
+
+ return 0;
+}
+
+static void ptp_clock_unregister_all(struct idtcm *idtcm)
+{
+ u8 i;
+ struct idtcm_channel *channel;
+
+ for (i = 0; i < MAX_PHC_PLL; i++) {
+
+ channel = &idtcm->channel[i];
+
+ if (channel->ptp_clock)
+ ptp_clock_unregister(channel->ptp_clock);
+ }
+}
+
+static void set_default_masks(struct idtcm *idtcm)
+{
+ idtcm->pll_mask = DEFAULT_PLL_MASK;
+
+ idtcm->channel[0].output_mask = DEFAULT_OUTPUT_MASK_PLL0;
+ idtcm->channel[1].output_mask = DEFAULT_OUTPUT_MASK_PLL1;
+ idtcm->channel[2].output_mask = DEFAULT_OUTPUT_MASK_PLL2;
+ idtcm->channel[3].output_mask = DEFAULT_OUTPUT_MASK_PLL3;
+}
+
+static s32 set_tod_write_overhead(struct idtcm *idtcm)
+{
+ s32 err;
+ u8 i;
+
+ s64 total_ns = 0;
+
+ ktime_t start;
+ ktime_t stop;
+
+ char buf[TOD_BYTE_COUNT];
+
+ struct idtcm_channel *channel = &idtcm->channel[2];
+
+ /* Set page offset */
+ idtcm_write(idtcm, channel->hw_dpll_n, HW_DPLL_TOD_OVR__0,
+ buf, sizeof(buf));
+
+ for (i = 0; i < TOD_WRITE_OVERHEAD_COUNT_MAX; i++) {
+
+ start = ktime_get_raw();
+
+ err = idtcm_write(idtcm, channel->hw_dpll_n,
+ HW_DPLL_TOD_OVR__0, buf, sizeof(buf));
+
+ if (err)
+ return err;
+
+ stop = ktime_get_raw();
+
+ total_ns += ktime_to_ns(stop - start);
+ }
+
+ idtcm->tod_write_overhead_ns = (total_ns
+ / (TOD_WRITE_OVERHEAD_COUNT_MAX));
+
+ return err;
+}
+
+static s32 idtcm_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct idtcm *idtcm;
+ s32 err;
+ u8 i;
+
+ /* Unused for now */
+ (void)id;
+
+ idtcm = devm_kzalloc(&client->dev, sizeof(struct idtcm), GFP_KERNEL);
+
+ if (!idtcm)
+ return -ENOMEM;
+
+ idtcm->client = client;
+ idtcm->page_offset = 0xff;
+
+ set_default_masks(idtcm);
+
+ set_default_function_pointers(idtcm);
+
+ mutex_init(&idtcm->reg_lock);
+ mutex_lock(&idtcm->reg_lock);
+
+ idtcm_display_version_info(idtcm);
+
+ err = set_tod_write_overhead(idtcm);
+
+ if (err)
+ return err;
+
+ err = idtcm_load_firmware(idtcm, &client->dev);
+
+ if (err)
+ pr_warn("loading firmware failed with %d\n", err);
+
+ if (idtcm->pll_mask) {
+ for (i = 0; i < MAX_PHC_PLL; i++) {
+ if (idtcm->pll_mask & (1 << i)) {
+ err = idtcm_enable_channel(idtcm, i);
+ if (err)
+ break;
+ }
+ }
+ } else {
+ pr_warn("no PLLs flagged as PHCs, nothing to do\n");
+ err = -ENODEV;
+ }
+
+ mutex_unlock(&idtcm->reg_lock);
+
+ if (err) {
+ ptp_clock_unregister_all(idtcm);
+ return err;
+ }
+
+ i2c_set_clientdata(client, idtcm);
+
+ return 0;
+}
+
+static s32 idtcm_remove(struct i2c_client *client)
+{
+ struct idtcm *idtcm = i2c_get_clientdata(client);
+
+ ptp_clock_unregister_all(idtcm);
+
+ mutex_destroy(&idtcm->reg_lock);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id idtcm_dt_id[] = {
+ { .compatible = "idt,8a3400x-ptp" },
+ { .compatible = "idt,8a3401x-ptp" },
+ { .compatible = "idt,8a3404x-ptp" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, idtcm_dt_id);
+#endif
+
+static const struct i2c_device_id idtcm_i2c_id[] = {
+ { "8a3400x-ptp" },
+ { "8a3401x-ptp" },
+ { "8a3404x-ptp" },
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, idtcm_i2c_id);
+
+static struct i2c_driver idtcm_driver = {
+ .driver = {
+ .of_match_table = of_match_ptr(idtcm_dt_id),
+ .name = "idtcm",
+ },
+ .probe = idtcm_probe,
+ .remove = idtcm_remove,
+ .id_table = idtcm_i2c_id,
+};
+
+module_i2c_driver(idtcm_driver);
diff --git a/drivers/ptp/ptp_clockmatrix.h b/drivers/ptp/ptp_clockmatrix.h
new file mode 100644
index 0000000..36c10e9
--- /dev/null
+++ b/drivers/ptp/ptp_clockmatrix.h
@@ -0,0 +1,123 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * PTP hardware clock driver for the IDT ClockMatrix(TM) family of timing and
+ * synchronization devices.
+ *
+ * Copyright (C) 2019 Integrated Device Technology, Inc., a Renesas Company.
+ */
+#ifndef PTP_IDTCLOCKMATRIX_H
+#define PTP_IDTCLOCKMATRIX_H
+
+#include <linux/ktime.h>
+
+#include "idt8a340_reg.h"
+
+#define FW_FILENAME "idtcm.bin"
+#define MAX_PHC_PLL 4
+
+#define PLL_MASK_ADDR (0xFFA5)
+#define DEFAULT_PLL_MASK (0x04)
+
+#define SET_U16_LSB(orig, val8) (orig = (0xff00 & (orig)) | (val8))
+#define SET_U16_MSB(orig, val8) (orig = (0x00ff & (orig)) | (val8 << 8))
+
+#define OUTPUT_MASK_PLL0_ADDR (0xFFB0)
+#define OUTPUT_MASK_PLL1_ADDR (0xFFB2)
+#define OUTPUT_MASK_PLL2_ADDR (0xFFB4)
+#define OUTPUT_MASK_PLL3_ADDR (0xFFB6)
+
+#define DEFAULT_OUTPUT_MASK_PLL0 (0x003)
+#define DEFAULT_OUTPUT_MASK_PLL1 (0x00c)
+#define DEFAULT_OUTPUT_MASK_PLL2 (0x030)
+#define DEFAULT_OUTPUT_MASK_PLL3 (0x0c0)
+
+#define POST_SM_RESET_DELAY_MS (3000)
+#define PHASE_PULL_IN_THRESHOLD_NS (150000)
+#define TOD_WRITE_OVERHEAD_COUNT_MAX (5)
+#define TOD_BYTE_COUNT (11)
+
+/* Values of DPLL_N.DPLL_MODE.PLL_MODE */
+enum pll_mode {
+ PLL_MODE_MIN = 0,
+ PLL_MODE_NORMAL = PLL_MODE_MIN,
+ PLL_MODE_WRITE_PHASE = 1,
+ PLL_MODE_WRITE_FREQUENCY = 2,
+ PLL_MODE_GPIO_INC_DEC = 3,
+ PLL_MODE_SYNTHESIS = 4,
+ PLL_MODE_PHASE_MEASUREMENT = 5,
+ PLL_MODE_MAX = PLL_MODE_PHASE_MEASUREMENT,
+};
+
+enum hw_tod_write_trig_sel {
+ HW_TOD_WR_TRIG_SEL_MIN = 0,
+ HW_TOD_WR_TRIG_SEL_MSB = HW_TOD_WR_TRIG_SEL_MIN,
+ HW_TOD_WR_TRIG_SEL_RESERVED = 1,
+ HW_TOD_WR_TRIG_SEL_TOD_PPS = 2,
+ HW_TOD_WR_TRIG_SEL_IRIGB_PPS = 3,
+ HW_TOD_WR_TRIG_SEL_PWM_PPS = 4,
+ HW_TOD_WR_TRIG_SEL_GPIO = 5,
+ HW_TOD_WR_TRIG_SEL_FOD_SYNC = 6,
+ WR_TRIG_SEL_MAX = HW_TOD_WR_TRIG_SEL_FOD_SYNC,
+};
+
+struct idtcm;
+
+struct idtcm_channel {
+ struct ptp_clock_info caps;
+ struct ptp_clock *ptp_clock;
+ struct idtcm *idtcm;
+ u16 dpll_phase;
+ u16 dpll_freq;
+ u16 dpll_n;
+ u16 dpll_ctrl_n;
+ u16 dpll_phase_pull_in;
+ u16 tod_read_primary;
+ u16 tod_write;
+ u16 tod_n;
+ u16 hw_dpll_n;
+ enum pll_mode pll_mode;
+ u16 output_mask;
+};
+
+struct idtcm {
+ struct idtcm_channel channel[MAX_PHC_PLL];
+ struct i2c_client *client;
+ u8 page_offset;
+ u8 pll_mask;
+
+ /* Overhead calculation for adjtime */
+ u8 calculate_overhead_flag;
+ s64 tod_write_overhead_ns;
+ ktime_t start_time;
+
+ /* Protects I2C read/modify/write registers from concurrent access */
+ struct mutex reg_lock;
+
+ s32 (*_idtcm_gettime)(struct idtcm_channel *channel,
+ struct timespec64 *ts);
+
+ s32 (*_idtcm_settime)(struct idtcm_channel *channel,
+ struct timespec64 const *ts,
+ enum hw_tod_write_trig_sel wr_trig);
+
+ s32 (*_idtcm_rdwr)(struct idtcm *idtcm,
+ u16 regaddr,
+ u8 *buf,
+ u16 count,
+ bool write);
+
+ s32 (*_sync_pll_output)(struct idtcm *idtcm,
+ u8 pll,
+ u8 sync_src,
+ u8 qn,
+ u8 qn_plus_1);
+};
+
+struct idtcm_fwrc {
+ u8 hiaddr;
+ u8 loaddr;
+ u8 value;
+ u8 reserved;
+} __packed;
+
+#endif /* PTP_IDTCLOCKMATRIX_H */
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: ptp: Add binding doc for IDT ClockMatrix based PTP clock
From: vincent.cheng.xh @ 2019-09-18 20:06 UTC (permalink / raw)
To: robh+dt, mark.rutland, richardcochran
Cc: devicetree, netdev, linux-kernel, Vincent Cheng
From: Vincent Cheng <vincent.cheng.xh@renesas.com>
Add device tree binding doc for the IDT ClockMatrix PTP clock driver.
Signed-off-by: Vincent Cheng <vincent.cheng.xh@renesas.com>
---
Documentation/devicetree/bindings/ptp/ptp-idtcm.txt | 15 +++++++++++++++
1 file changed, 15 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ptp/ptp-idtcm.txt
diff --git a/Documentation/devicetree/bindings/ptp/ptp-idtcm.txt b/Documentation/devicetree/bindings/ptp/ptp-idtcm.txt
new file mode 100644
index 0000000..4eaa34d
--- /dev/null
+++ b/Documentation/devicetree/bindings/ptp/ptp-idtcm.txt
@@ -0,0 +1,15 @@
+* IDT ClockMatrix (TM) PTP clock
+
+Required properties:
+
+ - compatible Should be "idt,8a3400x-ptp" for System Synchronizer
+ Should be "idt,8a3401x-ptp" for Port Synchronizer
+ Should be "idt,8a3404x-ptp" for Universal Frequency Translator (UFT)
+ - reg I2C slave address of the device
+
+Example:
+
+ phc@5b {
+ compatible = "idt,8a3400x-ptp";
+ reg = <0x5b>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v3] net: dsa: sja1105: prevent leaking memory
From: Navid Emamdoost @ 2019-09-18 20:34 UTC (permalink / raw)
To: olteanv
Cc: emamd001, smccaman, kjlu, Navid Emamdoost, Andrew Lunn,
Vivien Didelot, Florian Fainelli, David S. Miller, linux-kernel,
netdev
In-Reply-To: <8d6f6c54-1758-7d98-c9b5-5c16b171c885@gmail.com>
In sja1105_static_config_upload, in two cases memory is leaked: when
static_config_buf_prepare_for_upload fails and when sja1105_inhibit_tx
fails. In both cases config_buf should be released.
Fixes: 8aa9ebccae87 ("net: dsa: Introduce driver for NXP SJA1105 5-port
L2 switch")
Fixes: 1a4c69406cc1 ("net: dsa: sja1105: Prevent PHY jabbering during
switch reset")
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_spi.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index 84dc603138cf..58dd37ecde17 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -409,7 +409,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
rc = static_config_buf_prepare_for_upload(priv, config_buf, buf_len);
if (rc < 0) {
dev_err(dev, "Invalid config, cannot upload\n");
- return -EINVAL;
+ rc = -EINVAL;
+ goto out;
}
/* Prevent PHY jabbering during switch reset by inhibiting
* Tx on all ports and waiting for current packet to drain.
@@ -418,7 +419,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
rc = sja1105_inhibit_tx(priv, port_bitmap, true);
if (rc < 0) {
dev_err(dev, "Failed to inhibit Tx on ports\n");
- return -ENXIO;
+ rc = -ENXIO;
+ goto out;
}
/* Wait for an eventual egress packet to finish transmission
* (reach IFG). It is guaranteed that a second one will not
--
2.17.1
^ permalink raw reply related
* Re: [GIT] Networking
From: Linus Torvalds @ 2019-09-18 20:37 UTC (permalink / raw)
To: David Miller; +Cc: Andrew Morton, Netdev, Linux Kernel Mailing List
In-Reply-To: <20190918.003903.2143222297141990229.davem@davemloft.net>
Hmm. This adds that NET_TC_SKB_EXT config thing, and makes it "default y".
Why?
It's also done in a crazy way:
+ depends on NET_CLS_ACT
+ default y if NET_CLS_ACT
yeah, that's some screwed-up thinking right there. First it depends on
another config variable, and then it defaults to "y" if that variable
is set.
That's all kinds of messed up:
- we shouldn't "default y" for new features unless those features are
somehow critical (ie typically maybe it was a feature we already had,
but that now grew a config option to configure it _away_)
- that's a very confused way of saying "default y" (which you
shouldn't say in the first place)
- there's no explanation for why it should be enabled by default anyway.
I've obviously already pulled this (and only noticed when I was
testing further on my laptop), but please explain or fix.
Linus
^ permalink raw reply
* Re: [PATCH v3] net: dsa: sja1105: prevent leaking memory
From: Vladimir Oltean @ 2019-09-18 20:40 UTC (permalink / raw)
To: Navid Emamdoost
Cc: Navid Emamdoost, Stephen McCamant, kjlu, Andrew Lunn,
Vivien Didelot, Florian Fainelli, David S. Miller, lkml, netdev
In-Reply-To: <20190918203407.23826-1-navid.emamdoost@gmail.com>
On Wed, 18 Sep 2019 at 23:34, Navid Emamdoost <navid.emamdoost@gmail.com> wrote:
>
> In sja1105_static_config_upload, in two cases memory is leaked: when
> static_config_buf_prepare_for_upload fails and when sja1105_inhibit_tx
> fails. In both cases config_buf should be released.
>
> Fixes: 8aa9ebccae87 ("net: dsa: Introduce driver for NXP SJA1105 5-port
> L2 switch")
>
> Fixes: 1a4c69406cc1 ("net: dsa: sja1105: Prevent PHY jabbering during
> switch reset")
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
> drivers/net/dsa/sja1105/sja1105_spi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
> index 84dc603138cf..58dd37ecde17 100644
> --- a/drivers/net/dsa/sja1105/sja1105_spi.c
> +++ b/drivers/net/dsa/sja1105/sja1105_spi.c
> @@ -409,7 +409,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
> rc = static_config_buf_prepare_for_upload(priv, config_buf, buf_len);
> if (rc < 0) {
> dev_err(dev, "Invalid config, cannot upload\n");
> - return -EINVAL;
> + rc = -EINVAL;
> + goto out;
> }
> /* Prevent PHY jabbering during switch reset by inhibiting
> * Tx on all ports and waiting for current packet to drain.
> @@ -418,7 +419,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
> rc = sja1105_inhibit_tx(priv, port_bitmap, true);
> if (rc < 0) {
> dev_err(dev, "Failed to inhibit Tx on ports\n");
> - return -ENXIO;
> + rc = -ENXIO;
> + goto out;
> }
> /* Wait for an eventual egress packet to finish transmission
> * (reach IFG). It is guaranteed that a second one will not
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH] ionic: remove useless return code
From: Shannon Nelson @ 2019-09-18 20:46 UTC (permalink / raw)
To: Arnd Bergmann, Pensando Drivers, David S. Miller
Cc: netdev, linux-kernel, clang-built-linux
In-Reply-To: <20190918195745.2158829-1-arnd@arndb.de>
On 9/18/19 12:57 PM, Arnd Bergmann wrote:
> The debugfs function was apparently changed from returning an error code
> to a void return, but the return code left in place, causing a warning
> from clang:
>
> drivers/net/ethernet/pensando/ionic/ionic_debugfs.c:60:37: error: expression result unused [-Werror,-Wunused-value]
> ionic, &identity_fops) ? 0 : -EOPNOTSUPP;
> ^~~~~~~~~~~
>
> Fixes: fbfb8031533c ("ionic: Add hardware init and device commands")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ethernet/pensando/ionic/ionic_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> index 7afc4a365b75..bc03cecf80cc 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> @@ -57,7 +57,7 @@ DEFINE_SHOW_ATTRIBUTE(identity);
> void ionic_debugfs_add_ident(struct ionic *ionic)
> {
> debugfs_create_file("identity", 0400, ionic->dentry,
> - ionic, &identity_fops) ? 0 : -EOPNOTSUPP;
> + ionic, &identity_fops);
> }
>
> void ionic_debugfs_add_sizes(struct ionic *ionic)
This has just recently been addressed by Nathan Chancellor
<natechancellor@gmail.com>
Either way,
Acked-by: Shannon Nelson <snelson@pensando.io>
sln
^ permalink raw reply
* Re: [PATCH] dynamic_debug: provide dynamic_hex_dump stub
From: Shannon Nelson @ 2019-09-18 20:47 UTC (permalink / raw)
To: Arnd Bergmann, Pensando Drivers, David S. Miller, Jason Baron
Cc: Andrew Morton, netdev, linux-kernel
In-Reply-To: <20190918195607.2080036-1-arnd@arndb.de>
On 9/18/19 12:55 PM, Arnd Bergmann wrote:
> The ionic driver started using dymamic_hex_dump(), but
> that is not always defined:
>
> drivers/net/ethernet/pensando/ionic/ionic_main.c:229:2: error: implicit declaration of function 'dynamic_hex_dump' [-Werror,-Wimplicit-function-declaration]
>
> Add a dummy implementation to use when CONFIG_DYNAMIC_DEBUG
> is disabled, printing nothing.
>
> Fixes: 938962d55229 ("ionic: Add adminq action")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 ++
> drivers/net/ethernet/pensando/ionic/ionic_main.c | 2 ++
> include/linux/dynamic_debug.h | 6 ++++++
> 3 files changed, 10 insertions(+)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index db7c82742828..a255d24c8e40 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -1,6 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
>
> +#include <linux/printk.h>
> +#include <linux/dynamic_debug.h>
> #include <linux/netdevice.h>
> #include <linux/etherdevice.h>
> #include <linux/rtnetlink.h>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> index 15e432386b35..aab311413412 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> @@ -1,6 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
>
> +#include <linux/printk.h>
> +#include <linux/dynamic_debug.h>
> #include <linux/module.h>
> #include <linux/netdevice.h>
> #include <linux/utsname.h>
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index 6c809440f319..4cf02ecd67de 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -204,6 +204,12 @@ static inline int ddebug_dyndbg_module_param_cb(char *param, char *val,
> do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
> #define dynamic_dev_dbg(dev, fmt, ...) \
> do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); } while (0)
> +#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
> + groupsize, buf, len, ascii) \
> + do { if (0) \
> + print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, \
> + rowsize, groupsize, buf, len, ascii); \
> + } while (0)
> #endif
>
> #endif
Thanks, I hadn't had a chance to look into that one yet.
Acked-by: Shannon Nelson <snelson@pensando.io>
^ permalink raw reply
* Re: [PATCH 2/2] ptp: Add a ptp clock driver for IDT ClockMatrix.
From: Andrew Lunn @ 2019-09-18 21:18 UTC (permalink / raw)
To: vincent.cheng.xh
Cc: robh+dt, mark.rutland, richardcochran, devicetree, netdev,
linux-kernel
In-Reply-To: <1568837198-27211-2-git-send-email-vincent.cheng.xh@renesas.com>
On Wed, Sep 18, 2019 at 04:06:38PM -0400, vincent.cheng.xh@renesas.com wrote:
> From: Vincent Cheng <vincent.cheng.xh@renesas.com>
>
> The IDT ClockMatrix (TM) family includes integrated devices that provide
> eight PLL channels. Each PLL channel can be independently configured as a
> frequency synthesizer, jitter attenuator, digitally controlled
> oscillator (DCO), or a digital phase lock loop (DPLL). Typically
> these devices are used as timing references and clock sources for PTP
> applications. This patch adds support for the device.
>
> Co-developed-by: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: Vincent Cheng <vincent.cheng.xh@renesas.com>
Hi Vincent
> +static s32 idtcm_xfer(struct idtcm *idtcm,
> + u8 regaddr,
> + u8 *buf,
> + u16 count,
> + bool write)
> +{
> + struct i2c_client *client = idtcm->client;
> + struct i2c_msg msg[2];
> + s32 cnt;
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = 0;
> + msg[0].len = 1;
> + msg[0].buf = ®addr;
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = write ? 0 : I2C_M_RD;
> + msg[1].len = count;
> + msg[1].buf = buf;
> +
> + cnt = i2c_transfer(client->adapter, msg, 2);
> +
> + if (cnt < 0) {
> + pr_err("i2c_transfer returned %d\n", cnt);
dev_err(client->dev, "i2c_transfer returned %d\n", cnt);
We then have an idea which device has a transfer error.
Please try to not use pr_err() when you have some sort of device.
> +static s32 idtcm_state_machine_reset(struct idtcm *idtcm)
> +{
> + s32 err;
> + u8 byte = SM_RESET_CMD;
> +
> + err = idtcm_write(idtcm, RESET_CTRL, SM_RESET, &byte, sizeof(byte));
> +
> + if (!err) {
> + /* delay */
> + set_current_state(TASK_INTERRUPTIBLE);
> + schedule_timeout(_msecs_to_jiffies(POST_SM_RESET_DELAY_MS));
Maybe use msleep_interruptable()?
> + }
> +
> + return err;
> +}
> +
> +static s32 idtcm_load_firmware(struct idtcm *idtcm,
> + struct device *dev)
> +{
> + const struct firmware *fw;
> + struct idtcm_fwrc *rec;
> + u32 regaddr;
> + s32 err;
> + s32 len;
> + u8 val;
> + u8 loaddr;
> +
> + pr_info("requesting firmware '%s'\n", FW_FILENAME);
dev_debug()
> +
> + err = request_firmware(&fw, FW_FILENAME, dev);
> +
> + if (err)
> + return err;
> +
> + pr_info("firmware size %zu bytes\n", fw->size);
dev_debug()
Maybe look through all your pr_info and downgrade most of them to
dev_debug()
Andrew
^ permalink raw reply
* Re: [PATCH v3 bpf-next 07/14] samples: bpf: add makefile.target for separate CC target build
From: Andrii Nakryiko @ 2019-09-18 21:22 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: Alexei Starovoitov, Daniel Borkmann, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
john fastabend, open list, Networking, bpf, clang-built-linux,
sergei.shtylyov
In-Reply-To: <20190918101216.GA2908@khorivan>
On Wed, Sep 18, 2019 at 3:12 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> On Tue, Sep 17, 2019 at 04:19:40PM -0700, Andrii Nakryiko wrote:
> >On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk
> ><ivan.khoronzhuk@linaro.org> wrote:
> >>
> >> The makefile.target is added only and will be used in
> >
> >typo: Makefile
> >
> >> sample/bpf/Makefile later in order to switch cross-compiling on CC
> >
> >on -> to
> >
> >> from HOSTCC environment.
> >>
> >> The HOSTCC is supposed to build binaries and tools running on the host
> >> afterwards, in order to simplify build or so, like "fixdep" or else.
> >> In case of cross compiling "fixdep" is executed on host when the rest
> >> samples should run on target arch. In order to build binaries for
> >> target arch with CC and tools running on host with HOSTCC, lets add
> >> Makefile.target for simplicity, having definition and routines similar
> >> to ones, used in script/Makefile.host. This allows later add
> >> cross-compilation to samples/bpf with minimum changes.
> >>
> >> The tprog stands for target programs built with CC.
> >
> >Why tprog? Could we just use prog: hostprog vs prog.
> Prev. version was with prog, but Yonghong Song found it ambiguous.
> As prog can be bpf also. So, decision was made to follow logic:
> * target prog - non bpf progs
> * bpf prog = bpf prog, that can be later smth similar, providing build options
> for each bpf object separately.
>
Well, I'm not going to insist, but BPF program is a C function,
compiled BPF .o file is BPF object, so I don't think there is going to
be too much confusion to have progs and hostprogs in Makefile. But I'm
fine with tprog.
> Details here:
> https://lkml.org/lkml/2019/9/13/1037
>
> >
> >>
> >> Makefile.target contains only stuff needed for samples/bpf, potentially
> >> can be reused later and now needed only for unblocking tricky
> >> samples/bpf cross compilation.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >> samples/bpf/Makefile.target | 75 +++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 75 insertions(+)
> >> create mode 100644 samples/bpf/Makefile.target
> >>
> >> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >> new file mode 100644
> >> index 000000000000..fb6de63f7d2f
> >> --- /dev/null
> >> +++ b/samples/bpf/Makefile.target
> >> @@ -0,0 +1,75 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# ==========================================================================
> >> +# Building binaries on the host system
> >> +# Binaries are not used during the compilation of the kernel, and intendent
> >
> >typo: intended
> >
> >> +# to be build for target board, target board can be host ofc. Added to build
> >
> >What's ofc, is it "of course"?
> yes, ofc )
Alright, let's not try to save 5 letters, it's quite confusing.
>
> >
> >> +# binaries to run not on host system.
> >> +#
> >> +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference)
> >> +# tprogs-y := xsk_example
> >> +# Will compile xdpsock_example.c and create an executable named xsk_example
> >
> >You mix references to xsk_example and xdpsock_example, which is very
> >confusing. I'm guessing you meant to use xdpsock_example consistently.
> Oh, yes. Thanks.
>
> >
> >> +#
> >> +# tprogs-y := xdpsock
> >> +# xdpsock-objs := xdpsock_1.o xdpsock_2.o
> >> +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable
> >> +# xdpsock, based on xdpsock_1.o and xdpsock_2.o
> >> +#
> >> +# Inherited from scripts/Makefile.host
> >
> >"Inspired by" or "Derived from" would be probably more appropriate term :)
> I will replace with "Derived from", looks better.
>
sounds good
> >
> >> +#
> >> +__tprogs := $(sort $(tprogs-y))
> >> +
> >> +# C code
> >> +# Executables compiled from a single .c file
> >> +tprog-csingle := $(foreach m,$(__tprogs), \
> >> + $(if $($(m)-objs),,$(m)))
> >> +
> >> +# C executables linked based on several .o files
> >> +tprog-cmulti := $(foreach m,$(__tprogs),\
> >> + $(if $($(m)-objs),$(m)))
> >> +
> >> +# Object (.o) files compiled from .c files
> >> +tprog-cobjs := $(sort $(foreach m,$(__tprogs),$($(m)-objs)))
> >> +
> >> +tprog-csingle := $(addprefix $(obj)/,$(tprog-csingle))
> >> +tprog-cmulti := $(addprefix $(obj)/,$(tprog-cmulti))
> >> +tprog-cobjs := $(addprefix $(obj)/,$(tprog-cobjs))
> >> +
> >> +#####
> >> +# Handle options to gcc. Support building with separate output directory
> >> +
> >> +_tprogc_flags = $(TPROGS_CFLAGS) \
> >> + $(TPROGCFLAGS_$(basetarget).o)
> >> +
> >> +# $(objtree)/$(obj) for including generated headers from checkin source files
> >> +ifeq ($(KBUILD_EXTMOD),)
> >> +ifdef building_out_of_srctree
> >> +_tprogc_flags += -I $(objtree)/$(obj)
> >> +endif
> >> +endif
> >> +
> >> +tprogc_flags = -Wp,-MD,$(depfile) $(_tprogc_flags)
> >> +
> >> +# Create executable from a single .c file
> >> +# tprog-csingle -> Executable
> >> +quiet_cmd_tprog-csingle = CC $@
> >> + cmd_tprog-csingle = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ $< \
> >> + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
> >> +$(tprog-csingle): $(obj)/%: $(src)/%.c FORCE
> >> + $(call if_changed_dep,tprog-csingle)
> >> +
> >> +# Link an executable based on list of .o files, all plain c
> >> +# tprog-cmulti -> executable
> >> +quiet_cmd_tprog-cmulti = LD $@
> >> + cmd_tprog-cmulti = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ \
> >> + $(addprefix $(obj)/,$($(@F)-objs)) \
> >> + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
> >> +$(tprog-cmulti): $(tprog-cobjs) FORCE
> >> + $(call if_changed,tprog-cmulti)
> >> +$(call multi_depend, $(tprog-cmulti), , -objs)
> >> +
> >> +# Create .o file from a single .c file
> >> +# tprog-cobjs -> .o
> >> +quiet_cmd_tprog-cobjs = CC $@
> >> + cmd_tprog-cobjs = $(CC) $(tprogc_flags) -c -o $@ $<
> >> +$(tprog-cobjs): $(obj)/%.o: $(src)/%.c FORCE
> >> + $(call if_changed_dep,tprog-cobjs)
> >> --
> >> 2.17.1
> >>
> >
> >tprogs is quite confusing, but overall looks good to me.
> I tend to leave it as tprogs, unless it's going to be progs and agreed with
> Yonghong.
>
> It follows logic:
> - tprogs for bins
> - bpfprogs or bojs or bprogs (could be) for bpf obj
as mentioned above, we never build "BPF programs", they are always
part of BPF objects. But as I mentioned, I'm fine with sticking to
tprog.
>
> --
> Regards,
> Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH V2 net 1/1] net/tls(TLS_SW): Fix list_del double free caused by a race condition in tls_tx_records
From: Jakub Kicinski @ 2019-09-18 21:25 UTC (permalink / raw)
To: Pooja Trivedi
Cc: netdev, davem, daniel, john.fastabend, davejwatson, aviadye,
borisp, Pooja Trivedi, Mallesham Jatharakonda
In-Reply-To: <1568754836-25124-1-git-send-email-poojatrivedi@gmail.com>
On Tue, 17 Sep 2019 21:13:56 +0000, Pooja Trivedi wrote:
> From: Pooja Trivedi <pooja.trivedi@stackpath.com>
Ugh the same problem was diagnosed recently by Mallesham but I just
realized he took the conversation off list so you can't see it.
> Enclosing tls_tx_records within lock_sock/release_sock pair to ensure
> write-synchronization is not sufficient because socket lock gets released
> under memory pressure situation by sk_wait_event while it sleeps waiting
> for memory, allowing another writer into tls_tx_records. This causes a
> race condition with record deletion post transmission.
>
> To fix this bug, use a flag set in tx_bitmask field of TLS context to
> ensure single writer in tls_tx_records at a time
Could you point me to the place where socket lock gets released in/under
tls_tx_records()? I thought it's only done in tls_sw_do_sendpage()/
tls_sw_do_sendmsg().
FWIW this was my answer to Mallesham:
If I understand you correctly after we release and re-acquire socket
lock msg_pl may be pointing to already freed message? Could we perhaps
reload the pointer from the context/record? Something like:
if (ret) {
rec = ctx->open_rec;
if (rec)
tls_trim_both_msgs(sk, &rec->msg_plaintext.sg.size);
goto sendpage_end;
}
I'm not 100% sure if that makes sense, perhaps John will find time to
look or you could experiment?
We could try to add some state like we have ctx->in_tcp_sendpages to
let the async processing know it's not needed since there's still a
writer present, but I get a feeling that'd end up being more complex.
> The bug resulted in the following crash:
>
> [ 270.888952] ------------[ cut here ]------------
> [ 270.890450] list_del corruption, ffff91cc3753a800->prev is
> LIST_POISON2 (dead000000000122)
> [ 270.891194] WARNING: CPU: 1 PID: 7387 at lib/list_debug.c:50
> __list_del_entry_valid+0x62/0x90
> [ 270.892037] Modules linked in: n5pf(OE) netconsole tls(OE) bonding
> intel_rapl_msr intel_rapl_common sb_edac x86_pkg_temp_thermal
> intel_powerclamp coretemp kvm_intel kvm iTCO_wdt iTCO_vendor_support
> irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
> aesni_intel crypto_simd mei_me cryptd glue_helper ipmi_si sg mei
> lpc_ich pcspkr joydev ioatdma i2c_i801 ipmi_devintf ipmi_msghandler
> wmi ip_tables xfs libcrc32c sd_mod mgag200 drm_vram_helper ttm
> drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm isci
> libsas ahci scsi_transport_sas libahci crc32c_intel serio_raw igb
> libata ptp pps_core dca i2c_algo_bit dm_mirror dm_region_hash dm_log
> dm_mod [last unloaded: nitrox_drv]
> [ 270.896836] CPU: 1 PID: 7387 Comm: uperf Kdump: loaded Tainted: G
> OE 5.3.0-rc4 #1
> [ 270.897711] Hardware name: Supermicro SYS-1027R-N3RF/X9DRW, BIOS
> 3.0c 03/24/2014
> [ 270.898597] RIP: 0010:__list_del_entry_valid+0x62/0x90
> [ 270.899478] Code: 00 00 00 c3 48 89 fe 48 89 c2 48 c7 c7 e0 f9 ee
> 8d e8 b2 cf c8 ff 0f 0b 31 c0 c3 48 89 fe 48 c7 c7 18 fa ee 8d e8 9e
> cf c8 ff <0f> 0b 31 c0 c3 48 89 f2 48 89 fe 48 c7 c7 50 fa ee 8d e8 87
> cf c8
> [ 270.901321] RSP: 0018:ffffb6ea86eb7c20 EFLAGS: 00010282
> [ 270.902240] RAX: 0000000000000000 RBX: ffff91cc3753c000 RCX: 0000000000000000
> [ 270.903157] RDX: ffff91bc3f867080 RSI: ffff91bc3f857738 RDI: ffff91bc3f857738
> [ 270.904074] RBP: ffff91bc36020940 R08: 0000000000000560 R09: 0000000000000000
> [ 270.904988] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [ 270.905902] R13: ffff91cc3753a800 R14: ffff91cc37cc6400 R15: ffff91cc3753a800
> [ 270.906809] FS: 00007f454a88d700(0000) GS:ffff91bc3f840000(0000)
> knlGS:0000000000000000
> [ 270.907715] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 270.908606] CR2: 00007f453c00292c CR3: 000000103554e003 CR4: 00000000001606e0
> [ 270.909490] Call Trace:
> [ 270.910373] tls_tx_records+0x138/0x1c0 [tls]
> [ 270.911262] tls_sw_sendpage+0x3e0/0x420 [tls]
> [ 270.912154] inet_sendpage+0x52/0x90
> [ 270.913045] ? direct_splice_actor+0x40/0x40
> [ 270.913941] kernel_sendpage+0x1a/0x30
> [ 270.914831] sock_sendpage+0x20/0x30
> [ 270.915714] pipe_to_sendpage+0x62/0x90
> [ 270.916592] __splice_from_pipe+0x80/0x180
> [ 270.917461] ? direct_splice_actor+0x40/0x40
> [ 270.918334] splice_from_pipe+0x5d/0x90
> [ 270.919208] direct_splice_actor+0x35/0x40
> [ 270.920086] splice_direct_to_actor+0x103/0x230
> [ 270.920966] ? generic_pipe_buf_nosteal+0x10/0x10
> [ 270.921850] do_splice_direct+0x9a/0xd0
> [ 270.922733] do_sendfile+0x1c9/0x3d0
> [ 270.923612] __x64_sys_sendfile64+0x5c/0xc0
>
> Signed-off-by: Pooja Trivedi <pooja.trivedi@stackpath.com>
> ---
> include/net/tls.h | 1 +
> net/tls/tls_sw.c | 7 +++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/include/net/tls.h b/include/net/tls.h
> index 41b2d41..f346a54 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -161,6 +161,7 @@ struct tls_sw_context_tx {
>
> #define BIT_TX_SCHEDULED 0
> #define BIT_TX_CLOSING 1
> +#define BIT_TX_IN_PROGRESS 2
> unsigned long tx_bitmask;
> };
>
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 91d21b0..6e99c61 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -367,6 +367,10 @@ int tls_tx_records(struct sock *sk, int flags)
> struct sk_msg *msg_en;
> int tx_flags, rc = 0;
>
> + /* If another writer is already in tls_tx_records, backoff and leave */
> + if (test_and_set_bit(BIT_TX_IN_PROGRESS, &ctx->tx_bitmask))
> + return 0;
> +
> if (tls_is_partially_sent_record(tls_ctx)) {
> rec = list_first_entry(&ctx->tx_list,
> struct tls_rec, list);
> @@ -415,6 +419,9 @@ int tls_tx_records(struct sock *sk, int flags)
> if (rc < 0 && rc != -EAGAIN)
> tls_err_abort(sk, EBADMSG);
>
> + /* clear the bit so another writer can get into tls_tx_records */
> + clear_bit(BIT_TX_IN_PROGRESS, &ctx->tx_bitmask);
> +
> return rc;
> }
>
^ permalink raw reply
* Re: [PATCH v3 bpf-next 09/14] samples: bpf: makefile: use own flags but not host when cross compile
From: Andrii Nakryiko @ 2019-09-18 21:29 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: Alexei Starovoitov, Daniel Borkmann, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
john fastabend, open list, Networking, bpf, clang-built-linux,
sergei.shtylyov
In-Reply-To: <20190918103508.GC2908@khorivan>
On Wed, Sep 18, 2019 at 3:35 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> On Tue, Sep 17, 2019 at 04:42:07PM -0700, Andrii Nakryiko wrote:
> >On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
> ><ivan.khoronzhuk@linaro.org> wrote:
> >>
> >> While compile natively, the hosts cflags and ldflags are equal to ones
> >> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
> >> have own, used for target arch. While verification, for arm, arm64 and
> >> x86_64 the following flags were used alsways:
> >>
> >> -Wall
> >> -O2
> >> -fomit-frame-pointer
> >> -Wmissing-prototypes
> >> -Wstrict-prototypes
> >>
> >> So, add them as they were verified and used before adding
> >> Makefile.target, but anyway limit it only for cross compile options as
> >> for host can be some configurations when another options can be used,
> >> So, for host arch samples left all as is, it allows to avoid potential
> >> option mistmatches for existent environments.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >> samples/bpf/Makefile | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >> index 1579cc16a1c2..b5c87a8b8b51 100644
> >> --- a/samples/bpf/Makefile
> >> +++ b/samples/bpf/Makefile
> >> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
> >> TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
> >> endif
> >>
> >> +ifdef CROSS_COMPILE
> >> +TPROGS_CFLAGS += -Wall
> >> +TPROGS_CFLAGS += -O2
> >
> >Specifying one arg per line seems like overkill, put them in one line?
> Will combine.
>
> >
> >> +TPROGS_CFLAGS += -fomit-frame-pointer
> >
> >Why this one?
> I've explained in commit msg. The logic is to have as much as close options
> to have smiliar binaries. As those options are used before for hosts and kinda
> cross builds - better follow same way.
I'm just asking why omit frame pointers and make it harder to do stuff
like profiling? What performance benefits are we seeking for in BPF
samples?
>
> >
> >> +TPROGS_CFLAGS += -Wmissing-prototypes
> >> +TPROGS_CFLAGS += -Wstrict-prototypes
> >
> >Are these in some way special that we want them in cross-compile mode only?
> >
> >All of those flags seem useful regardless of cross-compilation or not,
> >shouldn't they be common? I'm a bit lost about the intent here...
> They are common but split is needed to expose it at least. Also host for
> different arches can have some own opts already used that shouldn't be present
> for cross, better not mix it for safety.
We want -Wmissing-prototypes and -Wstrict-prototypes for cross-compile
and non-cross-compile cases, right? So let's specify them as common
set of options, instead of relying on KBUILD_HOSTCFLAGS or
HOST_EXTRACFLAGS to have them. Otherwise we'll be getting extra
warnings for just cross-compile case, which is not good. If you are
worrying about having duplicate -W flags, seems like it's handled by
GCC already, so shouldn't be a problem.
>
> >
> >> +else
> >> TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
> >> TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
> >> +endif
> >> +
> >> TPROGS_CFLAGS += -I$(objtree)/usr/include
> >> TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/
> >> TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> >> --
> >> 2.17.1
> >>
>
> --
> Regards,
> Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH net] net: sched: fix possible crash in tcf_action_destroy()
From: Cong Wang @ 2019-09-18 21:37 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, syzbot, Vlad Buslov,
Jiri Pirko
In-Reply-To: <20190918195704.218413-1-edumazet@google.com>
On Wed, Sep 18, 2019 at 12:57 PM 'Eric Dumazet' via syzkaller
<syzkaller@googlegroups.com> wrote:
>
> If the allocation done in tcf_exts_init() failed,
> we end up with a NULL pointer in exts->actions.
...
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index efd3cfb80a2ad775dc8ab3c4900bd73d52c7aaad..9aef93300f1c11791acbb9262dfe77996872eafe 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -3027,8 +3027,10 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
> void tcf_exts_destroy(struct tcf_exts *exts)
> {
> #ifdef CONFIG_NET_CLS_ACT
> - tcf_action_destroy(exts->actions, TCA_ACT_UNBIND);
> - kfree(exts->actions);
> + if (exts->actions) {
I think it is _slightly_ better to check exts->nr_actions!=0 here,
as it would help exts->actions!=NULL&& exts->nr_actions==0
cases too.
What do you think?
^ permalink raw reply
* Re: [PATCH V2 net 1/1] net/tls(TLS_SW): Fix list_del double free caused by a race condition in tls_tx_records
From: Pooja Trivedi @ 2019-09-18 21:37 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, daniel, john.fastabend, davejwatson, aviadye,
borisp, Pooja Trivedi, Mallesham Jatharakonda
In-Reply-To: <20190918142549.69bfa285@cakuba.netronome.com>
Hi Jakub,
I have explained one potential way for the race to happen in my
original message to the netdev mailing list here:
https://marc.info/?l=linux-netdev&m=156805120229554&w=2
Here is the part out of there that's relevant to your question:
-----------------------------------------
One potential way for race condition to appear:
When under tcp memory pressure, Thread 1 takes the following code path:
do_sendfile ---> ... ---> .... ---> tls_sw_sendpage --->
tls_sw_do_sendpage ---> tls_tx_records ---> tls_push_sg --->
do_tcp_sendpages ---> sk_stream_wait_memory ---> sk_wait_event
sk_wait_event releases the socket lock and sleeps waiting for memory:
#define sk_wait_event(__sk, __timeo, __condition, __wait) \
({ int __rc; \
release_sock(__sk); \
__rc = __condition; \
if (!__rc) { \
*(__timeo) = wait_woken(__wait, \
TASK_INTERRUPTIBLE, \
*(__timeo)); \
} \
sched_annotate_sleep(); \
lock_sock(__sk); \
__rc = __condition; \
__rc; \
})
Thread 2 code path:
tx_work_handler ---> tls_tx_records
Thread 2 is able to obtain the socket lock and go through the
transmission of the ctx->tx_list, deleting the sent ones (as in the
for loop below).
int tls_tx_records(struct sock *sk, int flags)
{
....
....
....
....
list_for_each_entry_safe(rec, tmp, &ctx->tx_list, list) {
if (READ_ONCE(rec->tx_ready)) {
if (flags == -1)
tx_flags = rec->tx_flags;
else
tx_flags = flags;
msg_en = &rec->msg_encrypted;
rc = tls_push_sg(sk, tls_ctx,
&msg_en->sg.data[msg_en->sg.curr],
0, tx_flags);
if (rc)
goto tx_err;
list_del(&rec->list); // **** crash location ****
sk_msg_free(sk, &rec->msg_plaintext);
kfree(rec);
} else {
break;
}
}
....
....
....
....
}
When Thread 1 wakes up from tls_push_sg call and attempts list_del on
previously grabbed record which was sent and deleted by Thread 2, it
causes the crash.
To fix this race, a flag or bool inside of ctx can be used to
synchronize access to tls_tx_records.
-----------------------------------------
Let me know if you need more information. Thanks!
On Wed, Sep 18, 2019 at 5:25 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Tue, 17 Sep 2019 21:13:56 +0000, Pooja Trivedi wrote:
> > From: Pooja Trivedi <pooja.trivedi@stackpath.com>
>
> Ugh the same problem was diagnosed recently by Mallesham but I just
> realized he took the conversation off list so you can't see it.
>
> > Enclosing tls_tx_records within lock_sock/release_sock pair to ensure
> > write-synchronization is not sufficient because socket lock gets released
> > under memory pressure situation by sk_wait_event while it sleeps waiting
> > for memory, allowing another writer into tls_tx_records. This causes a
> > race condition with record deletion post transmission.
> >
> > To fix this bug, use a flag set in tx_bitmask field of TLS context to
> > ensure single writer in tls_tx_records at a time
>
> Could you point me to the place where socket lock gets released in/under
> tls_tx_records()? I thought it's only done in tls_sw_do_sendpage()/
> tls_sw_do_sendmsg().
>
> FWIW this was my answer to Mallesham:
>
> If I understand you correctly after we release and re-acquire socket
> lock msg_pl may be pointing to already freed message? Could we perhaps
> reload the pointer from the context/record? Something like:
>
> if (ret) {
> rec = ctx->open_rec;
> if (rec)
> tls_trim_both_msgs(sk, &rec->msg_plaintext.sg.size);
> goto sendpage_end;
> }
>
> I'm not 100% sure if that makes sense, perhaps John will find time to
> look or you could experiment?
>
> We could try to add some state like we have ctx->in_tcp_sendpages to
> let the async processing know it's not needed since there's still a
> writer present, but I get a feeling that'd end up being more complex.
>
> > The bug resulted in the following crash:
> >
> > [ 270.888952] ------------[ cut here ]------------
> > [ 270.890450] list_del corruption, ffff91cc3753a800->prev is
> > LIST_POISON2 (dead000000000122)
> > [ 270.891194] WARNING: CPU: 1 PID: 7387 at lib/list_debug.c:50
> > __list_del_entry_valid+0x62/0x90
> > [ 270.892037] Modules linked in: n5pf(OE) netconsole tls(OE) bonding
> > intel_rapl_msr intel_rapl_common sb_edac x86_pkg_temp_thermal
> > intel_powerclamp coretemp kvm_intel kvm iTCO_wdt iTCO_vendor_support
> > irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
> > aesni_intel crypto_simd mei_me cryptd glue_helper ipmi_si sg mei
> > lpc_ich pcspkr joydev ioatdma i2c_i801 ipmi_devintf ipmi_msghandler
> > wmi ip_tables xfs libcrc32c sd_mod mgag200 drm_vram_helper ttm
> > drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm isci
> > libsas ahci scsi_transport_sas libahci crc32c_intel serio_raw igb
> > libata ptp pps_core dca i2c_algo_bit dm_mirror dm_region_hash dm_log
> > dm_mod [last unloaded: nitrox_drv]
> > [ 270.896836] CPU: 1 PID: 7387 Comm: uperf Kdump: loaded Tainted: G
> > OE 5.3.0-rc4 #1
> > [ 270.897711] Hardware name: Supermicro SYS-1027R-N3RF/X9DRW, BIOS
> > 3.0c 03/24/2014
> > [ 270.898597] RIP: 0010:__list_del_entry_valid+0x62/0x90
> > [ 270.899478] Code: 00 00 00 c3 48 89 fe 48 89 c2 48 c7 c7 e0 f9 ee
> > 8d e8 b2 cf c8 ff 0f 0b 31 c0 c3 48 89 fe 48 c7 c7 18 fa ee 8d e8 9e
> > cf c8 ff <0f> 0b 31 c0 c3 48 89 f2 48 89 fe 48 c7 c7 50 fa ee 8d e8 87
> > cf c8
> > [ 270.901321] RSP: 0018:ffffb6ea86eb7c20 EFLAGS: 00010282
> > [ 270.902240] RAX: 0000000000000000 RBX: ffff91cc3753c000 RCX: 0000000000000000
> > [ 270.903157] RDX: ffff91bc3f867080 RSI: ffff91bc3f857738 RDI: ffff91bc3f857738
> > [ 270.904074] RBP: ffff91bc36020940 R08: 0000000000000560 R09: 0000000000000000
> > [ 270.904988] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> > [ 270.905902] R13: ffff91cc3753a800 R14: ffff91cc37cc6400 R15: ffff91cc3753a800
> > [ 270.906809] FS: 00007f454a88d700(0000) GS:ffff91bc3f840000(0000)
> > knlGS:0000000000000000
> > [ 270.907715] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 270.908606] CR2: 00007f453c00292c CR3: 000000103554e003 CR4: 00000000001606e0
> > [ 270.909490] Call Trace:
> > [ 270.910373] tls_tx_records+0x138/0x1c0 [tls]
> > [ 270.911262] tls_sw_sendpage+0x3e0/0x420 [tls]
> > [ 270.912154] inet_sendpage+0x52/0x90
> > [ 270.913045] ? direct_splice_actor+0x40/0x40
> > [ 270.913941] kernel_sendpage+0x1a/0x30
> > [ 270.914831] sock_sendpage+0x20/0x30
> > [ 270.915714] pipe_to_sendpage+0x62/0x90
> > [ 270.916592] __splice_from_pipe+0x80/0x180
> > [ 270.917461] ? direct_splice_actor+0x40/0x40
> > [ 270.918334] splice_from_pipe+0x5d/0x90
> > [ 270.919208] direct_splice_actor+0x35/0x40
> > [ 270.920086] splice_direct_to_actor+0x103/0x230
> > [ 270.920966] ? generic_pipe_buf_nosteal+0x10/0x10
> > [ 270.921850] do_splice_direct+0x9a/0xd0
> > [ 270.922733] do_sendfile+0x1c9/0x3d0
> > [ 270.923612] __x64_sys_sendfile64+0x5c/0xc0
> >
> > Signed-off-by: Pooja Trivedi <pooja.trivedi@stackpath.com>
> > ---
> > include/net/tls.h | 1 +
> > net/tls/tls_sw.c | 7 +++++++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/include/net/tls.h b/include/net/tls.h
> > index 41b2d41..f346a54 100644
> > --- a/include/net/tls.h
> > +++ b/include/net/tls.h
> > @@ -161,6 +161,7 @@ struct tls_sw_context_tx {
> >
> > #define BIT_TX_SCHEDULED 0
> > #define BIT_TX_CLOSING 1
> > +#define BIT_TX_IN_PROGRESS 2
> > unsigned long tx_bitmask;
> > };
> >
> > diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> > index 91d21b0..6e99c61 100644
> > --- a/net/tls/tls_sw.c
> > +++ b/net/tls/tls_sw.c
> > @@ -367,6 +367,10 @@ int tls_tx_records(struct sock *sk, int flags)
> > struct sk_msg *msg_en;
> > int tx_flags, rc = 0;
> >
> > + /* If another writer is already in tls_tx_records, backoff and leave */
> > + if (test_and_set_bit(BIT_TX_IN_PROGRESS, &ctx->tx_bitmask))
> > + return 0;
> > +
> > if (tls_is_partially_sent_record(tls_ctx)) {
> > rec = list_first_entry(&ctx->tx_list,
> > struct tls_rec, list);
> > @@ -415,6 +419,9 @@ int tls_tx_records(struct sock *sk, int flags)
> > if (rc < 0 && rc != -EAGAIN)
> > tls_err_abort(sk, EBADMSG);
> >
> > + /* clear the bit so another writer can get into tls_tx_records */
> > + clear_bit(BIT_TX_IN_PROGRESS, &ctx->tx_bitmask);
> > +
> > return rc;
> > }
> >
>
^ permalink raw reply
* [PATCH RFC] net/phy: fix Marvell PHYs probe failure when HWMON and THERMAL_OF are enabled
From: Peter Mamonov @ 2019-09-18 21:38 UTC (permalink / raw)
To: andrew, f.fainelli, hkallweit1; +Cc: netdev, Peter Mamonov
Hello,
Some time ago I've discovered that probe functions of certain Marvell PHYs
fail if both HWMON and THERMAL_OF config options are enabled. The root
cause of this problem is a lack of an OF node for a PHY's built-in
temperature sensor. However I consider adding this OF node to be a bit
excessive solution. Am I wrong? Below you will find a one line patch which
fixes the problem. I've sent it to the releveant maintainers three weeks
ago without any feedback yet. Could you, please, take a look at the problem
and give your considerations on how to fix it properly?
Regards,
Peter
thermal: make thermal_zone_of_sensor_register return -ENODEV
if a sensor OF node is missing
When devm_thermal_zone_of_sensor_register() is called from
hwmon_thermal_add_sensor() it is possible that the relevant sensor is
missing an OF node. In this case thermal_zone_of_sensor_register() returns
-EINVAL which causes hwmon_thermal_add_sensor() to fail as well. This patch
changes relevant return code of thermal_zone_of_sensor_register() to
-ENODEV, which is tolerated by hwmon_thermal_add_sensor().
Here is a particular case of such behaviour: the Marvell ethernet PHYs
driver registers hwmon device for the built-in temperature sensor (see
drivers/net/phy/marvell.c). Since the sensor doesn't have associated OF
node devm_hwmon_device_register() returns error which ultimately causes
failure of the PHY driver's probe function.
Signed-off-by: Peter Mamonov <pmamonov@gmail.com>
---
drivers/thermal/of-thermal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index dc5093be553e..34b0cc173f4a 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -493,7 +493,7 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
if (!dev || !dev->of_node) {
of_node_put(np);
- return ERR_PTR(-EINVAL);
+ return ERR_PTR(-ENODEV);
}
sensor_np = of_node_get(dev->of_node);
--
2.23.0
^ permalink raw reply related
* Re: [PATCH v3 bpf-next 11/14] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets
From: Andrii Nakryiko @ 2019-09-18 21:42 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: Alexei Starovoitov, Daniel Borkmann, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
john fastabend, open list, Networking, bpf, clang-built-linux,
sergei.shtylyov
In-Reply-To: <20190918110517.GD2908@khorivan>
On Wed, Sep 18, 2019 at 4:05 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> On Tue, Sep 17, 2019 at 10:19:22PM -0700, Andrii Nakryiko wrote:
> >On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
> ><ivan.khoronzhuk@linaro.org> wrote:
> >>
> >> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
> >> correctly to build command, for instance when --sysroot is used or
> >> external libraries are used, like -lelf, wich can be absent in
> >> toolchain. This can be used for samples/bpf cross-compiling allowing
> >> to get elf lib from sysroot.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >> tools/lib/bpf/Makefile | 11 ++++++++---
> >> 1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> >> index c6f94cffe06e..bccfa556ef4e 100644
> >> --- a/tools/lib/bpf/Makefile
> >> +++ b/tools/lib/bpf/Makefile
> >> @@ -94,6 +94,10 @@ else
> >> CFLAGS := -g -Wall
> >> endif
> >>
> >> +ifdef EXTRA_CXXFLAGS
> >> + CXXFLAGS := $(EXTRA_CXXFLAGS)
> >> +endif
> >> +
> >> ifeq ($(feature-libelf-mmap), 1)
> >> override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
> >> endif
> >> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
> >> $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
> >>
> >> $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
> >> - $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> >> - -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> >> + $(QUIET_LINK)$(CC) $(LDFLAGS) \
> >> + --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> >> + -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> >> @ln -sf $(@F) $(OUTPUT)libbpf.so
> >> @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
> >>
> >> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
> >> $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
> >>
> >> $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
> >> - $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
> >> + $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
> >
> >Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include
> >both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing
> >(and is actually recommended my make documentation way to do this).
> It's good practice to follow existent style, I've done similar way as for
> CFLAGS + EXTRACFLAGS here, didn't want to verify it can impact on
> smth else. And my goal is not to correct everything but embed my
> functionality, series tool large w/o it.
Alright, we'll have to eventually clean up this Makefile. What we do
with EXTRA_CFLAGS is not exactly correct, as in this Makefile
EXTRA_CFLAGS are overriding CFLAGS, instead of extending them, which
doesn't seem correct to me. BTW, bpftool does += instead of :=. All
this is avoided by just keeping CFLAGS and EXTRA_CFLAGS separate and
specifying both of them in $(CC)/$(CLANG) invocations. But feel free
to ignore this for now.
>
> >
> >But actually, there is no need to use C++ compiler here,
> >test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and
> >using C compiler instead?
> Seems like, will try in next v.
Thanks!
>
> >
> >>
> >> $(OUTPUT)libbpf.pc:
> >> $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
> >> --
> >> 2.17.1
> >>
>
> --
> Regards,
> Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH V2 net 1/1] net/tls(TLS_SW): Fix list_del double free caused by a race condition in tls_tx_records
From: Jakub Kicinski @ 2019-09-18 21:45 UTC (permalink / raw)
To: Pooja Trivedi
Cc: netdev, davem, daniel, john.fastabend, davejwatson, aviadye,
borisp, Pooja Trivedi, Mallesham Jatharakonda
In-Reply-To: <CAOrEds=DqexwYUOfWQ7_yOxre8ojUTqF3wjxY0SC10CbY8KD0w@mail.gmail.com>
On Wed, 18 Sep 2019 17:37:44 -0400, Pooja Trivedi wrote:
> Hi Jakub,
>
> I have explained one potential way for the race to happen in my
> original message to the netdev mailing list here:
> https://marc.info/?l=linux-netdev&m=156805120229554&w=2
>
> Here is the part out of there that's relevant to your question:
>
> -----------------------------------------
>
> One potential way for race condition to appear:
>
> When under tcp memory pressure, Thread 1 takes the following code path:
> do_sendfile ---> ... ---> .... ---> tls_sw_sendpage --->
> tls_sw_do_sendpage ---> tls_tx_records ---> tls_push_sg --->
> do_tcp_sendpages ---> sk_stream_wait_memory ---> sk_wait_event
Ugh, so do_tcp_sendpages() can also release the lock :/
Since the problem occurs in tls_sw_do_sendpage() and
tls_sw_do_sendmsg() as well, should we perhaps fix it at that level?
^ permalink raw reply
* Re: dsa traffic priorization
From: Florian Fainelli @ 2019-09-18 22:02 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: Sascha Hauer, netdev, Andrew Lunn, Vivien Didelot, kernel
In-Reply-To: <CA+h21hrgODP1VrBrJG6Hy9AE3EqqmzPVtjkBAiNjkm+KkwZLHw@mail.gmail.com>
On 9/18/19 12:39 PM, Vladimir Oltean wrote:
> Hi Florian,
>
> On Wed, 18 Sep 2019 at 20:42, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> On 9/18/19 7:36 AM, Vladimir Oltean wrote:
>>> Hi Sascha,
>>>
>>> On Wed, 18 Sep 2019 at 17:03, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> We have a customer using a Marvell 88e6240 switch with Ethercat on one port and
>>>> regular network traffic on another port. The customer wants to configure two things
>>>> on the switch: First Ethercat traffic shall be priorized over other network traffic
>>>> (effectively prioritizing traffic based on port). Second the ethernet controller
>>>> in the CPU is not able to handle full bandwidth traffic, so the traffic to the CPU
>>>> port shall be rate limited.
>>>>
>>>
>>> You probably already know this, but egress shaping will not drop
>>> frames, just let them accumulate in the egress queue until something
>>> else happens (e.g. queue occupancy threshold triggers pause frames, or
>>> tail dropping is enabled, etc). Is this what you want? It sounds a bit
>>> strange to me to configure egress shaping on the CPU port of a DSA
>>> switch. That literally means you are buffering frames inside the
>>> system. What about ingress policing?
>>
>> Indeed, but I suppose that depending on the switch architecture and/or
>> nomenclature, configuring egress shaping amounts to determining ingress
>> for the ports where the frame is going to be forwarded to.
>
> Egress shaping in the switches I've played with has nothing to do with
> the ingress port, unless that is used as a key for some sort for QoS
> classification (aka selector for a traffic class).
> Furthermore, shaping means queuing (which furthermore means delaying,
> but not dropping except in extreme cases which are outside the scope
> of shaping itself), while policing by definition means early dropping
> (admission control). Like Dave Taht pointed out too, dropping might be
> better for the system's overall latency.
>
>>
>> For instance Broadcom switches rarely if at all mention ingress because
>> the frames have to originate from somewhere and be forwarded to other
>> port(s), therefore, they will egress their original port (which for all
>> practical purposes is the direct continuation of the ingress stage),
>> where shaping happens, which immediately influences the ingress shaping
>> of the destination port, which will egress the frame eventually because
>> packets have to be delivered to the final port's egress queue anyway.
>>
>
> You lost me.
> I have never heard of any shaping done inside the guts of a switch, so
> 'egress of an ingress port' and 'ingress of an egress port' makes no
> sense to me.
What I meant to explain is that when a packet enters the switch, the
forwarding logic will determine the destination port(s) and queue of
that packet. As soon as the egress port and queue is known the capacity
of that port and queue can be looked up, and that has the immediate
effect of changing how the packets are ingress the switch. I suspect
that the Marvell switches (like Broadcom) use the ability to perform any
form of packet mangling from the perspective of the port's egress (as
opposed to ingress) because ultimately packets need to go somewhere and
the switch logic tries as much as possible to "connect" the ingress port
to the egress port logic to limit on-chip buffering.
> I was talking about ingress policing at the front panel ports, for
> their best-effort traffic. I think that is actually preferable to
> egress shaping at the CPU port, since I don't think they would want
> the EtherCAT traffic getting delayed.
I would view this as classifying and putting a higher priority on
specific types of traffic and making sure that the switch is configured
not to drop certain traffic landing in specific queues. Where that is
applied (ingress, or egress) amounts to the same thing IMHO.
> Alternatively, maybe the DSA master port supports per-stream hardware
> policing, although that is more exotic.
Even if the DSA master network device does not support it, the switch
probably does, so you could do it on traffic that ingresses or egresses
the CPU port.
>
>>>
>>>> For reference the patch below configures the switch to their needs. Now the question
>>>> is how this can be implemented in a way suitable for mainline. It looks like the per
>>>> port priority mapping for VLAN tagged packets could be done via ip link add link ...
>>>> ingress-qos-map QOS-MAP. How the default priority would be set is unclear to me.
>>>>
>>>
>>> Technically, configuring a match-all rxnfc rule with ethtool would
>>> count as 'default priority' - I have proposed that before. Now I'm not
>>> entirely sure how intuitive it is, but I'm also interested in being
>>> able to configure this.
>>
>> That does not sound too crazy from my perspective.
>>
>
> Ok, well at least that requires no user space modification, then.
>
>>>
>>>> The other part of the problem seems to be that the CPU port has no network device
>>>> representation in Linux, so there's no interface to configure the egress limits via tc.
>>>> This has been discussed before, but it seems there hasn't been any consensous regarding how
>>>> we want to proceed?
>>
>> You have the DSA master network device which is on the other side of the
>> switch,
>> --
>> Florian
>
> Thanks,
> -Vladimir
>
--
Florian
^ permalink raw reply
* Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?
From: Russell King - ARM Linux admin @ 2019-09-18 22:04 UTC (permalink / raw)
To: George McCollister; +Cc: Florian Fainelli, netdev, Andrew Lunn, Heiner Kallweit
In-Reply-To: <CAFSKS=N_SF-S35eL=tLWOQFKiq7YKKY5B9YT6kxZc0usBayE7w@mail.gmail.com>
On Wed, Sep 18, 2019 at 01:44:33PM -0500, George McCollister wrote:
> Russell,
>
> On Mon, Sep 16, 2019 at 10:40 AM George McCollister
> <george.mccollister@gmail.com> wrote:
> >
> > On Sat, Sep 14, 2019 at 3:49 AM Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Fri, Sep 13, 2019 at 08:31:18PM -0700, Florian Fainelli wrote:
> > > > +Russell, Andrew, Heiner,
> > > >
> > > > On 9/13/2019 9:44 AM, George McCollister wrote:
> > > > > Every example of phylink SFP support I've seen is using an Ethernet
> > > > > MAC with native SGMII.
> > > > > Can phylink facilitate support of Fiber and Copper SFP modules
> > > > > connected to an RGMII MAC if all of the following are true?
> > > >
> > > > I don't think that use case has been presented before, but phylink
> > > > sounds like the tool that should help solve it. From your description
> > > > below, it sounds like all the pieces are there to support it. Is the
> > > > Ethernet MAC driver upstream?
> > >
> > > It has been presented, and it's something I've been trying to support
> > > for the last couple of years - in fact, I have patches in my tree that
> > > support a very similar scenario on the Macchiatobin with the 88x3310
> > > PHYs.
> > >
> > > > > 1) The MAC is connected via RGMII to a transceiver/PHY (such as
> > > > > Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
> > > > > you want to see a block diagram it's the first one here:
> > > > > https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf
> > >
> > > As mentioned above, this is no different from the Macchiatobin,
> > > where we have:
> > >
> > > .-------- RJ45
> > > MAC ---- 88x3310 PHY
> > > `-------- SFP+
> > >
> > > except instead of the MAC to PHY link being 10GBASE-R, it's RGMII,
> > > and the PHY to SFP+ link is 10GBASE-R instead of 1000BASE-X.
>
> Did you test with an SFP+ module that has a PHY?
I think you mean SFP module - SFP+ is for >1G. No, that won't work for
two reasons.
1. The copper SFP module requires SGMII not 1000BASE-X.
2. netdev/phylib doesn't support stacking two PHYs on a single
network device.
(1) can be worked around if the intermediary PHY can be configured for
it, but (2) is a lot harder to resolve, because phylib wants to put
the PHY into the netdev->phydev pointer, and you can't have two of
them.
I think fixing this properly is going to require quite a lot of
re-work, and as I don't have a setup that electrically supports SFP
modules in this setup (only SFP+ modules) it isn't something I'm
able to test.
> In my setup, nothing connects/attaches to the PHY (88E1111) in the
> RJ45 SFP module I'm testing with (is this intended?). Apparently since
> sfp_upstream_ops in the PHY driver used for the first PHY doesn't
> provide a .connect_phy.
Yep, because there's no way to stack phylib managed PHYs.
> This leaves .phy_link_change NULL. Eventually
> phy_link_down tries to call .phy_link_change resulting in a NULL
> pointer deference OOPs. If phy_link_change doesn't need to be called
> for the second PHY I can just send a patch that doesn't call it if
> it's NULL.
SGMII provides a way to discover the negotiated speed and duplex,
but not the pause settings back to the MAC - so in this setup, we
really do need to read the 88E1111 negotiation results (for the
pause settings) and communicate them back to the MAC for things
to work correctly.
It's something that we need to work out how to do...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH v3 2/2] net/ibmvnic: prevent more than one thread from running in reset
From: Juliet Kim @ 2019-09-18 22:21 UTC (permalink / raw)
To: Michael Ellerman, netdev; +Cc: tlfalcon, linuxppc-dev
In-Reply-To: <87ef0ew2so.fsf@mpe.ellerman.id.au>
On 9/18/19 1:12 AM, Michael Ellerman wrote:
> Hi Juliet,
>
> Juliet Kim <julietk@linux.vnet.ibm.com> writes:
>> Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
>> ---
>> drivers/net/ethernet/ibm/ibmvnic.c | 23 ++++++++++++++++++++++-
>> drivers/net/ethernet/ibm/ibmvnic.h | 3 +++
>> 2 files changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
>> index ba340aaff1b3..f344ccd68ad9 100644
>> --- a/drivers/net/ethernet/ibm/ibmvnic.c
>> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
>> @@ -2054,6 +2054,13 @@ static void __ibmvnic_reset(struct work_struct *work)
>>
>> adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
>>
>> + if (adapter->resetting) {
>> + schedule_delayed_work(&adapter->ibmvnic_delayed_reset,
>> + IBMVNIC_RESET_DELAY);
>> + return;
>> + }
>> +
>> + adapter->resetting = true;
>> reset_state = adapter->state;
> Is there some locking/serialisation around this?
>
> Otherwise that looks very racy. ie. two CPUs could both see
> adapter->resetting == false, then both set it to true, and then continue
> executing and stomp on each other.
>
> cheers
I agree there may be a race here. Thank you for reviewing.
I will address it in the next version.
^ permalink raw reply
* Re: [PATCH net 0/3] Fix Qdisc destroy issues caused by adding fine-grained locking to filter API
From: Cong Wang @ 2019-09-18 22:50 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <20190918073201.2320-1-vladbu@mellanox.com>
On Wed, Sep 18, 2019 at 12:32 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> TC filter API unlocking introduced several new fine-grained locks. The
> change caused sleeping-while-atomic BUGs in several Qdiscs that call cls
> APIs which need to obtain new mutex while holding sch tree spinlock. This
> series fixes affected Qdiscs by ensuring that cls API that became sleeping
> is only called outside of sch tree lock critical section.
Sorry I just took a deeper look. It seems harder than just moving it
out of the critical section.
qdisc_destroy() calls ops->reset() which usually purges queues,
I don't see how it is safe to move it out of tree spinlock without
respecting fast path.
What do you think?
^ permalink raw reply
* Re: [PATCH net 2/3] net: sched: multiq: don't call qdisc_put() while holding tree lock
From: Cong Wang @ 2019-09-18 22:56 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <20190918073201.2320-3-vladbu@mellanox.com>
On Wed, Sep 18, 2019 at 12:32 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
> index e1087746f6a2..4cfa9a7bd29e 100644
> --- a/net/sched/sch_multiq.c
> +++ b/net/sched/sch_multiq.c
> @@ -187,18 +187,21 @@ static int multiq_tune(struct Qdisc *sch, struct nlattr *opt,
>
> sch_tree_lock(sch);
> q->bands = qopt->bands;
> + sch_tree_unlock(sch);
> +
> for (i = q->bands; i < q->max_bands; i++) {
> if (q->queues[i] != &noop_qdisc) {
> struct Qdisc *child = q->queues[i];
>
> + sch_tree_lock(sch);
> q->queues[i] = &noop_qdisc;
> qdisc_tree_flush_backlog(child);
> + sch_tree_unlock(sch);
> +
> qdisc_put(child);
> }
> }
Repeatedly acquiring and releasing a spinlock in a loop
does not seem to be a good idea. Is it possible to save
those qdisc pointers to an array or something similar?
Thanks.
^ 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