Re: [net] 5478fcd0f4: BUG:sleeping_function_called_from_invalid_context_at_include/linux/sched/mm.h
Quoting Matthew Wilcox (2021-03-22 10:05:36) > On Mon, Mar 22, 2021 at 09:55:50AM +0100, Antoine Tenart wrote: > > I only had a quick look at this, but I think the issue should be fixed > > with: > > > > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c > > index e16d54aabd4c..3ae3c20eb64c 100644 > > --- a/net/core/net-sysfs.c > > +++ b/net/core/net-sysfs.c > > @@ -1378,7 +1378,7 @@ static ssize_t xps_queue_show(struct net_device *dev, > > unsigned int index, > > nr_ids = dev_maps ? dev_maps->nr_ids : > > (type == XPS_CPUS ? nr_cpu_ids : dev->num_rx_queues); > > > > - mask = bitmap_zalloc(nr_ids, GFP_KERNEL); > > + mask = bitmap_zalloc(nr_ids, GFP_ATOMIC); > > if (!mask) { > > rcu_read_unlock(); > > return -ENOMEM; > > sysfs isn't a good reason to use GFP_ATOMIC. > > try something like this: > > - mask = bitmap_zalloc(nr_ids, GFP_KERNEL); > + mask = bitmap_zalloc(nr_ids, GFP_NOWAIT); > if (!mask) { > + int new_nr_ids; > + > rcu_read_unlock(); > - return -ENOMEM; > + mask = bitmap_zalloc(nr_ids, GFP_KERNEL); > + if (!mask) > + return -ENOMEM; > + rcu_read_lock(); > + dev_maps = rcu_dereference(dev->xps_maps[type]); > + /* if nr_ids shrank while we slept, do not overrun array. > +* if it increased, we just won't show the new ones > +*/ > + new_nr_ids = dev_maps ? dev_maps->nr_ids : > + (type == XPS_CPUS ? nr_cpu_ids : dev->num_rx_queues); > + if (new_nr_ids < nr_ids) > + nr_ids = new_nr_ids; Thanks for the suggestion, I'll look into that. We could also just return -ENOMEM if the first allocation fails, retrying adds a lot of complexity. Antoine
Re: [net] 5478fcd0f4: BUG:sleeping_function_called_from_invalid_context_at_include/linux/sched/mm.h
I only had a quick look at this, but I think the issue should be fixed with: diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index e16d54aabd4c..3ae3c20eb64c 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -1378,7 +1378,7 @@ static ssize_t xps_queue_show(struct net_device *dev, unsigned int index, nr_ids = dev_maps ? dev_maps->nr_ids : (type == XPS_CPUS ? nr_cpu_ids : dev->num_rx_queues); - mask = bitmap_zalloc(nr_ids, GFP_KERNEL); + mask = bitmap_zalloc(nr_ids, GFP_ATOMIC); if (!mask) { rcu_read_unlock(); return -ENOMEM; I'll run some tests and send a fix. Antoine Quoting kernel test robot (2021-03-22 09:35:53) > > FYI, we noticed the following commit (built with gcc-9): > > commit: 5478fcd0f48322e04ae6c173ad3a1959e066dc83 ("net: embed nr_ids in the > xps maps") > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master > > > in testcase: ltp > version: ltp-x86_64-14c1f76-1_20210319 > with following parameters: > > disk: 1HDD > fs: xfs > test: fs-02 > ucode: 0xde > > test-description: The LTP testsuite contains a collection of tools for > testing the Linux kernel and related features. > test-url: http://linux-test-project.github.io/ > > > on test machine: 8 threads Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz with 32G > memory > > caused below changes (please refer to attached dmesg/kmsg for entire > log/backtrace): > > > > If you fix the issue, kindly add following tag > Reported-by: kernel test robot > > > [ 253.104647] BUG: sleeping function called from invalid context at > include/linux/sched/mm.h:196 > [ 253.113269] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 9371, > name: read_all > [ 253.121296] CPU: 1 PID: 9371 Comm: read_all Tainted: G I > 5.12.0-rc2-00796-g5478fcd0f483 #1 > [ 253.130887] Hardware name: Dell Inc. OptiPlex 7050/062KRH, BIOS 1.2.0 > 12/22/2016 > [ 253.138288] Call Trace: > [ 253.140734] dump_stack (kbuild/src/consumer/lib/dump_stack.c:122) > [ 253.144055] ___might_sleep.cold > (kbuild/src/consumer/kernel/sched/core.c:8331 > kbuild/src/consumer/kernel/sched/core.c:8288) > [ 253.148161] __kmalloc (kbuild/src/consumer/include/linux/sched/mm.h:196 > kbuild/src/consumer/mm/slab.h:497 kbuild/src/consumer/mm/slub.c:2826 > kbuild/src/consumer/mm/slub.c:2915 kbuild/src/consumer/mm/slub.c:4051) > [ 253.151568] ? xps_rxqs_show (kbuild/src/consumer/net/core/net-sysfs.c:1498 > (discriminator 2)) > [ 253.155498] xps_rxqs_show (kbuild/src/consumer/net/core/net-sysfs.c:1498 > (discriminator 2)) > [ 253.159255] sysfs_kf_seq_show (kbuild/src/consumer/fs/sysfs/file.c:62) > [ 253.163273] seq_read_iter (kbuild/src/consumer/fs/seq_file.c:227) > [ 253.167030] new_sync_read (kbuild/src/consumer/fs/read_write.c:416 > (discriminator 1)) > [ 253.170787] vfs_read (kbuild/src/consumer/fs/read_write.c:496) > [ 253.174105] ksys_read (kbuild/src/consumer/fs/read_write.c:634) > [ 253.177334] do_syscall_64 (kbuild/src/consumer/arch/x86/entry/common.c:46) > [ 253.180923] entry_SYSCALL_64_after_hwframe > (kbuild/src/consumer/arch/x86/entry/entry_64.S:112) > [ 253.185978] RIP: 0033:0x7fa906c7c50e > [ 253.189555] Code: 48 8b 15 cd aa 00 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff > eb b6 0f 1f 80 00 00 00 00 8b 05 3a ef 00 00 85 c0 75 16 31 c0 0f 05 <48> 3d > 00 f0 ff ff 77 5a c3 66 0f 1f 84 00 00 00 00 00 41 54 49 89 > All code > >0: 48 8b 15 cd aa 00 00mov0xaacd(%rip),%rdx# 0xaad4 >7: f7 d8 neg%eax >9: 64 89 02mov%eax,%fs:(%rdx) >c: 48 c7 c0 ff ff ff ffmov$0x,%rax > 13: eb b6 jmp0xffcb > 15: 0f 1f 80 00 00 00 00nopl 0x0(%rax) > 1c: 8b 05 3a ef 00 00 mov0xef3a(%rip),%eax# 0xef5c > 22: 85 c0 test %eax,%eax > 24: 75 16 jne0x3c > 26: 31 c0 xor%eax,%eax > 28: 0f 05 syscall > 2a:* 48 3d 00 f0 ff ff cmp$0xf000,%rax <-- > trapping instruction > 30: 77 5a ja 0x8c > 32: c3 retq > 33: 66 0f 1f 84 00 00 00nopw 0x0(%rax,%rax,1) > 3a: 00 00 > 3c: 41 54 push %r12 > 3e: 49 rex.WB > 3f: 89 .byte 0x89 > > Code starting with the faulting instruction > === >0: 48 3d 00 f0 ff ff cmp$0xfff
Re: [PATCH] crypto: inside-secure: Minor typo fix in the file safexcel.c
Quoting Bhaskar Chowdhury (2021-03-17 10:14:45) > > s/procesing/processing/ > > Signed-off-by: Bhaskar Chowdhury Acked-by: Antoine Tenart Thanks, Antoine > --- > drivers/crypto/inside-secure/safexcel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/inside-secure/safexcel.c > b/drivers/crypto/inside-secure/safexcel.c > index 6364583b88b2..9ff885d50edf 100644 > --- a/drivers/crypto/inside-secure/safexcel.c > +++ b/drivers/crypto/inside-secure/safexcel.c > @@ -688,7 +688,7 @@ static int safexcel_hw_init(struct safexcel_crypto_priv > *priv) > /* Leave the DSE threads reset state */ > writel(0, EIP197_HIA_DSE_THR(priv) + > EIP197_HIA_DSE_THR_CTRL(pe)); > > - /* Configure the procesing engine thresholds */ > + /* Configure the processing engine thresholds */ > writel(EIP197_PE_OUT_DBUF_THRES_MIN(opbuflo) | >EIP197_PE_OUT_DBUF_THRES_MAX(opbufhi), >EIP197_PE(priv) + EIP197_PE_OUT_DBUF_THRES(pe)); > -- > 2.30.2 >
Re: [PATCH] reset: berlin: Put parent device node on error path
Hello, Quoting Pan Bian (2021-01-21 16:11:26) > Put parent device node parent_np if there is no enough memory. > > Fixes: aed6f3cadc86 ("reset: berlin: convert to a platform driver") > Signed-off-by: Pan Bian > --- > drivers/reset/reset-berlin.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/reset/reset-berlin.c b/drivers/reset/reset-berlin.c > index 371197bbd055..cae58e40f639 100644 > --- a/drivers/reset/reset-berlin.c > +++ b/drivers/reset/reset-berlin.c > @@ -72,8 +72,10 @@ static int berlin2_reset_probe(struct platform_device > *pdev) > struct berlin_reset_priv *priv; > > priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > - if (!priv) > + if (!priv) { > + of_node_put(parent_np); > return -ENOMEM; > + } You could also move the of_get_parent() call here, to avoid having to handle parent_np in an error path. That would improve maintainability imho. Thanks, Antoine > priv->regmap = syscon_node_to_regmap(parent_np); > of_node_put(parent_np);
Re: [PATCH net] net: mvpp2: Add TCAM entry to drop flow control pause frames
Quoting stef...@marvell.com (2020-12-17 18:45:06) > From: Stefan Chulski > > Issue: > Flow control frame used to pause GoP(MAC) was delivered to the CPU > and created a load on the CPU. Since XOFF/XON frames are used only > by MAC, these frames should be dropped inside MAC. > > Fix: > According to 802.3-2012 - IEEE Standard for Ethernet pause frame > has unique destination MAC address 01-80-C2-00-00-01. > Add TCAM parser entry to track and drop pause frames by destination MAC. > > Fixes: db9d7d36eecc ("net: mvpp2: Split the PPv2 driver to a dedicated > directory") Same here, you should go further in the git history. Also, was that introduced when the TCAM support landed in (overriding its default configuration?)? Or is that the behaviour since the beginning? I'm asking because while this could very be a fix, it could also fall in the improvements category. Thanks! Antoine > Signed-off-by: Stefan Chulski > --- > drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c | 34 > ++ > drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.h | 2 +- > 2 files changed, 35 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c > index 1a272c2..3a9c747 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c > @@ -405,6 +405,39 @@ static int mvpp2_prs_tcam_first_free(struct mvpp2 *priv, > unsigned char start, > return -EINVAL; > } > > +/* Drop flow control pause frames */ > +static void mvpp2_prs_drop_fc(struct mvpp2 *priv) > +{ > + struct mvpp2_prs_entry pe; > + unsigned int len; > + unsigned char da[ETH_ALEN] = { > + 0x01, 0x80, 0xC2, 0x00, 0x00, 0x01 }; > + > + memset(&pe, 0, sizeof(pe)); > + > + /* For all ports - drop flow control frames */ > + pe.index = MVPP2_PE_FC_DROP; > + mvpp2_prs_tcam_lu_set(&pe, MVPP2_PRS_LU_MAC); > + > + /* Set match on DA */ > + len = ETH_ALEN; > + while (len--) > + mvpp2_prs_tcam_data_byte_set(&pe, len, da[len], 0xff); > + > + mvpp2_prs_sram_ri_update(&pe, MVPP2_PRS_RI_DROP_MASK, > +MVPP2_PRS_RI_DROP_MASK); > + > + mvpp2_prs_sram_bits_set(&pe, MVPP2_PRS_SRAM_LU_GEN_BIT, 1); > + mvpp2_prs_sram_next_lu_set(&pe, MVPP2_PRS_LU_FLOWS); > + > + /* Mask all ports */ > + mvpp2_prs_tcam_port_map_set(&pe, MVPP2_PRS_PORT_MASK); > + > + /* Update shadow table and hw entry */ > + mvpp2_prs_shadow_set(priv, pe.index, MVPP2_PRS_LU_MAC); > + mvpp2_prs_hw_write(priv, &pe); > +} > + > /* Enable/disable dropping all mac da's */ > static void mvpp2_prs_mac_drop_all_set(struct mvpp2 *priv, int port, bool > add) > { > @@ -1168,6 +1201,7 @@ static void mvpp2_prs_mac_init(struct mvpp2 *priv) > mvpp2_prs_hw_write(priv, &pe); > > /* Create dummy entries for drop all and promiscuous modes */ > + mvpp2_prs_drop_fc(priv); > mvpp2_prs_mac_drop_all_set(priv, 0, false); > mvpp2_prs_mac_promisc_set(priv, 0, MVPP2_PRS_L2_UNI_CAST, false); > mvpp2_prs_mac_promisc_set(priv, 0, MVPP2_PRS_L2_MULTI_CAST, false); > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.h > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.h > index e22f6c8..4b68dd3 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.h > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.h > @@ -129,7 +129,7 @@ > #define MVPP2_PE_VID_EDSA_FLTR_DEFAULT (MVPP2_PRS_TCAM_SRAM_SIZE - 7) > #define MVPP2_PE_VLAN_DBL (MVPP2_PRS_TCAM_SRAM_SIZE - 6) > #define MVPP2_PE_VLAN_NONE (MVPP2_PRS_TCAM_SRAM_SIZE - 5) > -/* reserved */ > +#define MVPP2_PE_FC_DROP (MVPP2_PRS_TCAM_SRAM_SIZE - 4) > #define MVPP2_PE_MAC_MC_PROMISCUOUS(MVPP2_PRS_TCAM_SRAM_SIZE - 3) > #define MVPP2_PE_MAC_UC_PROMISCUOUS(MVPP2_PRS_TCAM_SRAM_SIZE - 2) > #define MVPP2_PE_MAC_NON_PROMISCUOUS (MVPP2_PRS_TCAM_SRAM_SIZE - 1) > -- > 1.9.1 >
Re: [PATCH net] net: mvpp2: prs: fix PPPoE with ipv6 packet parse
Hi Stefan, Quoting stef...@marvell.com (2020-12-17 18:23:11) > From: Stefan Chulski > > Current PPPoE+IPv6 entry is jumping to 'next-hdr' > field and not to 'DIP' field as done for IPv4. > > Fixes: db9d7d36eecc ("net: mvpp2: Split the PPv2 driver to a dedicated > directory") That's not the commit introducing the issue. You can use `git log --follow` to go further back (or directly pointing to the old mvpp2.c file). Thanks! Antoine > Reported-by: Liron Himi > Signed-off-by: Stefan Chulski > --- > drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c > index b9e5b08..1a272c2 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c > @@ -1655,8 +1655,9 @@ static int mvpp2_prs_pppoe_init(struct mvpp2 *priv) > mvpp2_prs_sram_next_lu_set(&pe, MVPP2_PRS_LU_IP6); > mvpp2_prs_sram_ri_update(&pe, MVPP2_PRS_RI_L3_IP6, > MVPP2_PRS_RI_L3_PROTO_MASK); > - /* Skip eth_type + 4 bytes of IPv6 header */ > - mvpp2_prs_sram_shift_set(&pe, MVPP2_ETH_TYPE_LEN + 4, > + /* Jump to DIP of IPV6 header */ > + mvpp2_prs_sram_shift_set(&pe, MVPP2_ETH_TYPE_LEN + 8 + > +MVPP2_MAX_L3_ADDR_SIZE, > MVPP2_PRS_SRAM_OP_SEL_SHIFT_ADD); > /* Set L3 offset */ > mvpp2_prs_sram_offset_set(&pe, MVPP2_PRS_SRAM_UDF_TYPE_L3, > -- > 1.9.1 >
Re: [PATCH] irqchip/alpine-msi: Fix freeing of interrupts on allocation error path
Hello Marc, Quoting Marc Zyngier (2020-11-29 14:55:25) > The alpine-msi driver has an interesting allocation error handling, > where it frees the same interrupts repeatedly. Hilarity follows. That's interesting indeed... > This code is probably never executed, but let's fix it nonetheless. > > Fixes: e6b78f2c3e14 ("irqchip: Add the Alpine MSIX interrupt controller") > Cc: Tsahee Zidenberg > Cc: Antoine Tenart > Signed-off-by: Marc Zyngier Reviewed-by: Antoine Tenart Thanks, Antoine > --- > drivers/irqchip/irq-alpine-msi.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-alpine-msi.c > b/drivers/irqchip/irq-alpine-msi.c > index 23a3b877f7f1..ede02dc2bcd0 100644 > --- a/drivers/irqchip/irq-alpine-msi.c > +++ b/drivers/irqchip/irq-alpine-msi.c > @@ -165,8 +165,7 @@ static int alpine_msix_middle_domain_alloc(struct > irq_domain *domain, > return 0; > > err_sgi: > - while (--i >= 0) > - irq_domain_free_irqs_parent(domain, virq, i); > + irq_domain_free_irqs_parent(domain, virq, i - 1); > alpine_msix_free_sgi(priv, sgi, nr_irqs); > return err; > } > -- > 2.29.2 >
Re: [PATCH net-next 3/3] net: phy: mscc: use new PTP_MSGTYPE_* defines
Hello Christian, Quoting Christian Eggers (2020-11-22 09:26:36) > Use recently introduced PTP_MSGTYPE_SYNC and PTP_MSGTYPE_DELAY_REQ > defines instead of a driver internal enumeration. > > Signed-off-by: Christian Eggers Reviewed-by: Antoine Tenart Thanks! Antoine > Cc: Quentin Schulz > Cc: Antoine Tenart > Cc: Antoine Tenart > --- > drivers/net/phy/mscc/mscc_ptp.c | 14 +++--- > drivers/net/phy/mscc/mscc_ptp.h | 5 - > 2 files changed, 7 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c > index d8a61456d1ce..924ed5b034a4 100644 > --- a/drivers/net/phy/mscc/mscc_ptp.c > +++ b/drivers/net/phy/mscc/mscc_ptp.c > @@ -506,9 +506,9 @@ static int vsc85xx_ptp_cmp_init(struct phy_device > *phydev, enum ts_blk blk) > { > struct vsc8531_private *vsc8531 = phydev->priv; > bool base = phydev->mdio.addr == vsc8531->ts_base_addr; > - enum vsc85xx_ptp_msg_type msgs[] = { > - PTP_MSG_TYPE_SYNC, > - PTP_MSG_TYPE_DELAY_REQ > + u8 msgs[] = { > + PTP_MSGTYPE_SYNC, > + PTP_MSGTYPE_DELAY_REQ > }; > u32 val; > u8 i; > @@ -847,9 +847,9 @@ static int vsc85xx_ts_ptp_action_flow(struct phy_device > *phydev, enum ts_blk blk > static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk, > bool one_step, bool enable) > { > - enum vsc85xx_ptp_msg_type msgs[] = { > - PTP_MSG_TYPE_SYNC, > - PTP_MSG_TYPE_DELAY_REQ > + u8 msgs[] = { > + PTP_MSGTYPE_SYNC, > + PTP_MSGTYPE_DELAY_REQ > }; > u32 val; > u8 i; > @@ -858,7 +858,7 @@ static int vsc85xx_ptp_conf(struct phy_device *phydev, > enum ts_blk blk, > if (blk == INGRESS) > vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i], >PTP_WRITE_NS); > - else if (msgs[i] == PTP_MSG_TYPE_SYNC && one_step) > + else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step) > /* no need to know Sync t when sending in one_step */ > vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i], >PTP_WRITE_1588); > diff --git a/drivers/net/phy/mscc/mscc_ptp.h b/drivers/net/phy/mscc/mscc_ptp.h > index 3ea163af0f4f..da3465360e90 100644 > --- a/drivers/net/phy/mscc/mscc_ptp.h > +++ b/drivers/net/phy/mscc/mscc_ptp.h > @@ -436,11 +436,6 @@ enum ptp_cmd { > PTP_SAVE_IN_TS_FIFO = 11, /* invalid when writing in reg */ > }; > > -enum vsc85xx_ptp_msg_type { > - PTP_MSG_TYPE_SYNC, > - PTP_MSG_TYPE_DELAY_REQ, > -}; > - > struct vsc85xx_ptphdr { > u8 tsmt; /* transportSpecific | messageType */ > u8 ver; /* reserved0 | versionPTP */ > -- > Christian Eggers > Embedded software developer >
Re: [PATCH v2] net: phy: mscc: fix excluded_middle.cocci warnings
Hello Julia, Quoting Julia Lawall (2020-11-16 16:34:44) > From: kernel test robot > > Condition !A || A && B is equivalent to !A || B. > > Generated by: scripts/coccinelle/misc/excluded_middle.cocci > > Fixes: b76f0ea01312 ("coccinelle: misc: add excluded_middle.cocci script") > CC: Denis Efremov > Reported-by: kernel test robot > Signed-off-by: kernel test robot > Signed-off-by: Julia Lawall Reviewed-by: Antoine Tenart Thanks! Antoine > --- > > v2: add netdev mailing list > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > master > head: e28c0d7c92c89016c12a677616668957351e7542 > commit: b76f0ea013125358d1b4ca147a6f9b6883dd2493 coccinelle: misc: add > excluded_middle.cocci script > :: branch date: 8 hours ago > :: commit date: 8 weeks ago > > Please take the patch only if it's a positive warning. Thanks! > > mscc_ptp.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- a/drivers/net/phy/mscc/mscc_ptp.c > +++ b/drivers/net/phy/mscc/mscc_ptp.c > @@ -136,7 +136,7 @@ static void vsc85xx_ts_write_csr(struct > > phy_ts_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_1588); > > - if (!cond || (cond && upper)) > + if (!cond || upper) > phy_ts_base_write(phydev, MSCC_PHY_TS_CSR_DATA_MSB, upper); > > phy_ts_base_write(phydev, MSCC_PHY_TS_CSR_DATA_LSB, lower);
Re: [PATCH net v2] net: phy: mscc: remove non-MACSec compatible phy
Quoting Steen Hegelund (2020-11-13 10:11:16) > Selecting VSC8575 as a MACSec PHY was not correct > > The relevant datasheet can be found here: > - VSC8575: https://www.microchip.com/wwwproducts/en/VSC8575 > > History: > v1 -> v2: >- Corrected the sha in the "Fixes:" tag > > Fixes: 1bbe0ecc2a1a ("net: phy: mscc: macsec initialization") > Signed-off-by: Steen Hegelund Reviewed-by: Antoine Tenart Small comment: you can put the commit history after the --- so it doesn't end-up in the commit log (except when it's relevant, which isn't the case here). I don't think that's a blocker though. Thanks Steen! Antoine > --- > drivers/net/phy/mscc/mscc_macsec.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/net/phy/mscc/mscc_macsec.c > b/drivers/net/phy/mscc/mscc_macsec.c > index 1d4c012194e9..72292bf6c51c 100644 > --- a/drivers/net/phy/mscc/mscc_macsec.c > +++ b/drivers/net/phy/mscc/mscc_macsec.c > @@ -981,7 +981,6 @@ int vsc8584_macsec_init(struct phy_device *phydev) > > switch (phydev->phy_id & phydev->drv->phy_id_mask) { > case PHY_ID_VSC856X: > - case PHY_ID_VSC8575: > case PHY_ID_VSC8582: > case PHY_ID_VSC8584: > INIT_LIST_HEAD(&vsc8531->macsec_flows); > -- > 2.29.2 >
Re: [net v2] net: phy: mscc: adjust the phy support for PTP and MACsec
Hi Steen, Quoting Steen Hegelund (2020-11-11 16:17:53) > The MSCC PHYs selected for PTP and MACSec was not correct > > - PTP > - Add VSC8572 and VSC8574 > > - MACsec > - Removed VSC8575 > > The relevant datasheets can be found here: > - VSC8572: https://www.microchip.com/wwwproducts/en/VSC8572 > - VSC8574: https://www.microchip.com/wwwproducts/en/VSC8574 > - VSC8575: https://www.microchip.com/wwwproducts/en/VSC8575 > > History: > v1 -> v2: > - Added "fixes:" tags to the commit message > > Fixes: bb56c016a1257 ("net: phy: mscc: split the driver into separate files") This commit splitting the driver didn't introduced the issue, it only moved code around. You can remove this Fixes tag. (You usually/should have a single Fixes tag per patch). > Fixes: ab2bf93393571 ("net: phy: mscc: 1588 block initialization") The PTP and the MACsec support were introduced in separate patches (and were introduced in different releases of the kernel). This patch is fixing two different issues then, and its changes can't apply to the same kernel versions. You should send them in two separate patches. With the changes sent in two different patches, I would suggest to only send the MACsec one as a fix for net (it's really fixing something, by removing a non-compatible PHY from using MACsec) and the PTP one for net-next as it's adding PTP support for two new PHYs (not fixing anything). When you do so, please use the following commands to format the patches, to end up with the correct prefix in the subject: git format-patch --subject-prefix='PATCH net' ... git format-patch --subject-prefix='PATCH net-next' ... Thanks! Antoine > Signed-off-by: Steen Hegelund > --- > drivers/net/phy/mscc/mscc_macsec.c | 1 - > drivers/net/phy/mscc/mscc_ptp.c| 2 ++ > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/mscc/mscc_macsec.c > b/drivers/net/phy/mscc/mscc_macsec.c > index 1d4c012194e9..72292bf6c51c 100644 > --- a/drivers/net/phy/mscc/mscc_macsec.c > +++ b/drivers/net/phy/mscc/mscc_macsec.c > @@ -981,7 +981,6 @@ int vsc8584_macsec_init(struct phy_device *phydev) > > switch (phydev->phy_id & phydev->drv->phy_id_mask) { > case PHY_ID_VSC856X: > - case PHY_ID_VSC8575: > case PHY_ID_VSC8582: > case PHY_ID_VSC8584: > INIT_LIST_HEAD(&vsc8531->macsec_flows); > diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c > index b97ee79f3cdf..f0537299c441 100644 > --- a/drivers/net/phy/mscc/mscc_ptp.c > +++ b/drivers/net/phy/mscc/mscc_ptp.c > @@ -1510,6 +1510,8 @@ void vsc8584_config_ts_intr(struct phy_device *phydev) > int vsc8584_ptp_init(struct phy_device *phydev) > { > switch (phydev->phy_id & phydev->drv->phy_id_mask) { > + case PHY_ID_VSC8572: > + case PHY_ID_VSC8574: > case PHY_ID_VSC8575: > case PHY_ID_VSC8582: > case PHY_ID_VSC8584: > -- > 2.29.2 >
Re: [net] net: phy: mscc: adjust the phy support for PTP and MACsec
Hi Steen! Either this is a fix and it would need a Fixes: tag in addition to the Signed-off-by: one (you can have a look at the git history to see what is the format); or the patch is not a fix and then it should have [net-next] in its subject instead of [net]. Please have a look at the relevant documentation, https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html Thanks! Antoine Quoting Steen Hegelund (2020-11-11 10:55:11) > The MSCC PHYs selected for PTP and MACSec was not correct > > - PTP > - Add VSC8572 and VSC8574 > > - MACsec > - Removed VSC8575 > > The relevant datasheets can be found here: > - VSC8572: https://www.microchip.com/wwwproducts/en/VSC8572 > - VSC8574: https://www.microchip.com/wwwproducts/en/VSC8574 > - VSC8575: https://www.microchip.com/wwwproducts/en/VSC8575 > > Signed-off-by: Steen Hegelund > --- > drivers/net/phy/mscc/mscc_macsec.c | 1 - > drivers/net/phy/mscc/mscc_ptp.c| 2 ++ > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/mscc/mscc_macsec.c > b/drivers/net/phy/mscc/mscc_macsec.c > index 1d4c012194e9..72292bf6c51c 100644 > --- a/drivers/net/phy/mscc/mscc_macsec.c > +++ b/drivers/net/phy/mscc/mscc_macsec.c > @@ -981,7 +981,6 @@ int vsc8584_macsec_init(struct phy_device *phydev) > > switch (phydev->phy_id & phydev->drv->phy_id_mask) { > case PHY_ID_VSC856X: > - case PHY_ID_VSC8575: > case PHY_ID_VSC8582: > case PHY_ID_VSC8584: > INIT_LIST_HEAD(&vsc8531->macsec_flows); > diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c > index b97ee79f3cdf..f0537299c441 100644 > --- a/drivers/net/phy/mscc/mscc_ptp.c > +++ b/drivers/net/phy/mscc/mscc_ptp.c > @@ -1510,6 +1510,8 @@ void vsc8584_config_ts_intr(struct phy_device *phydev) > int vsc8584_ptp_init(struct phy_device *phydev) > { > switch (phydev->phy_id & phydev->drv->phy_id_mask) { > + case PHY_ID_VSC8572: > + case PHY_ID_VSC8574: > case PHY_ID_VSC8575: > case PHY_ID_VSC8582: > case PHY_ID_VSC8584: > -- > 2.29.2 >
Re: [PATCH] crypto: inside-secure: Fix sizeof() mismatch
Hello, Quoting Colin King (2020-10-10 18:47:36) > From: Colin Ian King > > An incorrect sizeof() is being used, sizeof(priv->ring[i].rdr_req) is > not correct, it should be sizeof(*priv->ring[i].rdr_req). Note that > since the size of ** is the same size as * this is not causing any > issues. > > Addresses-Coverity: ("Sizeof not portable (SIZEOF_MISMATCH)") > Fixes: 9744fec95f06 ("crypto: inside-secure - remove request list to improve > performance") > Signed-off-by: Colin Ian King Acked-by: Antoine Tenart Thanks! Antoine > --- > drivers/crypto/inside-secure/safexcel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/inside-secure/safexcel.c > b/drivers/crypto/inside-secure/safexcel.c > index eb2418450f12..2e1562108a85 100644 > --- a/drivers/crypto/inside-secure/safexcel.c > +++ b/drivers/crypto/inside-secure/safexcel.c > @@ -1639,7 +1639,7 @@ static int safexcel_probe_generic(void *pdev, > > priv->ring[i].rdr_req = devm_kcalloc(dev, > EIP197_DEFAULT_RING_SIZE, > - sizeof(priv->ring[i].rdr_req), > + sizeof(*priv->ring[i].rdr_req), > GFP_KERNEL); > if (!priv->ring[i].rdr_req) > return -ENOMEM; > -- > 2.27.0 >
[PATCH] MAINTAINERS: update my email address
Use my kernel.org address instead of my bootlin.com one. Signed-off-by: Antoine Tenart --- .mailmap| 3 ++- MAINTAINERS | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.mailmap b/.mailmap index a780211468e4..a11a8b9d1e43 100644 --- a/.mailmap +++ b/.mailmap @@ -41,7 +41,8 @@ Andrew Murray Andrew Vasquez Andrey Ryabinin Andy Adamson -Antoine Tenart +Antoine Tenart +Antoine Tenart Antonio Ospite Archit Taneja Ard Biesheuvel diff --git a/MAINTAINERS b/MAINTAINERS index 33b27e62ce19..dc1e29b94958 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1623,7 +1623,7 @@ N:meson ARM/Annapurna Labs ALPINE ARCHITECTURE M: Tsahee Zidenberg -M: Antoine Tenart +M: Antoine Tenart L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) S: Maintained F: arch/arm/boot/dts/alpine* @@ -8673,7 +8673,7 @@ F:drivers/input/input-mt.c K: \b(ABS|SYN)_MT_ INSIDE SECURE CRYPTO DRIVER -M: Antoine Tenart +M: Antoine Tenart L: linux-cry...@vger.kernel.org S: Maintained F: drivers/crypto/inside-secure/ -- 2.26.2
Greetings from Mrs Elodie Antoine,
Greetings from Mrs Elodie Antoine, Calvary Greetings in the name of the LORD Almighty and Our LORD JESUS CHRIST the giver of every good thing. Good day,i know this letter will definitely come to you as a huge surprise, but I implore you to take the time to go through it carefully as the decision you make will go off a long way to determine my future and continued existence. I am Mrs Elodie Antoine aging widow of 59 years old suffering from long time illness. I have some funds I inherited from my late husband, The sum of (US$4.5 Million Dollars) and I needed a very honest and God fearing who can withdraw this money then use the funds for Charity works. I WISH TO GIVE THIS FUNDS TO YOU FOR CHARITY WORKS. I found your email address from the internet after honest prayers to the LORD to bring me a helper and i decided to contact you if you may be willing and interested to handle these trust funds in good faith before anything happens to me. I accept this decision because I do not have any child who will inherit this money after I die. I want your urgent reply to me so that I will give you the deposit receipt which the COMPANY issued to me as next of kin for immediate transfer of the money to your account in your country, to start the good work of God, I want you to use the 15/percent of the total amount to help yourself in doing the project. I am desperately in keen need of assistance and I have summoned up courage to contact you for this task, you must not fail me and the millions of the poor people in our todays WORLD. This is no stolen money and there are no dangers involved,100% RISK FREE with full legal proof. Please if you would be able to use the funds for the Charity works kindly let me know immediately.I will appreciate your utmost confidentiality and trust in this matter to accomplish my heart desire, as I don't want anything that will jeopardize my last wish. I want you to take 15 percent of the total money for your personal use while 85% of the money will go to charity.I will appreciate your utmost confidentiality and trust in this matter to accomplish my heart desire, as I don't want anything that will jeopardize my last wish. kindly respond for further details. Thanks and God bless you, Mrs Elodie Antoine
Re: [PATCH v2 1/4] crypto: inside-secure - use kfree_sensitive()
Hello Denis, Quoting Denis Efremov (2020-08-27 08:43:59) > Use kfree_sensitive() instead of open-coding it. > > Signed-off-by: Denis Efremov Acked-by: Antoine Tenart Thanks! Antoine > --- > drivers/crypto/inside-secure/safexcel_hash.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/crypto/inside-secure/safexcel_hash.c > b/drivers/crypto/inside-secure/safexcel_hash.c > index 16a467969d8e..5ffdc1cd5847 100644 > --- a/drivers/crypto/inside-secure/safexcel_hash.c > +++ b/drivers/crypto/inside-secure/safexcel_hash.c > @@ -1082,8 +1082,7 @@ static int safexcel_hmac_init_pad(struct ahash_request > *areq, > } > > /* Avoid leaking */ > - memzero_explicit(keydup, keylen); > - kfree(keydup); > + kfree_sensitive(keydup); > > if (ret) > return ret; > -- > 2.26.2 > -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Greetings from Mrs Elodie Antoine,
Greetings from Mrs Elodie Antoine, Calvary Greetings in the name of the LORD Almighty and Our LORD JESUS CHRIST the giver of every good thing. Good day,i know this letter will definitely come to you as a huge surprise, but I implore you to take the time to go through it carefully as the decision you make will go off a long way to determine my future and continued existence. I am Mrs Elodie Antoine aging widow of 59 years old suffering from long time illness. I have some funds I inherited from my late husband, The sum of (US$4.5 Million Dollars) and I needed a very honest and God fearing who can withdraw this money then use the funds for Charity works. I WISH TO GIVE THIS FUNDS TO YOU FOR CHARITY WORKS. I found your email address from the internet after honest prayers to the LORD to bring me a helper and i decided to contact you if you may be willing and interested to handle these trust funds in good faith before anything happens to me. I accept this decision because I do not have any child who will inherit this money after I die. I want your urgent reply to me so that I will give you the deposit receipt which the COMPANY issued to me as next of kin for immediate transfer of the money to your account in your country, to start the good work of God, I want you to use the 15/percent of the total amount to help yourself in doing the project. I am desperately in keen need of assistance and I have summoned up courage to contact you for this task, you must not fail me and the millions of the poor people in our todays WORLD. This is no stolen money and there are no dangers involved,100% RISK FREE with full legal proof. Please if you would be able to use the funds for the Charity works kindly let me know immediately.I will appreciate your utmost confidentiality and trust in this matter to accomplish my heart desire, as I don't want anything that will jeopardize my last wish. I want you to take 15 percent of the total money for your personal use while 85% of the money will go to charity.I will appreciate your utmost confidentiality and trust in this matter to accomplish my heart desire, as I don't want anything that will jeopardize my last wish. kindly respond for further details. Thanks and God bless you, Mrs Elodie Antoine
Greetings from Mrs Elodie Antoine,
Greetings from Mrs Elodie Antoine, Calvary Greetings in the name of the LORD Almighty and Our LORD JESUS CHRIST the giver of every good thing. Good day,i know this letter will definitely come to you as a huge surprise, but I implore you to take the time to go through it carefully as the decision you make will go off a long way to determine my future and continued existence. I am Mrs Elodie Antoine aging widow of 59 years old suffering from long time illness. I have some funds I inherited from my late husband, The sum of (US$4.5 Million Dollars) and I needed a very honest and God fearing who can withdraw this money then use the funds for Charity works. I WISH TO GIVE THIS FUNDS TO YOU FOR CHARITY WORKS. I found your email address from the internet after honest prayers to the LORD to bring me a helper and i decided to contact you if you may be willing and interested to handle these trust funds in good faith before anything happens to me. I accept this decision because I do not have any child who will inherit this money after I die. I want your urgent reply to me so that I will give you the deposit receipt which the COMPANY issued to me as next of kin for immediate transfer of the money to your account in your country, to start the good work of God, I want you to use the 15/percent of the total amount to help yourself in doing the project. I am desperately in keen need of assistance and I have summoned up courage to contact you for this task, you must not fail me and the millions of the poor people in our todays WORLD. This is no stolen money and there are no dangers involved,100% RISK FREE with full legal proof. Please if you would be able to use the funds for the Charity works kindly let me know immediately.I will appreciate your utmost confidentiality and trust in this matter to accomplish my heart desire, as I don't want anything that will jeopardize my last wish. I want you to take 15 percent of the total money for your personal use while 85% of the money will go to charity.I will appreciate your utmost confidentiality and trust in this matter to accomplish my heart desire, as I don't want anything that will jeopardize my last wish. kindly respond for further details. Thanks and God bless you, Mrs Elodie Antoine
Re: [PATCH v5 0/6] Amazon's Annapurna Labs Alpine v3 device-tree
Hello Hanna, Arnd, Quoting Hawa, Hanna (2020-07-23 12:20:02) > On 7/23/2020 1:10 PM, Arnd Bergmann wrote: > > > > I just came across this old series and noticed we had never merged it. > > > > I don't know if the patches all still apply. Could you check and perhaps > > resend to...@kernel.org if they are still good to go into the coming > > merge window? > Sure, will rebase the patches and send ASAP. Thanks Hanna! It seems Tsahee, the original Alpine maintainer, is not active anymore; I guess I'll need to do better and set up everything required to send real pull requests for next releases as activity on Alpine happens from time to time. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH net-next 2/8] net: phy: mscc: fix a possible double unlock
On vsc8584_ptp_init failure we jump to the 'err' label, which unlocks the MDIO bus lock. But vsc8584_ptp_init isn't called with the MDIO bus lock taken, which could result in a double unlock. Fix this. Fixes: ab2bf9339357 ("net: phy: mscc: 1588 block initialization") Reported-by: kernel test robot Reported-by: Dan Carpenter Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 2a7082983c09..cb4e15d6e2db 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1436,7 +1436,7 @@ static int vsc8584_config_init(struct phy_device *phydev) ret = vsc8584_ptp_init(phydev); if (ret) - goto err; + return ret; phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); -- 2.26.2
[PATCH net-next 5/8] net: phy: mscc: do not access the MDIO bus lock directly
This patch improves the MSCC driver by using the provided phy_lock_mdio_bus and phy_unlock_mdio_bus helpers instead of locking and unlocking the MDIO bus lock directly. The patch is only cosmetic but should improve maintenance and consistency. Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_main.c | 24 drivers/net/phy/mscc/mscc_ptp.c | 12 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index cb4e15d6e2db..03680933f530 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1288,7 +1288,7 @@ static void vsc8584_get_base_addr(struct phy_device *phydev) struct vsc8531_private *vsc8531 = phydev->priv; u16 val, addr; - mutex_lock(&phydev->mdio.bus->mdio_lock); + phy_lock_mdio_bus(phydev); __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_EXTENDED); addr = __phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_4); @@ -1297,7 +1297,7 @@ static void vsc8584_get_base_addr(struct phy_device *phydev) val = __phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL); __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); - mutex_unlock(&phydev->mdio.bus->mdio_lock); + phy_unlock_mdio_bus(phydev); /* In the package, there are two pairs of PHYs (PHY0 + PHY2 and * PHY1 + PHY3). The first PHY of each pair (PHY0 and PHY1) is @@ -1331,7 +1331,7 @@ static int vsc8584_config_init(struct phy_device *phydev) phydev->mdix_ctrl = ETH_TP_MDI_AUTO; - mutex_lock(&phydev->mdio.bus->mdio_lock); + phy_lock_mdio_bus(phydev); /* Some parts of the init sequence are identical for every PHY in the * package. Some parts are modifying the GPIO register bank which is a @@ -1428,7 +1428,7 @@ static int vsc8584_config_init(struct phy_device *phydev) if (ret) goto err; - mutex_unlock(&phydev->mdio.bus->mdio_lock); + phy_unlock_mdio_bus(phydev); ret = vsc8584_macsec_init(phydev); if (ret) @@ -1469,7 +1469,7 @@ static int vsc8584_config_init(struct phy_device *phydev) return 0; err: - mutex_unlock(&phydev->mdio.bus->mdio_lock); + phy_unlock_mdio_bus(phydev); return ret; } @@ -1755,7 +1755,7 @@ static int vsc8514_config_init(struct phy_device *phydev) phydev->mdix_ctrl = ETH_TP_MDI_AUTO; - mutex_lock(&phydev->mdio.bus->mdio_lock); + phy_lock_mdio_bus(phydev); /* Some parts of the init sequence are identical for every PHY in the * package. Some parts are modifying the GPIO register bank which is a @@ -1843,14 +1843,14 @@ static int vsc8514_config_init(struct phy_device *phydev) reg = vsc85xx_csr_ctrl_phy_read(phydev, PHY_MCB_TARGET, PHY_S6G_PLL_STATUS); if (reg == 0x) { - mutex_unlock(&phydev->mdio.bus->mdio_lock); + phy_unlock_mdio_bus(phydev); return -EIO; } } while (time_before(jiffies, deadline) && (reg & BIT(12))); if (reg & BIT(12)) { - mutex_unlock(&phydev->mdio.bus->mdio_lock); + phy_unlock_mdio_bus(phydev); return -ETIMEDOUT; } @@ -1870,18 +1870,18 @@ static int vsc8514_config_init(struct phy_device *phydev) reg = vsc85xx_csr_ctrl_phy_read(phydev, PHY_MCB_TARGET, PHY_S6G_IB_STATUS0); if (reg == 0x) { - mutex_unlock(&phydev->mdio.bus->mdio_lock); + phy_unlock_mdio_bus(phydev); return -EIO; } } while (time_before(jiffies, deadline) && !(reg & BIT(8))); if (!(reg & BIT(8))) { - mutex_unlock(&phydev->mdio.bus->mdio_lock); + phy_unlock_mdio_bus(phydev); return -ETIMEDOUT; } - mutex_unlock(&phydev->mdio.bus->mdio_lock); + phy_unlock_mdio_bus(phydev); ret = phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); @@ -1908,7 +1908,7 @@ static int vsc8514_config_init(struct phy_device *phydev) return ret; err: - mutex_unlock(&phydev->mdio.bus->mdio_lock); + phy_unlock_mdio_bus(phydev); return ret; } diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c index d4266911efc5..ef3441747348 100644 --- a/drivers/net/phy/mscc/mscc_ptp.c +++ b/drivers/net/phy/mscc/mscc_ptp.c @@ -80,7 +80,7 @@ static u32 vsc85xx_ts_read_csr(struct phy_device *phydev, enum ts_blk blk,
[PATCH net-next 0/8] net: phy: mscc: multiple improvements
Hello, This series contains various improvements to the MSCC PHY driver, fixing sparse and smatch warnings, using functions provided by the PHY core, and improving the driver consistency and maintenance. I don't think any of those improvements and fixes is worth backporting to stable trees. Thanks! Antoine Antoine Tenart (8): net: phy: mscc: macsec: fix sparse warnings net: phy: mscc: fix a possible double unlock net: phy: mscc: ptp: fix a smatch error net: phy: mscc: ptp: fix a typo in a comment net: phy: mscc: do not access the MDIO bus lock directly net: phy: mscc: restore the base page in vsc8514/8584_config_init net: phy: mscc: remove useless page configuration in the config init net: phy: mscc: improve vsc8514/8584_config_init consistency drivers/net/phy/mscc/mscc_macsec.c | 12 --- drivers/net/phy/mscc/mscc_main.c | 54 +- drivers/net/phy/mscc/mscc_ptp.c| 15 + 3 files changed, 45 insertions(+), 36 deletions(-) -- 2.26.2
[PATCH net-next 6/8] net: phy: mscc: restore the base page in vsc8514/8584_config_init
In the vsc8584_config_init and vsc8514_config_init, the base page is set to 'GPIO', configuration is done, and the page is never explicitly restored to the standard page. No bug was triggered as it turns out helpers called in those config_init functions do modify the base page, and set it back to standard. But that is dangerous and any modification to those functions would introduce bugs. This patch fixes this, to improve maintenance, by restoring the base page to 'standard' once 'GPIO' accesses are completed. Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_main.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 03680933f530..f625109df00a 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1395,6 +1395,11 @@ static int vsc8584_config_init(struct phy_device *phydev) if (ret) goto err; + ret = phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, +MSCC_PHY_PAGE_STANDARD); + if (ret) + goto err; + if (!phy_interface_is_rgmii(phydev)) { val = PROC_CMD_MCB_ACCESS_MAC_CONF | PROC_CMD_RST_CONF_PORT | PROC_CMD_READ_MOD_WRITE_PORT; @@ -1779,7 +1784,11 @@ static int vsc8514_config_init(struct phy_device *phydev) val &= ~MAC_CFG_MASK; val |= MAC_CFG_QSGMII; ret = phy_base_write(phydev, MSCC_PHY_MAC_CFG_FASTLINK, val); + if (ret) + goto err; + ret = phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, +MSCC_PHY_PAGE_STANDARD); if (ret) goto err; -- 2.26.2
[PATCH net-next 7/8] net: phy: mscc: remove useless page configuration in the config init
In the middle of vsc8584_config_init and vsc8514_config_init, the page is set to 'standard'. This is the default value, and the page isn't set to another value before. Those pages configuration can be safely removed. Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_main.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index f625109df00a..04e1ef791cec 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1443,8 +1443,6 @@ static int vsc8584_config_init(struct phy_device *phydev) if (ret) return ret; - phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); - val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1); val &= ~(MEDIA_OP_MODE_MASK | VSC8584_MAC_IF_SELECTION_MASK); val |= (MEDIA_OP_MODE_COPPER << MEDIA_OP_MODE_POS) | @@ -1892,11 +1890,6 @@ static int vsc8514_config_init(struct phy_device *phydev) phy_unlock_mdio_bus(phydev); - ret = phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); - - if (ret) - return ret; - ret = phy_modify(phydev, MSCC_PHY_EXT_PHY_CNTL_1, MEDIA_OP_MODE_MASK, MEDIA_OP_MODE_COPPER << MEDIA_OP_MODE_POS); -- 2.26.2
[PATCH net-next 1/8] net: phy: mscc: macsec: fix sparse warnings
This patch fixes the following sparse warnings when building MACsec support in the MSCC PHY driver. mscc_macsec.c:393:42: warning: cast from restricted sci_t mscc_macsec.c:395:42: warning: restricted sci_t degrades to integer mscc_macsec.c:402:42: warning: restricted __be16 degrades to integer mscc_macsec.c:608:34: warning: cast from restricted sci_t mscc_macsec.c:610:34: warning: restricted sci_t degrades to integer Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_macsec.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index 713c62b1d1f0..77c8c2fb28de 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -385,21 +385,23 @@ static void vsc8584_macsec_flow(struct phy_device *phydev, } if (bank == MACSEC_INGR && flow->match.sci && flow->rx_sa->sc->sci) { + u64 sci = (__force u64)flow->rx_sa->sc->sci; + match |= MSCC_MS_SAM_MISC_MATCH_TCI(BIT(3)); mask |= MSCC_MS_SAM_MASK_TCI_MASK(BIT(3)) | MSCC_MS_SAM_MASK_SCI_MASK; vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_SAM_MATCH_SCI_LO(idx), -lower_32_bits(flow->rx_sa->sc->sci)); +lower_32_bits(sci)); vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_SAM_MATCH_SCI_HI(idx), -upper_32_bits(flow->rx_sa->sc->sci)); +upper_32_bits(sci)); } if (flow->match.etype) { mask |= MSCC_MS_SAM_MASK_MAC_ETYPE_MASK; vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_SAM_MAC_SA_MATCH_HI(idx), - MSCC_MS_SAM_MAC_SA_MATCH_HI_ETYPE(htons(flow->etype))); + MSCC_MS_SAM_MAC_SA_MATCH_HI_ETYPE((__force u32)htons(flow->etype))); } match |= MSCC_MS_SAM_MISC_MATCH_PRIORITY(flow->priority); @@ -545,7 +547,7 @@ static int vsc8584_macsec_transformation(struct phy_device *phydev, int i, ret, index = flow->index; u32 rec = 0, control = 0; u8 hkey[16]; - sci_t sci; + u64 sci; ret = vsc8584_macsec_derive_key(flow->key, priv->secy->key_len, hkey); if (ret) @@ -603,7 +605,7 @@ static int vsc8584_macsec_transformation(struct phy_device *phydev, priv->secy->replay_window); /* Set the input vectors */ - sci = bank == MACSEC_INGR ? flow->rx_sa->sc->sci : priv->secy->sci; + sci = (__force u64)(bank == MACSEC_INGR ? flow->rx_sa->sc->sci : priv->secy->sci); vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_XFORM_REC(index, rec++), lower_32_bits(sci)); vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_XFORM_REC(index, rec++), -- 2.26.2
[PATCH net-next 8/8] net: phy: mscc: improve vsc8514/8584_config_init consistency
All PHY read and write return values are checked for errors in vsc8514_config_init and vsc8584_config_init, except for one. Fix this. Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_main.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 04e1ef791cec..a4fbf3a4fa97 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1375,8 +1375,10 @@ static int vsc8584_config_init(struct phy_device *phydev) goto err; } - phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, - MSCC_PHY_PAGE_EXTENDED_GPIO); + ret = phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, +MSCC_PHY_PAGE_EXTENDED_GPIO); + if (ret) + goto err; val = phy_base_read(phydev, MSCC_PHY_MAC_CFG_FASTLINK); val &= ~MAC_CFG_MASK; @@ -1774,8 +1776,10 @@ static int vsc8514_config_init(struct phy_device *phydev) if (phy_package_init_once(phydev)) vsc8514_config_pre_init(phydev); - phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, - MSCC_PHY_PAGE_EXTENDED_GPIO); + ret = phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, +MSCC_PHY_PAGE_EXTENDED_GPIO); + if (ret) + goto err; val = phy_base_read(phydev, MSCC_PHY_MAC_CFG_FASTLINK); -- 2.26.2
[PATCH net-next 4/8] net: phy: mscc: ptp: fix a typo in a comment
This patch fixes a typo in a comment, s/Ths/This/. The patch is cosmetic only. Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_ptp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c index 030a56c9a06d..d4266911efc5 100644 --- a/drivers/net/phy/mscc/mscc_ptp.c +++ b/drivers/net/phy/mscc/mscc_ptp.c @@ -1564,7 +1564,7 @@ int vsc8584_ptp_probe(struct phy_device *phydev) /* Retrieve the shared load/save GPIO. Request it as non exclusive as * the same GPIO can be requested by all the PHYs of the same package. -* Ths GPIO must be used with the gpio_lock taken (the lock is shared +* This GPIO must be used with the gpio_lock taken (the lock is shared * between all PHYs). */ vsc8531->load_save = devm_gpiod_get_optional(&phydev->mdio.dev, "load-save", -- 2.26.2
[PATCH net-next 3/8] net: phy: mscc: ptp: fix a smatch error
The following error was reported by smatch: vsc85xx_ts_read_csr() error: uninitialized symbol 'blk_hw'. In practice this is very unlikely, as all the block identifiers given to this functions are handled and described in an enum. The smatch error is fixed by doing what is already done in vsc85xx_ts_write_csr: using the "PROCESSOR" block by default. Reported-by: kernel test robot Reported-by: Dan Carpenter Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_ptp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c index 17256d85cedf..030a56c9a06d 100644 --- a/drivers/net/phy/mscc/mscc_ptp.c +++ b/drivers/net/phy/mscc/mscc_ptp.c @@ -75,6 +75,7 @@ static u32 vsc85xx_ts_read_csr(struct phy_device *phydev, enum ts_blk blk, blk_hw = base_port ? EGRESS_ENGINE_0 : EGRESS_ENGINE_1; break; case PROCESSOR: + default: blk_hw = base_port ? PROCESSOR_0 : PROCESSOR_1; break; } -- 2.26.2
Re: [PATCH net-next v4 6/8] net: phy: mscc: timestamping and PHC support
Hello Richard, Quoting Richard Cochran (2020-06-25 15:22:26) > On Tue, Jun 23, 2020 at 04:30:12PM +0200, Antoine Tenart wrote: > > @@ -978,9 +1483,32 @@ static int __vsc8584_init_ptp(struct phy_device > > *phydev) > > > > vsc85xx_ts_eth_cmp1_sig(phydev); > > > > + vsc8531->mii_ts.rxtstamp = vsc85xx_rxtstamp; > > + vsc8531->mii_ts.txtstamp = vsc85xx_txtstamp; > > + vsc8531->mii_ts.hwtstamp = vsc85xx_hwtstamp; > > + vsc8531->mii_ts.ts_info = vsc85xx_ts_info; > > + phydev->mii_ts = &vsc8531->mii_ts; > > + > > + memcpy(&vsc8531->ptp->caps, &vsc85xx_clk_caps, > > sizeof(vsc85xx_clk_caps)); > > + > > + vsc8531->ptp->ptp_clock = ptp_clock_register(&vsc8531->ptp->caps, > > + &phydev->mdio.dev); > > + if (IS_ERR(vsc8531->ptp->ptp_clock)) > > + return PTR_ERR(vsc8531->ptp->ptp_clock); > > The ptp_clock_register() method can also return NULL: > > * Returns a valid pointer on success or PTR_ERR on failure. If PHC > * support is missing at the configuration level, this function > * returns NULL, and drivers are expected to gracefully handle that > * case separately. If ptp_clock_register returns NULL because PHC support is missing, the clock won't be exported. Beside that, I don't think we can do much in the error path, so this should be safe. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH net-next v4 5/8] net: phy: mscc: 1588 block initialization
From: Quentin Schulz This patch adds the first parts of the 1588 support in the MSCC PHY, with registers definition and the 1588 block initialization. Those PHYs are distributed in hardware packages containing multiple times the PHY. The VSC8584 for example is composed of 4 PHYs. With hardware packages, parts of the logic is usually common and one of the PHY has to be used for some parts of the initialization. Following this logic, the 1588 blocks of those PHYs are shared between two PHYs and accessing the registers has to be done using the "base" PHY of the group. This is handled thanks to helpers in the PTP code (and locks). We also need the MDIO bus lock while performing a single read or write to the 1588 registers as the read/write are composed of multiple MDIO transactions (and we don't want other threads updating the page). Co-developed-by: Antoine Tenart Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/Makefile|4 + drivers/net/phy/mscc/mscc.h | 33 + drivers/net/phy/mscc/mscc_main.c | 30 +- drivers/net/phy/mscc/mscc_ptp.c | 1010 ++ drivers/net/phy/mscc/mscc_ptp.h | 477 ++ 5 files changed, 1552 insertions(+), 2 deletions(-) create mode 100644 drivers/net/phy/mscc/mscc_ptp.c create mode 100644 drivers/net/phy/mscc/mscc_ptp.h diff --git a/drivers/net/phy/mscc/Makefile b/drivers/net/phy/mscc/Makefile index 10af42cd9839..d8e22a4eeeff 100644 --- a/drivers/net/phy/mscc/Makefile +++ b/drivers/net/phy/mscc/Makefile @@ -8,3 +8,7 @@ mscc-objs := mscc_main.o ifdef CONFIG_MACSEC mscc-objs += mscc_macsec.o endif + +ifdef CONFIG_NETWORK_PHY_TIMESTAMPING +mscc-objs += mscc_ptp.o +endif diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index 756ec418f4f8..eabb6ab3c374 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -133,6 +133,7 @@ enum rgmii_clock_delay { * in the same package. */ #define MSCC_PHY_PAGE_EXTENDED_GPIO 0x0010 /* Extended reg - GPIO */ +#define MSCC_PHY_PAGE_1588 0x1588 /* PTP (1588) */ #define MSCC_PHY_PAGE_TEST 0x2a30 /* Test reg */ #define MSCC_PHY_PAGE_TR 0x52b5 /* Token ring registers */ @@ -373,6 +374,20 @@ struct vsc8531_private { unsigned long ingr_flows; unsigned long egr_flows; #endif + + bool input_clk_init; + struct vsc85xx_ptp *ptp; + + /* For multiple port PHYs; the MDIO address of the base PHY in the +* pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are coupled. +* PHY1 and PHY3 as well. PHY0 and PHY1 are base PHYs for their +* respective pair. +*/ + unsigned int ts_base_addr; + u8 ts_base_phy; + + /* ts_lock: used for per-PHY timestamping operations. */ + struct mutex ts_lock; }; #if IS_ENABLED(CONFIG_OF_MDIO) @@ -399,4 +414,22 @@ static inline void vsc8584_config_macsec_intr(struct phy_device *phydev) } #endif +#if IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) +void vsc85xx_link_change_notify(struct phy_device *phydev); +int vsc8584_ptp_init(struct phy_device *phydev); +int vsc8584_ptp_probe(struct phy_device *phydev); +#else +static inline void vsc85xx_link_change_notify(struct phy_device *phydev) +{ +} +static inline int vsc8584_ptp_init(struct phy_device *phydev) +{ + return 0; +} +static inline int vsc8584_ptp_probe(struct phy_device *phydev) +{ + return 0; +} +#endif + #endif /* _MSCC_PHY_H_ */ diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 052a0def6e83..ea004712540f 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1299,10 +1299,26 @@ static void vsc8584_get_base_addr(struct phy_device *phydev) __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); mutex_unlock(&phydev->mdio.bus->mdio_lock); - if (val & PHY_ADDR_REVERSED) + /* In the package, there are two pairs of PHYs (PHY0 + PHY2 and +* PHY1 + PHY3). The first PHY of each pair (PHY0 and PHY1) is +* the base PHY for timestamping operations. +*/ + vsc8531->ts_base_addr = phydev->mdio.addr; + vsc8531->ts_base_phy = addr; + + if (val & PHY_ADDR_REVERSED) { vsc8531->base_addr = phydev->mdio.addr + addr; - else + if (addr > 1) { + vsc8531->ts_base_addr += 2; + vsc8531->ts_base_phy += 2; + } + } else { vsc8531->base_addr = phydev->mdio.addr - addr; + if (addr > 1) { + vsc8531->ts_base_addr -= 2; + vsc8531->ts_base_phy -= 2; + } + } vsc8531->addr = addr; } @@ -1418,6 +1434,10 @@ static int vsc8584_config_init(struct phy_device *phydev) if (ret)
[PATCH net-next v4 7/8] dt-bindings: net: phy: vsc8531: document the load/save GPIO
A new optional property can be used to reference the load/save GPIO, used for PTP hardware clock (PHC) operations. This patch documents it in the binding documentation. Signed-off-by: Antoine Tenart --- Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt index 5ff37c68c941..87a27d775d48 100644 --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt @@ -31,6 +31,8 @@ Optional properties: VSC8531_LINK_100_ACTIVITY (2), VSC8531_LINK_ACTIVITY (0) and VSC8531_DUPLEX_COLLISION (8). +- load-save-gpios : GPIO used for the load/save operation of the PTP + hardware clock (PHC). Table: 1 - Edge rate change @@ -67,4 +69,5 @@ Example: vsc8531,edge-slowdown = <7>; vsc8531,led-0-mode = ; vsc8531,led-1-mode = ; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; -- 2.26.2
[PATCH net-next v4 1/8] net: phy: add support for a common probe between shared PHYs
Shared PHYs (PHYs in the same hardware package) may have shared registers and their drivers would usually need to share information. There is currently a way to have a shared (part of the) init, by using phy_package_init_once(). This patch extends the logic to share parts of the probe to allow sharing the initialization of locks or resources retrieval. Signed-off-by: Antoine Tenart Reviewed-by: Andrew Lunn --- include/linux/phy.h | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/include/linux/phy.h b/include/linux/phy.h index 9248dd2ce4ca..457489f1951c 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -244,7 +244,8 @@ struct phy_package_shared { }; /* used as bit number in atomic bitops */ -#define PHY_SHARED_F_INIT_DONE 0 +#define PHY_SHARED_F_INIT_DONE 0 +#define PHY_SHARED_F_PROBE_DONE 1 /* * The Bus class for PHYs. Devices which provide access to @@ -1558,14 +1559,25 @@ static inline int __phy_package_write(struct phy_device *phydev, return __mdiobus_write(phydev->mdio.bus, shared->addr, regnum, val); } -static inline bool phy_package_init_once(struct phy_device *phydev) +static inline bool __phy_package_set_once(struct phy_device *phydev, + unsigned int b) { struct phy_package_shared *shared = phydev->shared; if (!shared) return false; - return !test_and_set_bit(PHY_SHARED_F_INIT_DONE, &shared->flags); + return !test_and_set_bit(b, &shared->flags); +} + +static inline bool phy_package_init_once(struct phy_device *phydev) +{ + return __phy_package_set_once(phydev, PHY_SHARED_F_INIT_DONE); +} + +static inline bool phy_package_probe_once(struct phy_device *phydev) +{ + return __phy_package_set_once(phydev, PHY_SHARED_F_PROBE_DONE); } extern struct bus_type mdio_bus_type; -- 2.26.2
[PATCH net-next v4 3/8] net: phy: mscc: remove the TR CLK disable magic value
From: Quentin Schulz This patch adds a define for the 0x8000 magic value used to perform enable/disable actions on the "token ring clock". The patch is only cosmetic. Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart Reviewed-by: Andrew Lunn --- drivers/net/phy/mscc/mscc.h | 1 + drivers/net/phy/mscc/mscc_main.c | 10 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index fbcee5fce7b2..756ec418f4f8 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -252,6 +252,7 @@ enum rgmii_clock_delay { /* Test page Registers */ #define MSCC_PHY_TEST_PAGE_5 5 #define MSCC_PHY_TEST_PAGE_8 8 +#define TR_CLK_DISABLE 0x8000 #define MSCC_PHY_TEST_PAGE_9 9 #define MSCC_PHY_TEST_PAGE_2020 #define MSCC_PHY_TEST_PAGE_2424 diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 5ddc44f87eaf..052a0def6e83 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -629,7 +629,7 @@ static int vsc8531_pre_init_seq_set(struct phy_device *phydev) if (rc < 0) return rc; rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_TEST, - MSCC_PHY_TEST_PAGE_8, 0x8000, 0x8000); + MSCC_PHY_TEST_PAGE_8, TR_CLK_DISABLE, TR_CLK_DISABLE); if (rc < 0) return rc; @@ -1026,7 +1026,7 @@ static int vsc8574_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_PHY_TEST_PAGE_5, 0x1b20); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg |= 0x8000; + reg |= TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR); @@ -1046,7 +1046,7 @@ static int vsc8574_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg &= ~0x8000; + reg &= ~TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); @@ -1196,7 +1196,7 @@ static int vsc8584_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_PHY_TEST_PAGE_5, 0x1f20); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg |= 0x8000; + reg |= TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR); @@ -1225,7 +1225,7 @@ static int vsc8584_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg &= ~0x8000; + reg &= ~TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); -- 2.26.2
[PATCH net-next v4 2/8] net: phy: mscc: fix copyright and author information in MACsec
All headers in the MSCC PHY driver have been copied and pasted from the original mscc.c file. However the information is not necessarily correct, as in the MACsec support. Fix this. Signed-off-by: Antoine Tenart Reviewed-by: Andrew Lunn --- drivers/net/phy/mscc/mscc_fc_buffer.h | 2 +- drivers/net/phy/mscc/mscc_mac.h | 2 +- drivers/net/phy/mscc/mscc_macsec.c| 6 +++--- drivers/net/phy/mscc/mscc_macsec.h| 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/mscc/mscc_fc_buffer.h b/drivers/net/phy/mscc/mscc_fc_buffer.h index 3803e826c37d..399e803395a5 100644 --- a/drivers/net/phy/mscc/mscc_fc_buffer.h +++ b/drivers/net/phy/mscc/mscc_fc_buffer.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (C) 2019 Microsemi Corporation + * Copyright (C) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_FC_BUFFER_H_ diff --git a/drivers/net/phy/mscc/mscc_mac.h b/drivers/net/phy/mscc/mscc_mac.h index 59b6837c60b3..8dd38dc6edbf 100644 --- a/drivers/net/phy/mscc/mscc_mac.h +++ b/drivers/net/phy/mscc/mscc_mac.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (c) 2017 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_LINE_MAC_H_ diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index b4d3dc4068e2..c0eeb62cb940 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -1,10 +1,10 @@ // SPDX-License-Identifier: (GPL-2.0 OR MIT) /* - * Driver for Microsemi VSC85xx PHYs + * Driver for Microsemi VSC85xx PHYs - MACsec support * - * Author: Nagaraju Lakkaraju + * Author: Antoine Tenart * License: Dual MIT/GPL - * Copyright (c) 2016 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #include diff --git a/drivers/net/phy/mscc/mscc_macsec.h b/drivers/net/phy/mscc/mscc_macsec.h index d751f2946b79..9c6d25e36de2 100644 --- a/drivers/net/phy/mscc/mscc_macsec.h +++ b/drivers/net/phy/mscc/mscc_macsec.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (c) 2018 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_MACSEC_H_ -- 2.26.2
[PATCH net-next v4 4/8] net: phy: mscc: take into account the 1588 block in MACsec init
This patch takes in account the use of the 1588 block in the MACsec initialization, as a conditional configuration has to be done (when the 1588 block is used). Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_macsec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index c0eeb62cb940..713c62b1d1f0 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -285,7 +285,9 @@ static void vsc8584_macsec_mac_init(struct phy_device *phydev, MSCC_MAC_CFG_PKTINF_CFG_STRIP_PREAMBLE_ENA | MSCC_MAC_CFG_PKTINF_CFG_INSERT_PREAMBLE_ENA | (bank == HOST_MAC ? - MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : 0)); + MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : 0) | +(IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) ? + MSCC_MAC_CFG_PKTINF_CFG_MACSEC_BYPASS_NUM_PTP_STALL_CLKS(0x8) : 0)); val = vsc8584_macsec_phy_read(phydev, bank, MSCC_MAC_CFG_MODE_CFG); val &= ~MSCC_MAC_CFG_MODE_CFG_DISABLE_DIC; -- 2.26.2
[PATCH net-next v4 8/8] MIPS: dts: ocelot: describe the load/save GPIO
From: Quentin Schulz This patch adds a description of the load/save GPIN pin, used in the VSC8584 PHY for timestamping operations. The related pinctrl description is also added. Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts index 33991fd209f5..897de5025d7f 100644 --- a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts +++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts @@ -3,6 +3,7 @@ /dts-v1/; +#include #include #include #include "ocelot.dtsi" @@ -25,6 +26,11 @@ phy_int_pins: phy_int_pins { pins = "GPIO_4"; function = "gpio"; }; + + phy_load_save_pins: phy_load_save_pins { + pins = "GPIO_10"; + function = "ptp2"; + }; }; &mdio0 { @@ -34,27 +40,31 @@ &mdio0 { &mdio1 { status = "okay"; pinctrl-names = "default"; - pinctrl-0 = <&miim1>, <&phy_int_pins>; + pinctrl-0 = <&miim1>, <&phy_int_pins>, <&phy_load_save_pins>; phy7: ethernet-phy@0 { reg = <0>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy6: ethernet-phy@1 { reg = <1>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy5: ethernet-phy@2 { reg = <2>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy4: ethernet-phy@3 { reg = <3>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; }; -- 2.26.2
[PATCH net-next v4 6/8] net: phy: mscc: timestamping and PHC support
This patch adds support for PHC and timestamping operations for the MSCC PHY. PTP 1-step and 2-step modes are supported, over Ethernet and UDP. To get and set the PHC time, a GPIO has to be used and changes are only retrieved or committed when on a rising edge. The same GPIO is shared by all PHYs, so the granularity of the lock protecting it has to be different from the ones protecting the 1588 registers (the VSC8584 PHY has 2 1588 blocks, and a single load/save pin). Co-developed-by: Quentin Schulz Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc.h | 31 +- drivers/net/phy/mscc/mscc_main.c | 21 +- drivers/net/phy/mscc/mscc_ptp.c | 582 +++ 3 files changed, 630 insertions(+), 4 deletions(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index eabb6ab3c374..9481bce94c2e 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -375,8 +375,12 @@ struct vsc8531_private { unsigned long egr_flows; #endif + struct mii_timestamper mii_ts; + bool input_clk_init; struct vsc85xx_ptp *ptp; + /* LOAD/SAVE GPIO pin, used for retrieving or setting time to the PHC. */ + struct gpio_desc *load_save; /* For multiple port PHYs; the MDIO address of the base PHY in the * pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are coupled. @@ -386,8 +390,19 @@ struct vsc8531_private { unsigned int ts_base_addr; u8 ts_base_phy; - /* ts_lock: used for per-PHY timestamping operations. */ + /* ts_lock: used for per-PHY timestamping operations. +* phc_lock: used for per-PHY PHC opertations. +*/ struct mutex ts_lock; + struct mutex phc_lock; +}; + +/* Shared structure between the PHYs of the same package. + * gpio_lock: used for PHC operations. Common for all PHYs as the load/save GPIO + * is shared. + */ +struct vsc85xx_shared_private { + struct mutex gpio_lock; }; #if IS_ENABLED(CONFIG_OF_MDIO) @@ -416,20 +431,34 @@ static inline void vsc8584_config_macsec_intr(struct phy_device *phydev) #if IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) void vsc85xx_link_change_notify(struct phy_device *phydev); +void vsc8584_config_ts_intr(struct phy_device *phydev); int vsc8584_ptp_init(struct phy_device *phydev); +int vsc8584_ptp_probe_once(struct phy_device *phydev); int vsc8584_ptp_probe(struct phy_device *phydev); +irqreturn_t vsc8584_handle_ts_interrupt(struct phy_device *phydev); #else static inline void vsc85xx_link_change_notify(struct phy_device *phydev) { } +static inline void vsc8584_config_ts_intr(struct phy_device *phydev) +{ +} static inline int vsc8584_ptp_init(struct phy_device *phydev) { return 0; } +static inline int vsc8584_ptp_probe_once(struct phy_device *phydev) +{ + return 0; +} static inline int vsc8584_ptp_probe(struct phy_device *phydev) { return 0; } +static inline irqreturn_t vsc8584_handle_ts_interrupt(struct phy_device *phydev) +{ + return IRQ_NONE; +} #endif #endif /* _MSCC_PHY_H_ */ diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index ea004712540f..2a7082983c09 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1475,12 +1475,20 @@ static int vsc8584_config_init(struct phy_device *phydev) static irqreturn_t vsc8584_handle_interrupt(struct phy_device *phydev) { + irqreturn_t ret; int irq_status; irq_status = phy_read(phydev, MII_VSC85XX_INT_STATUS); - if (irq_status < 0 || !(irq_status & MII_VSC85XX_INT_MASK_MASK)) + if (irq_status < 0) return IRQ_NONE; + /* Timestamping IRQ does not set a bit in the global INT_STATUS, so +* irq_status would be 0. +*/ + ret = vsc8584_handle_ts_interrupt(phydev); + if (!(irq_status & MII_VSC85XX_INT_MASK_MASK)) + return ret; + if (irq_status & MII_VSC85XX_INT_MASK_EXT) vsc8584_handle_macsec_interrupt(phydev); @@ -1920,6 +1928,7 @@ static int vsc85xx_config_intr(struct phy_device *phydev) if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { vsc8584_config_macsec_intr(phydev); + vsc8584_config_ts_intr(phydev); rc = phy_write(phydev, MII_VSC85XX_INT_MASK, MII_VSC85XX_INT_MASK_MASK); @@ -2033,8 +2042,8 @@ static int vsc8584_probe(struct phy_device *phydev) phydev->priv = vsc8531; vsc8584_get_base_addr(phydev); - devm_phy_package_join(&phydev->mdio.dev, phydev, - vsc8531->base_addr, 0); + devm_phy_package_join(&phydev->mdio.dev, phydev, vsc8531->base_addr, + sizeof(struct vsc85xx_shared_private)); vsc8531->nleds = 4; vsc8531->supp_led_m
[PATCH net-next v4 0/8] net: phy: mscc: PHC and timestamping support
Hello, This series aims at adding support for PHC and timestamping operations in the MSCC PHY driver, for the VSC858x and VSC8575. Those PHYs are capable of timestamping in 1-step and 2-step for both L2 and L4 traffic. As of this series, only IPv4 support was implemented when using L4 mode. This is because of an hardware limitation which prevents us for supporting both IPv4 and IPv6 at the same time. Implementing support for IPv6 should be quite easy (I do have the modifications needed for the hardware configuration) but I did not see a way to retrieve this information in hwtstamp(). What would you suggest? Those PHYs are distributed in hardware packages containing multiple times the PHY. The VSC8584 for example is composed of 4 PHYs. With hardware packages, parts of the logic is usually common and one of the PHY has to be used for some parts of the initialization. Following this logic, the 1588 blocks of those PHYs are shared between two PHYs and accessing the registers has to be done using the "base" PHY of the group. This is handled thanks to helpers in the PTP code (and locks). We also need the MDIO bus lock while performing a single read or write to the 1588 registers as the read/write are composed of multiple MDIO transactions (and we don't want other threads updating the page). To get and set the PHC time, a GPIO has to be used and changes are only retrieved or committed when on a rising edge. The same GPIO is shared by all PHYs, so the granularity of the lock protecting it has to be different from the ones protecting the 1588 registers (the VSC8584 PHY has 2 1588 blocks, and a single load/save pin). Patch 1 extends the recently added helpers to share information between PHYs of the same hardware package; to allow having part of the probe to be shared (in addition to the already supported init part). This will be used when adding support for PHC/TS to initialize locks. Patches 2 and 3 are mostly cosmetic. Patch 4 takes into account the 1588 block in the MACsec initialization, to allow having both the MACsec and 1588 blocks initialized on a running system. Patches 5 and 6 add support for PHC and timestamping operations in the MSCC driver. An initialization of the 1588 block (plus all the registers definition; and helpers) is added first; and then comes a patch to implement the PHC and timestamping API. Patches 7 and 8 add the required hardware description for device trees, to be able to use the load/save GPIO pin on the PCB120 board. To use this on a PCB120 board, two other series are needed and have already been sent upstream (one is merged). There are no dependency between all those series. Thanks! Antoine Since v3: - Fixed a SKB leak. - Removed ts_lock from the init, as TS and PHC operations aren't registered at this time. - Refectored the ts_base_addr/phy intialization. - Cleaned up the ingr/egr latencies definitons. - Fixed a comment about locking and the shared GPIO. - A few cosmetic fixes. Since v2: - Removed explicit inlines from .c files. - Fixed three warnings. Since v1: - Removed checks in rxtstamp/txtstamp as skb cannot be NULL here. - Reworked get_ptp_header_rx/get_ptp_header. - Reworked the locking logic between the PHC and timestamping operations. - Fixed a compilation issue on x86 reported by Jakub. Antoine Tenart (5): net: phy: add support for a common probe between shared PHYs net: phy: mscc: fix copyright and author information in MACsec net: phy: mscc: take into account the 1588 block in MACsec init net: phy: mscc: timestamping and PHC support dt-bindings: net: phy: vsc8531: document the load/save GPIO Quentin Schulz (3): net: phy: mscc: remove the TR CLK disable magic value net: phy: mscc: 1588 block initialization MIPS: dts: ocelot: describe the load/save GPIO .../bindings/net/mscc-phy-vsc8531.txt |3 + arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 12 +- drivers/net/phy/mscc/Makefile |4 + drivers/net/phy/mscc/mscc.h | 63 + drivers/net/phy/mscc/mscc_fc_buffer.h |2 +- drivers/net/phy/mscc/mscc_mac.h |2 +- drivers/net/phy/mscc/mscc_macsec.c| 10 +- drivers/net/phy/mscc/mscc_macsec.h|2 +- drivers/net/phy/mscc/mscc_main.c | 61 +- drivers/net/phy/mscc/mscc_ptp.c | 1592 + drivers/net/phy/mscc/mscc_ptp.h | 477 + include/linux/phy.h | 18 +- 12 files changed, 2225 insertions(+), 21 deletions(-) create mode 100644 drivers/net/phy/mscc/mscc_ptp.c create mode 100644 drivers/net/phy/mscc/mscc_ptp.h -- 2.26.2
Re: [PATCH net-next v3 6/8] net: phy: mscc: timestamping and PHC support
Hi Quentin, Quoting Quentin Schulz (2020-06-21 19:35:20) > On 2020-06-19 14:22, Antoine Tenart wrote: > [...] > > @@ -999,9 +1553,35 @@ int vsc8584_ptp_probe(struct phy_device *phydev) > > if (!vsc8531->ptp) > > return -ENOMEM; > > > > + mutex_init(&vsc8531->phc_lock); > > mutex_init(&vsc8531->ts_lock); > > > > + /* Retrieve the shared load/save GPIO. Request it as non exclusive as > > + * the same GPIO can be requested by all the PHYs of the same > > package. > > + * Ths GPIO must be used with the phc_lock taken (the lock is shared > > Typo + wrong lock named in the comment, instead: > > * This GPIO must be used with the gpio_lock taken (the lock is shared > > Though technically both are taken when access to the GPIO is requested > AFAICT. That's right, thanks for pointing this out! I'll fix it for v4. > Also on another note, maybe we could actually make vsc8531->base_addr > be a part of vsc85xx_shared_private structure. > > We would still need to compute it to pass it to devm_phy_package_join > but it can easily be returned by vsc8584_get_base_addr instead of the > current void and it'd put all the things used for all PHYs in the > package at the same place. We actually do not use directly the base_addr anymore from within the driver, thanks to the shared package conversion. We're now using __phy_package_write/__phy_package_read which are using the base address. So the move could be to remove it from the vsc8531_private. If we were to do it, we would need to find a clean solution: it's still part of a structure as multiple values are computed in vsc8584_get_base_addr, and it's a lot easier and cleaner to have them all in the same struct. Also, that have nothing to do with the current series :) Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v3 5/8] net: phy: mscc: 1588 block initialization
Hi Quentin, Quoting Quentin Schulz (2020-06-21 18:57:14) > > Feels weird to review my own patches a year later having written them, > almost nostalgic :) :) > On 2020-06-19 14:22, Antoine Tenart wrote: > [...] > > @@ -373,6 +374,21 @@ struct vsc8531_private { > > unsigned long ingr_flows; > > unsigned long egr_flows; > > #endif > > + > > + bool input_clk_init; > > + struct vsc85xx_ptp *ptp; > > + > > + /* For multiple port PHYs; the MDIO address of the base PHY in the > > + * pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are > > coupled. > > + * PHY1 and PHY3 as well. PHY0 and PHY1 are base PHYs for their > > + * respective pair. > > + */ > > + unsigned int ts_base_addr; > > + u8 ts_base_phy; > > + > > I hate myself now for this bad naming. After reading the code, > ts_base_addr is the address > of the base PHY (of a pair) on the MDIO bus and ts_base_phy is the > "internal" (package) > address of the base PHy (of a pair). This is not very explicit. > > Would ts_base_phy renamed into a ts_base_pkg_addr work better? > > > + /* ts_lock: used for per-PHY timestamping operations. > > + */ > > I don't remember exactly the comment best practices in net anymore, but > one line > comment instead? If you look at next patches, you'll see the comment is then multi-lines, as other members are added to the structure. I'll fix it anyway. > [...] > > > #endif /* _MSCC_PHY_H_ */ > > diff --git a/drivers/net/phy/mscc/mscc_main.c > > b/drivers/net/phy/mscc/mscc_main.c > > index 052a0def6e83..87ddae514627 100644 > > --- a/drivers/net/phy/mscc/mscc_main.c > > +++ b/drivers/net/phy/mscc/mscc_main.c > > @@ -1299,11 +1299,29 @@ static void vsc8584_get_base_addr(struct > > phy_device *phydev) > > __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); > > mutex_unlock(&phydev->mdio.bus->mdio_lock); > > > > - if (val & PHY_ADDR_REVERSED) > > + /* In the package, there are two pairs of PHYs (PHY0 + PHY2 and > > + * PHY1 + PHY3). The first PHY of each pair (PHY0 and PHY1) is > > + * the base PHY for timestamping operations. > > + */ > > + if (val & PHY_ADDR_REVERSED) { > > vsc8531->base_addr = phydev->mdio.addr + addr; > > - else > > + vsc8531->ts_base_addr = phydev->mdio.addr; > > + vsc8531->ts_base_phy = addr; > > + if (addr > 1) { > > + vsc8531->ts_base_addr += 2; > > + vsc8531->ts_base_phy += 2; > > + } > > + } else { > > vsc8531->base_addr = phydev->mdio.addr - addr; > > > > + vsc8531->ts_base_addr = phydev->mdio.addr; > > + vsc8531->ts_base_phy = addr; > > The two lines above are identical in both conditions, what about moving > them just before the if (val & PHY_ADDR_REVERSED) line? That's right, I'll fix it for v4. > [...] > > > +static const u32 vsc85xx_egr_latency[] = { > > + /* Copper Egress */ > > + 1272, /* 1000Mbps */ > > + 12516, /* 100Mbps */ > > + 125444, /* 10Mbps */ > > + /* Fiber Egress */ > > + 1277, /* 1000Mbps */ > > + 12537, /* 100Mbps */ > > + /* Copper Egress when MACsec ON */ > > + 3496, /* 1000Mbps */ > > + 34760, /* 100Mbps */ > > + 347844, /* 10Mbps */ > > + /* Fiber Egress when MACsec ON */ > > + 3502, /* 1000Mbps */ > > + 34780, /* 100Mbps */ > > +}; > > + > > +static const u32 vsc85xx_ingr_latency[] = { > > + /* Copper Ingress */ > > + 208, /* 1000Mbps */ > > + 304, /* 100Mbps */ > > + 2023, /* 10Mbps */ > > + /* Fiber Ingress */ > > + 98, /* 1000Mbps */ > > + 197, /* 100Mbps */ > > + /* Copper Ingress when MACsec ON */ > > + 2408, /* 1000Mbps */ > > + 22300, /* 100Mbps */ > > + 222009, /* 10Mbps */ > > + /* Fiber Ingress when MACsec ON */ > > + 2299, /* 1000Mbps */ > > + 22192, /* 100Mbps */ > > +}; > > + > > Wouldn't it make more sense to separate the latencies into two different > arrays? One for non-MACsec and one with? No idx "hack" later in the > function that way. Removing the "idx += 5" means having an added logic on the struct to used to retrieve the delay (I'll use two extra variables).
Re: [PATCH net-next v3 4/8] net: phy: mscc: take into account the 1588 block in MACsec init
Hi Quentin, Quoting Quentin Schulz (2020-06-21 17:38:42) > On 2020-06-19 14:22, Antoine Tenart wrote: > > This patch takes in account the use of the 1588 block in the MACsec > > initialization, as a conditional configuration has to be done (when the > > 1588 block is used). > > > > Signed-off-by: Antoine Tenart > > --- > > drivers/net/phy/mscc/mscc_macsec.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/phy/mscc/mscc_macsec.c > > b/drivers/net/phy/mscc/mscc_macsec.c > > index c0eeb62cb940..713c62b1d1f0 100644 > > --- a/drivers/net/phy/mscc/mscc_macsec.c > > +++ b/drivers/net/phy/mscc/mscc_macsec.c > > @@ -285,7 +285,9 @@ static void vsc8584_macsec_mac_init(struct > > phy_device *phydev, > >MSCC_MAC_CFG_PKTINF_CFG_STRIP_PREAMBLE_ENA | > >MSCC_MAC_CFG_PKTINF_CFG_INSERT_PREAMBLE_ENA | > >(bank == HOST_MAC ? > > - MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : > > 0)); > > + MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : > > 0) | > > + (IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) > > ? > > + > > MSCC_MAC_CFG_PKTINF_CFG_MACSEC_BYPASS_NUM_PTP_STALL_CLKS(0x8) : > > 0)); > > Do we have more info on this 0x8? Where does it come from? What does it > mean? I unfortunately do not have more information about this. > Also this starts to get a little bit hard to read. Would it make sense > to have > two temp variables? e.g.: > > padding = bank == HOST_MAC ? > MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING > : 0; > ptp_stall_clks = IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) ? > MSCC_MAC_CFG_PKTINF_CFG_MACSEC_BYPASS_NUM_PTP_STALL_CLKS(0x8) > : 0; > > vsc8584_macsec_phy_write(phydev, bank, MSCC_MAC_CFG_PKTINF_CFG, > MSCC_MAC_CFG_PKTINF_CFG_STRIP_FCS_ENA | > MSCC_MAC_CFG_PKTINF_CFG_INSERT_FCS_ENA | > MSCC_MAC_CFG_PKTINF_CFG_LPI_RELAY_ENA | > MSCC_MAC_CFG_PKTINF_CFG_STRIP_PREAMBLE_ENA | > MSCC_MAC_CFG_PKTINF_CFG_INSERT_PREAMBLE_ENA | > padding | > ptp_stall_clks); I'm not convinced this would be better. I guess that is a question of personal preference; I don't really mind either solution. I'll keep it as-is for now, as it follows what was already done. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v3 6/8] net: phy: mscc: timestamping and PHC support
Hi Andrew, Quoting Andrew Lunn (2020-06-20 17:21:42) > > + /* Retrieve the shared load/save GPIO. Request it as non exclusive as > > + * the same GPIO can be requested by all the PHYs of the same package. > > + * Ths GPIO must be used with the phc_lock taken (the lock is shared > > + * between all PHYs). > > + */ > > + vsc8531->load_save = devm_gpiod_get_optional(&phydev->mdio.dev, > > "load-save", > > + > > GPIOD_FLAGS_BIT_NONEXCLUSIVE | > > + GPIOD_OUT_LOW); > > + if (IS_ERR(vsc8531->load_save)) { > > + phydev_err(phydev, "Can't get load-save GPIO (%ld)\n", > > +PTR_ERR(vsc8531->load_save)); > > + return PTR_ERR(vsc8531->load_save); > > + } > > + > > I can understand the GPIO being optional, it is only needed when PTP > is being used. But i don't see a test anywhere that when PTP is being > used the GPIO is provided. What actually happens if it is missing and > somebody tries to use the PTP? Not much would happen, the time set/get wouldn't be correct. > Maybe only register the PTP parts with the core if the GPIO has been > found in DT? It's not easy, as some versions of the PHY (or the way it's integrated, I'm not sure) do not need the GPIO. I'll double check, and if it is required for PHC operations in this series, I'll do that. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v3 6/8] net: phy: mscc: timestamping and PHC support
Hi Andrew, Quoting Andrew Lunn (2020-06-20 17:40:08) > On Fri, Jun 19, 2020 at 02:22:58PM +0200, Antoine Tenart wrote: > > To get and set the PHC time, a GPIO has to be used and changes are only > > retrieved or committed when on a rising edge. The same GPIO is shared by > > all PHYs, so the granularity of the lock protecting it has to be > > different from the ones protecting the 1588 registers (the VSC8584 PHY > > has 2 1588 blocks, and a single load/save pin). > > I guess you thought about this GPIO quite a bit. > > It appears you have the mutex in the shared structure, but each PHY > has its own gpio_desc, even though it should be for the same GPIO? Yes, that's right. I had an early solution were I was sharing the GPIO in the shared structure, allowing to have a single gpio_desc. (dt would still needs to have one GPIO per PHY). That turned out to be a lot more complex that the current solution, having to unregister and free the GPIO desc manually when the driver of the last PHY was unregistered. I had to add lots of logic (and not the nicely looking one) to the shared PHY helpers in the core. > The binding requires each PHY has the GPIO, even though it is the same > GPIO. And there does not appear to be any checking that each PHY > really does have the same GPIO. Right. I don't see a clean and nice way to do this. Do you have an idea? On another hand, this would only lead to not being able to set/get (a correct) time from the PHC. > Ideally there would be a section in DT for the package, and this GPIO > would be there. But i don't see an good way to do this. Yes, I agree. That wasn't the design chosen when PHY packages were added. This PHY already has a shared description, duplicated for each PHY of the same package (for the interrupt line). If we were to change this one day, we probably would have to break dt compatibility anyway as there's already a property that is shared. > This does not feel right to me, but i've no good idea how it can be > made better :-( I think this kind of rework is out of this series scope; but I would be happy to discuss a better solution for someday. Thanks! Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v3 6/8] net: phy: mscc: timestamping and PHC support
Hello Richard, Quoting Richard Cochran (2020-06-20 17:00:45) > On Fri, Jun 19, 2020 at 02:22:58PM +0200, Antoine Tenart wrote: > > > +static void vsc85xx_dequeue_skb(struct vsc85xx_ptp *ptp) > > +{ > > + struct skb_shared_hwtstamps shhwtstamps; > > + struct vsc85xx_ts_fifo fifo; > > + struct sk_buff *skb; > > + u8 skb_sig[16], *p; > > + int i, len; > > + u32 reg; > > + > > + memset(&fifo, 0, sizeof(fifo)); > > + p = (u8 *)&fifo; > > + > > + reg = vsc85xx_ts_read_csr(ptp->phydev, PROCESSOR, > > + MSCC_PHY_PTP_EGR_TS_FIFO(0)); > > + if (reg & PTP_EGR_TS_FIFO_EMPTY) > > + return; > > + > > + *p++ = reg & 0xff; > > + *p++ = (reg >> 8) & 0xff; > > + > > + /* Read the current FIFO item. Reading FIFO6 pops the next one. */ > > + for (i = 1; i < 7; i++) { > > + reg = vsc85xx_ts_read_csr(ptp->phydev, PROCESSOR, > > + MSCC_PHY_PTP_EGR_TS_FIFO(i)); > > + *p++ = reg & 0xff; > > + *p++ = (reg >> 8) & 0xff; > > + *p++ = (reg >> 16) & 0xff; > > + *p++ = (reg >> 24) & 0xff; > > + } > > + > > + len = skb_queue_len(&ptp->tx_queue); > > + if (len < 1) > > + return; > > + > > + while (len--) { > > + skb = __skb_dequeue(&ptp->tx_queue); > > + if (!skb) > > + return; > > + > > + /* Can't get the signature of the packet, won't ever > > + * be able to have one so let's dequeue the packet. > > + */ > > + if (get_sig(skb, skb_sig) < 0) > > + continue; > > This leaks the skb. That's right, thanks for pointing this out! I'll fix it. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v3 5/8] net: phy: mscc: 1588 block initialization
Hi Andrew, Quoting Andrew Lunn (2020-06-20 17:10:01) > On Fri, Jun 19, 2020 at 02:22:57PM +0200, Antoine Tenart wrote: > > From: Quentin Schulz > > > > This patch adds the first parts of the 1588 support in the MSCC PHY, > > with registers definition and the 1588 block initialization. > > > > Those PHYs are distributed in hardware packages containing multiple > > times the PHY. The VSC8584 for example is composed of 4 PHYs. With > > hardware packages, parts of the logic is usually common and one of the > > PHY has to be used for some parts of the initialization. Following this > > logic, the 1588 blocks of those PHYs are shared between two PHYs and > > accessing the registers has to be done using the "base" PHY of the > > group. This is handled thanks to helpers in the PTP code (and locks). > > We also need the MDIO bus lock while performing a single read or write > > to the 1588 registers as the read/write are composed of multiple MDIO > > transactions (and we don't want other threads updating the page). > > Locking sounds complex. I assume LOCKDEP was your friend in getting > this correct and deadlock free. I agree, locking is not straight forward. But it's actually not that complex: - The MDIO bus lock is used for all TS/PHC register access. - There is one lock for PHC operations and one for timestamping operations. The two are never used in the same function. We could use the same lock; introducing more waiting. - There is one shared lock for GPIO operations. It is only used in PHC functions, in two places. And I realized I can remove the locks from vsc8584_ptp_init, as PHC/TS helpers are not registered until the PHY is initialized. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v3 3/8] net: phy: mscc: remove the TR CLK disable magic value
Hello Andrew, Quoting Andrew Lunn (2020-06-20 16:52:52) > On Fri, Jun 19, 2020 at 02:22:55PM +0200, Antoine Tenart wrote: > > From: Quentin Schulz > > > > This patch adds a define for the 0x8000 magic value used to perform > > enable/disable actions on the "token ring clock". The patch is only > > cosmetic. > > I assume this is not 802.5 Token Ring? I have not a lot of details about this; but 802.5 Token Ring would be very surprising. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH net-next v3 4/8] net: phy: mscc: take into account the 1588 block in MACsec init
This patch takes in account the use of the 1588 block in the MACsec initialization, as a conditional configuration has to be done (when the 1588 block is used). Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_macsec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index c0eeb62cb940..713c62b1d1f0 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -285,7 +285,9 @@ static void vsc8584_macsec_mac_init(struct phy_device *phydev, MSCC_MAC_CFG_PKTINF_CFG_STRIP_PREAMBLE_ENA | MSCC_MAC_CFG_PKTINF_CFG_INSERT_PREAMBLE_ENA | (bank == HOST_MAC ? - MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : 0)); + MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : 0) | +(IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) ? + MSCC_MAC_CFG_PKTINF_CFG_MACSEC_BYPASS_NUM_PTP_STALL_CLKS(0x8) : 0)); val = vsc8584_macsec_phy_read(phydev, bank, MSCC_MAC_CFG_MODE_CFG); val &= ~MSCC_MAC_CFG_MODE_CFG_DISABLE_DIC; -- 2.26.2
[PATCH net-next v3 0/8] net: phy: mscc: PHC and timestamping support
Hello, This series aims at adding support for PHC and timestamping operations in the MSCC PHY driver, for the VSC858x and VSC8575. Those PHYs are capable of timestamping in 1-step and 2-step for both L2 and L4 traffic. As of this series, only IPv4 support was implemented when using L4 mode. This is because of an hardware limitation which prevents us for supporting both IPv4 and IPv6 at the same time. Implementing support for IPv6 should be quite easy (I do have the modifications needed for the hardware configuration) but I did not see a way to retrieve this information in hwtstamp(). What would you suggest? Those PHYs are distributed in hardware packages containing multiple times the PHY. The VSC8584 for example is composed of 4 PHYs. With hardware packages, parts of the logic is usually common and one of the PHY has to be used for some parts of the initialization. Following this logic, the 1588 blocks of those PHYs are shared between two PHYs and accessing the registers has to be done using the "base" PHY of the group. This is handled thanks to helpers in the PTP code (and locks). We also need the MDIO bus lock while performing a single read or write to the 1588 registers as the read/write are composed of multiple MDIO transactions (and we don't want other threads updating the page). To get and set the PHC time, a GPIO has to be used and changes are only retrieved or committed when on a rising edge. The same GPIO is shared by all PHYs, so the granularity of the lock protecting it has to be different from the ones protecting the 1588 registers (the VSC8584 PHY has 2 1588 blocks, and a single load/save pin). Patch 1 extends the recently added helpers to share information between PHYs of the same hardware package; to allow having part of the probe to be shared (in addition to the already supported init part). This will be used when adding support for PHC/TS to initialize locks. Patches 2 and 3 are mostly cosmetic. Patch 4 takes into account the 1588 block in the MACsec initialization, to allow having both the MACsec and 1588 blocks initialized on a running system. Patches 5 and 6 add support for PHC and timestamping operations in the MSCC driver. An initialization of the 1588 block (plus all the registers definition; and helpers) is added first; and then comes a patch to implement the PHC and timestamping API. Patches 7 and 8 add the required hardware description for device trees, to be able to use the load/save GPIO pin on the PCB120 board. To use this on a PCB120 board, two other series are needed and have already been sent upstream (one is merged). There are no dependency between all those series. Thanks! Antoine Since v2: - Removed explicit inlines from .c files. - Fixed three warnings. Since v1: - Removed checks in rxtstamp/txtstamp as skb cannot be NULL here. - Reworked get_ptp_header_rx/get_ptp_header. - Reworked the locking logic between the PHC and timestamping operations. - Fixed a compilation issue on x86 reported by Jakub. Antoine Tenart (5): net: phy: add support for a common probe between shared PHYs net: phy: mscc: fix copyright and author information in MACsec net: phy: mscc: take into account the 1588 block in MACsec init net: phy: mscc: timestamping and PHC support dt-bindings: net: phy: vsc8531: document the load/save GPIO Quentin Schulz (3): net: phy: mscc: remove the TR CLK disable magic value net: phy: mscc: 1588 block initialization MIPS: dts: ocelot: describe the load/save GPIO .../bindings/net/mscc-phy-vsc8531.txt |3 + arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 12 +- drivers/net/phy/mscc/Makefile |4 + drivers/net/phy/mscc/mscc.h | 64 + drivers/net/phy/mscc/mscc_fc_buffer.h |2 +- drivers/net/phy/mscc/mscc_mac.h |2 +- drivers/net/phy/mscc/mscc_macsec.c| 10 +- drivers/net/phy/mscc/mscc_macsec.h|2 +- drivers/net/phy/mscc/mscc_main.c | 63 +- drivers/net/phy/mscc/mscc_ptp.c | 1587 + drivers/net/phy/mscc/mscc_ptp.h | 477 + include/linux/phy.h | 18 +- 12 files changed, 2223 insertions(+), 21 deletions(-) create mode 100644 drivers/net/phy/mscc/mscc_ptp.c create mode 100644 drivers/net/phy/mscc/mscc_ptp.h -- 2.26.2
[PATCH net-next v3 3/8] net: phy: mscc: remove the TR CLK disable magic value
From: Quentin Schulz This patch adds a define for the 0x8000 magic value used to perform enable/disable actions on the "token ring clock". The patch is only cosmetic. Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc.h | 1 + drivers/net/phy/mscc/mscc_main.c | 10 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index fbcee5fce7b2..756ec418f4f8 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -252,6 +252,7 @@ enum rgmii_clock_delay { /* Test page Registers */ #define MSCC_PHY_TEST_PAGE_5 5 #define MSCC_PHY_TEST_PAGE_8 8 +#define TR_CLK_DISABLE 0x8000 #define MSCC_PHY_TEST_PAGE_9 9 #define MSCC_PHY_TEST_PAGE_2020 #define MSCC_PHY_TEST_PAGE_2424 diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 5ddc44f87eaf..052a0def6e83 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -629,7 +629,7 @@ static int vsc8531_pre_init_seq_set(struct phy_device *phydev) if (rc < 0) return rc; rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_TEST, - MSCC_PHY_TEST_PAGE_8, 0x8000, 0x8000); + MSCC_PHY_TEST_PAGE_8, TR_CLK_DISABLE, TR_CLK_DISABLE); if (rc < 0) return rc; @@ -1026,7 +1026,7 @@ static int vsc8574_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_PHY_TEST_PAGE_5, 0x1b20); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg |= 0x8000; + reg |= TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR); @@ -1046,7 +1046,7 @@ static int vsc8574_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg &= ~0x8000; + reg &= ~TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); @@ -1196,7 +1196,7 @@ static int vsc8584_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_PHY_TEST_PAGE_5, 0x1f20); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg |= 0x8000; + reg |= TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR); @@ -1225,7 +1225,7 @@ static int vsc8584_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg &= ~0x8000; + reg &= ~TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); -- 2.26.2
[PATCH net-next v3 7/8] dt-bindings: net: phy: vsc8531: document the load/save GPIO
A new optional property can be used to reference the load/save GPIO, used for PTP hardware clock (PHC) operations. This patch documents it in the binding documentation. Signed-off-by: Antoine Tenart --- Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt index 5ff37c68c941..87a27d775d48 100644 --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt @@ -31,6 +31,8 @@ Optional properties: VSC8531_LINK_100_ACTIVITY (2), VSC8531_LINK_ACTIVITY (0) and VSC8531_DUPLEX_COLLISION (8). +- load-save-gpios : GPIO used for the load/save operation of the PTP + hardware clock (PHC). Table: 1 - Edge rate change @@ -67,4 +69,5 @@ Example: vsc8531,edge-slowdown = <7>; vsc8531,led-0-mode = ; vsc8531,led-1-mode = ; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; -- 2.26.2
[PATCH net-next v3 8/8] MIPS: dts: ocelot: describe the load/save GPIO
From: Quentin Schulz This patch adds a description of the load/save GPIN pin, used in the VSC8584 PHY for timestamping operations. The related pinctrl description is also added. Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts index 33991fd209f5..897de5025d7f 100644 --- a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts +++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts @@ -3,6 +3,7 @@ /dts-v1/; +#include #include #include #include "ocelot.dtsi" @@ -25,6 +26,11 @@ phy_int_pins: phy_int_pins { pins = "GPIO_4"; function = "gpio"; }; + + phy_load_save_pins: phy_load_save_pins { + pins = "GPIO_10"; + function = "ptp2"; + }; }; &mdio0 { @@ -34,27 +40,31 @@ &mdio0 { &mdio1 { status = "okay"; pinctrl-names = "default"; - pinctrl-0 = <&miim1>, <&phy_int_pins>; + pinctrl-0 = <&miim1>, <&phy_int_pins>, <&phy_load_save_pins>; phy7: ethernet-phy@0 { reg = <0>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy6: ethernet-phy@1 { reg = <1>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy5: ethernet-phy@2 { reg = <2>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy4: ethernet-phy@3 { reg = <3>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; }; -- 2.26.2
[PATCH net-next v3 1/8] net: phy: add support for a common probe between shared PHYs
Shared PHYs (PHYs in the same hardware package) may have shared registers and their drivers would usually need to share information. There is currently a way to have a shared (part of the) init, by using phy_package_init_once(). This patch extends the logic to share parts of the probe to allow sharing the initialization of locks or resources retrieval. Signed-off-by: Antoine Tenart --- include/linux/phy.h | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/include/linux/phy.h b/include/linux/phy.h index 8c05d0fb5c00..058219b6441f 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -244,7 +244,8 @@ struct phy_package_shared { }; /* used as bit number in atomic bitops */ -#define PHY_SHARED_F_INIT_DONE 0 +#define PHY_SHARED_F_INIT_DONE 0 +#define PHY_SHARED_F_PROBE_DONE 1 /* * The Bus class for PHYs. Devices which provide access to @@ -1554,14 +1555,25 @@ static inline int __phy_package_write(struct phy_device *phydev, return __mdiobus_write(phydev->mdio.bus, shared->addr, regnum, val); } -static inline bool phy_package_init_once(struct phy_device *phydev) +static inline bool __phy_package_set_once(struct phy_device *phydev, + unsigned int b) { struct phy_package_shared *shared = phydev->shared; if (!shared) return false; - return !test_and_set_bit(PHY_SHARED_F_INIT_DONE, &shared->flags); + return !test_and_set_bit(b, &shared->flags); +} + +static inline bool phy_package_init_once(struct phy_device *phydev) +{ + return __phy_package_set_once(phydev, PHY_SHARED_F_INIT_DONE); +} + +static inline bool phy_package_probe_once(struct phy_device *phydev) +{ + return __phy_package_set_once(phydev, PHY_SHARED_F_PROBE_DONE); } extern struct bus_type mdio_bus_type; -- 2.26.2
[PATCH net-next v3 5/8] net: phy: mscc: 1588 block initialization
From: Quentin Schulz This patch adds the first parts of the 1588 support in the MSCC PHY, with registers definition and the 1588 block initialization. Those PHYs are distributed in hardware packages containing multiple times the PHY. The VSC8584 for example is composed of 4 PHYs. With hardware packages, parts of the logic is usually common and one of the PHY has to be used for some parts of the initialization. Following this logic, the 1588 blocks of those PHYs are shared between two PHYs and accessing the registers has to be done using the "base" PHY of the group. This is handled thanks to helpers in the PTP code (and locks). We also need the MDIO bus lock while performing a single read or write to the 1588 registers as the read/write are composed of multiple MDIO transactions (and we don't want other threads updating the page). Co-developed-by: Antoine Tenart Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/Makefile|4 + drivers/net/phy/mscc/mscc.h | 34 + drivers/net/phy/mscc/mscc_main.c | 32 +- drivers/net/phy/mscc/mscc_ptp.c | 1007 ++ drivers/net/phy/mscc/mscc_ptp.h | 477 ++ 5 files changed, 1552 insertions(+), 2 deletions(-) create mode 100644 drivers/net/phy/mscc/mscc_ptp.c create mode 100644 drivers/net/phy/mscc/mscc_ptp.h diff --git a/drivers/net/phy/mscc/Makefile b/drivers/net/phy/mscc/Makefile index 10af42cd9839..d8e22a4eeeff 100644 --- a/drivers/net/phy/mscc/Makefile +++ b/drivers/net/phy/mscc/Makefile @@ -8,3 +8,7 @@ mscc-objs := mscc_main.o ifdef CONFIG_MACSEC mscc-objs += mscc_macsec.o endif + +ifdef CONFIG_NETWORK_PHY_TIMESTAMPING +mscc-objs += mscc_ptp.o +endif diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index 756ec418f4f8..0881b22dbdac 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -133,6 +133,7 @@ enum rgmii_clock_delay { * in the same package. */ #define MSCC_PHY_PAGE_EXTENDED_GPIO 0x0010 /* Extended reg - GPIO */ +#define MSCC_PHY_PAGE_1588 0x1588 /* PTP (1588) */ #define MSCC_PHY_PAGE_TEST 0x2a30 /* Test reg */ #define MSCC_PHY_PAGE_TR 0x52b5 /* Token ring registers */ @@ -373,6 +374,21 @@ struct vsc8531_private { unsigned long ingr_flows; unsigned long egr_flows; #endif + + bool input_clk_init; + struct vsc85xx_ptp *ptp; + + /* For multiple port PHYs; the MDIO address of the base PHY in the +* pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are coupled. +* PHY1 and PHY3 as well. PHY0 and PHY1 are base PHYs for their +* respective pair. +*/ + unsigned int ts_base_addr; + u8 ts_base_phy; + + /* ts_lock: used for per-PHY timestamping operations. +*/ + struct mutex ts_lock; }; #if IS_ENABLED(CONFIG_OF_MDIO) @@ -399,4 +415,22 @@ static inline void vsc8584_config_macsec_intr(struct phy_device *phydev) } #endif +#if IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) +void vsc85xx_link_change_notify(struct phy_device *phydev); +int vsc8584_ptp_init(struct phy_device *phydev); +int vsc8584_ptp_probe(struct phy_device *phydev); +#else +static inline void vsc85xx_link_change_notify(struct phy_device *phydev) +{ +} +static inline int vsc8584_ptp_init(struct phy_device *phydev) +{ + return 0; +} +static inline int vsc8584_ptp_probe(struct phy_device *phydev) +{ + return 0; +} +#endif + #endif /* _MSCC_PHY_H_ */ diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 052a0def6e83..87ddae514627 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1299,11 +1299,29 @@ static void vsc8584_get_base_addr(struct phy_device *phydev) __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); mutex_unlock(&phydev->mdio.bus->mdio_lock); - if (val & PHY_ADDR_REVERSED) + /* In the package, there are two pairs of PHYs (PHY0 + PHY2 and +* PHY1 + PHY3). The first PHY of each pair (PHY0 and PHY1) is +* the base PHY for timestamping operations. +*/ + if (val & PHY_ADDR_REVERSED) { vsc8531->base_addr = phydev->mdio.addr + addr; - else + vsc8531->ts_base_addr = phydev->mdio.addr; + vsc8531->ts_base_phy = addr; + if (addr > 1) { + vsc8531->ts_base_addr += 2; + vsc8531->ts_base_phy += 2; + } + } else { vsc8531->base_addr = phydev->mdio.addr - addr; + vsc8531->ts_base_addr = phydev->mdio.addr; + vsc8531->ts_base_phy = addr; + if (addr > 1) { + vsc8531->ts_base_addr -= 2; + vsc8531->ts_base_phy -= 2; + }
[PATCH net-next v3 6/8] net: phy: mscc: timestamping and PHC support
This patch adds support for PHC and timestamping operations for the MSCC PHY. PTP 1-step and 2-step modes are supported, over Ethernet and UDP. To get and set the PHC time, a GPIO has to be used and changes are only retrieved or committed when on a rising edge. The same GPIO is shared by all PHYs, so the granularity of the lock protecting it has to be different from the ones protecting the 1588 registers (the VSC8584 PHY has 2 1588 blocks, and a single load/save pin). Co-developed-by: Quentin Schulz Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc.h | 29 ++ drivers/net/phy/mscc/mscc_main.c | 21 +- drivers/net/phy/mscc/mscc_ptp.c | 580 +++ 3 files changed, 627 insertions(+), 3 deletions(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index 0881b22dbdac..af3dc82f170a 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -375,8 +375,13 @@ struct vsc8531_private { unsigned long egr_flows; #endif + struct mii_timestamper mii_ts; + bool input_clk_init; struct vsc85xx_ptp *ptp; + /* LOAD/SAVE GPIO pin, used for retrieving or setting time to the PHC. +*/ + struct gpio_desc *load_save; /* For multiple port PHYs; the MDIO address of the base PHY in the * pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are coupled. @@ -387,8 +392,18 @@ struct vsc8531_private { u8 ts_base_phy; /* ts_lock: used for per-PHY timestamping operations. +* phc_lock: used for per-PHY PHC opertations. */ struct mutex ts_lock; + struct mutex phc_lock; +}; + +/* Shared structure between the PHYs of the same package. + * gpio_lock: used for PHC operations. Common for all PHYs as the load/save GPIO + * is shared. + */ +struct vsc85xx_shared_private { + struct mutex gpio_lock; }; #if IS_ENABLED(CONFIG_OF_MDIO) @@ -417,20 +432,34 @@ static inline void vsc8584_config_macsec_intr(struct phy_device *phydev) #if IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) void vsc85xx_link_change_notify(struct phy_device *phydev); +void vsc8584_config_ts_intr(struct phy_device *phydev); int vsc8584_ptp_init(struct phy_device *phydev); +int vsc8584_ptp_probe_once(struct phy_device *phydev); int vsc8584_ptp_probe(struct phy_device *phydev); +irqreturn_t vsc8584_handle_ts_interrupt(struct phy_device *phydev); #else static inline void vsc85xx_link_change_notify(struct phy_device *phydev) { } +static inline void vsc8584_config_ts_intr(struct phy_device *phydev) +{ +} static inline int vsc8584_ptp_init(struct phy_device *phydev) { return 0; } +static inline int vsc8584_ptp_probe_once(struct phy_device *phydev) +{ + return 0; +} static inline int vsc8584_ptp_probe(struct phy_device *phydev) { return 0; } +static inline irqreturn_t vsc8584_handle_ts_interrupt(struct phy_device *phydev) +{ + return IRQ_NONE; +} #endif #endif /* _MSCC_PHY_H_ */ diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 87ddae514627..5535901b9433 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1477,12 +1477,20 @@ static int vsc8584_config_init(struct phy_device *phydev) static irqreturn_t vsc8584_handle_interrupt(struct phy_device *phydev) { + irqreturn_t ret; int irq_status; irq_status = phy_read(phydev, MII_VSC85XX_INT_STATUS); - if (irq_status < 0 || !(irq_status & MII_VSC85XX_INT_MASK_MASK)) + if (irq_status < 0) return IRQ_NONE; + /* Timestamping IRQ does not set a bit in the global INT_STATUS, so +* irq_status would be 0. +*/ + ret = vsc8584_handle_ts_interrupt(phydev); + if (!(irq_status & MII_VSC85XX_INT_MASK_MASK)) + return ret; + if (irq_status & MII_VSC85XX_INT_MASK_EXT) vsc8584_handle_macsec_interrupt(phydev); @@ -1922,6 +1930,7 @@ static int vsc85xx_config_intr(struct phy_device *phydev) if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { vsc8584_config_macsec_intr(phydev); + vsc8584_config_ts_intr(phydev); rc = phy_write(phydev, MII_VSC85XX_INT_MASK, MII_VSC85XX_INT_MASK_MASK); @@ -2035,8 +2044,8 @@ static int vsc8584_probe(struct phy_device *phydev) phydev->priv = vsc8531; vsc8584_get_base_addr(phydev); - devm_phy_package_join(&phydev->mdio.dev, phydev, - vsc8531->base_addr, 0); + devm_phy_package_join(&phydev->mdio.dev, phydev, vsc8531->base_addr, + sizeof(struct vsc85xx_shared_private)); vsc8531->nleds = 4; vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES; @@ -2047,6 +2056,12 @@ static int vsc8584_probe(stru
[PATCH net-next v3 2/8] net: phy: mscc: fix copyright and author information in MACsec
All headers in the MSCC PHY driver have been copied and pasted from the original mscc.c file. However the information is not necessarily correct, as in the MACsec support. Fix this. Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_fc_buffer.h | 2 +- drivers/net/phy/mscc/mscc_mac.h | 2 +- drivers/net/phy/mscc/mscc_macsec.c| 6 +++--- drivers/net/phy/mscc/mscc_macsec.h| 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/mscc/mscc_fc_buffer.h b/drivers/net/phy/mscc/mscc_fc_buffer.h index 3803e826c37d..399e803395a5 100644 --- a/drivers/net/phy/mscc/mscc_fc_buffer.h +++ b/drivers/net/phy/mscc/mscc_fc_buffer.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (C) 2019 Microsemi Corporation + * Copyright (C) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_FC_BUFFER_H_ diff --git a/drivers/net/phy/mscc/mscc_mac.h b/drivers/net/phy/mscc/mscc_mac.h index 59b6837c60b3..8dd38dc6edbf 100644 --- a/drivers/net/phy/mscc/mscc_mac.h +++ b/drivers/net/phy/mscc/mscc_mac.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (c) 2017 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_LINE_MAC_H_ diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index b4d3dc4068e2..c0eeb62cb940 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -1,10 +1,10 @@ // SPDX-License-Identifier: (GPL-2.0 OR MIT) /* - * Driver for Microsemi VSC85xx PHYs + * Driver for Microsemi VSC85xx PHYs - MACsec support * - * Author: Nagaraju Lakkaraju + * Author: Antoine Tenart * License: Dual MIT/GPL - * Copyright (c) 2016 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #include diff --git a/drivers/net/phy/mscc/mscc_macsec.h b/drivers/net/phy/mscc/mscc_macsec.h index d751f2946b79..9c6d25e36de2 100644 --- a/drivers/net/phy/mscc/mscc_macsec.h +++ b/drivers/net/phy/mscc/mscc_macsec.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (c) 2018 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_MACSEC_H_ -- 2.26.2
Re: [PATCH net-next v2 6/8] net: phy: mscc: timestamping and PHC support
Hello Jakub, Quoting Jakub Kicinski (2020-06-17 18:32:17) > On Wed, 17 Jun 2020 15:31:25 +0200 Antoine Tenart wrote: > > This patch adds support for PHC and timestamping operations for the MSCC > > PHY. PTP 1-step and 2-step modes are supported, over Ethernet and UDP. > > > > To get and set the PHC time, a GPIO has to be used and changes are only > > retrieved or committed when on a rising edge. The same GPIO is shared by > > all PHYs, so the granularity of the lock protecting it has to be > > different from the ones protecting the 1588 registers (the VSC8584 PHY > > has 2 1588 blocks, and a single load/save pin). > > > > Co-developed-by: Quentin Schulz > > Signed-off-by: Quentin Schulz > > Signed-off-by: Antoine Tenart > > drivers/net/phy/mscc/mscc_ptp.c:406:24: warning: restricted __be16 degrades > to integer > drivers/net/phy/mscc/mscc_ptp.c:407:24: warning: restricted __be16 degrades > to integer > drivers/net/phy/mscc/mscc_ptp.c:1213:23: warning: symbol 'vsc85xx_clk_caps' > was not declared. Should it be static? > > Please make sure you don't add warnings when built with W=1 C=1 flags. I'll look into that. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v2 5/8] net: phy: mscc: 1588 block initialization
Hello Jakub, Quoting Jakub Kicinski (2020-06-17 18:32:58) > On Wed, 17 Jun 2020 15:31:24 +0200 Antoine Tenart wrote: > > +/* Two PHYs share the same 1588 processor and it's to be entirely > > configured > > + * through the base PHY of this processor. > > + */ > > +/* phydev->bus->mdio_lock should be locked when using this function */ > > +static inline int phy_ts_base_write(struct phy_device *phydev, u32 regnum, > > + u16 val) > > Please don't use static inline outside of headers in networking code. > The compiler will know best what to inline and when. I'll remove them. Thanks, Antoine > > +/* phydev->bus->mdio_lock should be locked when using this function */ > > +static inline int phy_ts_base_read(struct phy_device *phydev, u32 regnum) > > +{ > > + struct vsc8531_private *priv = phydev->priv; > > + > > + WARN_ON_ONCE(!mutex_is_locked(&phydev->mdio.bus->mdio_lock)); > > + return __mdiobus_read(phydev->mdio.bus, priv->ts_base_addr, regnum); > > +} -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH net-next v2 7/8] dt-bindings: net: phy: vsc8531: document the load/save GPIO
A new optional property can be used to reference the load/save GPIO, used for PTP hardware clock (PHC) operations. This patch documents it in the binding documentation. Signed-off-by: Antoine Tenart --- Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt index 5ff37c68c941..87a27d775d48 100644 --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt @@ -31,6 +31,8 @@ Optional properties: VSC8531_LINK_100_ACTIVITY (2), VSC8531_LINK_ACTIVITY (0) and VSC8531_DUPLEX_COLLISION (8). +- load-save-gpios : GPIO used for the load/save operation of the PTP + hardware clock (PHC). Table: 1 - Edge rate change @@ -67,4 +69,5 @@ Example: vsc8531,edge-slowdown = <7>; vsc8531,led-0-mode = ; vsc8531,led-1-mode = ; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; -- 2.26.2
[PATCH net-next v2 1/8] net: phy: add support for a common probe between shared PHYs
Shared PHYs (PHYs in the same hardware package) may have shared registers and their drivers would usually need to share information. There is currently a way to have a shared (part of the) init, by using phy_package_init_once(). This patch extends the logic to share parts of the probe to allow sharing the initialization of locks or resources retrieval. Signed-off-by: Antoine Tenart --- include/linux/phy.h | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/include/linux/phy.h b/include/linux/phy.h index 8c05d0fb5c00..058219b6441f 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -244,7 +244,8 @@ struct phy_package_shared { }; /* used as bit number in atomic bitops */ -#define PHY_SHARED_F_INIT_DONE 0 +#define PHY_SHARED_F_INIT_DONE 0 +#define PHY_SHARED_F_PROBE_DONE 1 /* * The Bus class for PHYs. Devices which provide access to @@ -1554,14 +1555,25 @@ static inline int __phy_package_write(struct phy_device *phydev, return __mdiobus_write(phydev->mdio.bus, shared->addr, regnum, val); } -static inline bool phy_package_init_once(struct phy_device *phydev) +static inline bool __phy_package_set_once(struct phy_device *phydev, + unsigned int b) { struct phy_package_shared *shared = phydev->shared; if (!shared) return false; - return !test_and_set_bit(PHY_SHARED_F_INIT_DONE, &shared->flags); + return !test_and_set_bit(b, &shared->flags); +} + +static inline bool phy_package_init_once(struct phy_device *phydev) +{ + return __phy_package_set_once(phydev, PHY_SHARED_F_INIT_DONE); +} + +static inline bool phy_package_probe_once(struct phy_device *phydev) +{ + return __phy_package_set_once(phydev, PHY_SHARED_F_PROBE_DONE); } extern struct bus_type mdio_bus_type; -- 2.26.2
[PATCH net-next v2 6/8] net: phy: mscc: timestamping and PHC support
This patch adds support for PHC and timestamping operations for the MSCC PHY. PTP 1-step and 2-step modes are supported, over Ethernet and UDP. To get and set the PHC time, a GPIO has to be used and changes are only retrieved or committed when on a rising edge. The same GPIO is shared by all PHYs, so the granularity of the lock protecting it has to be different from the ones protecting the 1588 registers (the VSC8584 PHY has 2 1588 blocks, and a single load/save pin). Co-developed-by: Quentin Schulz Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc.h | 29 ++ drivers/net/phy/mscc/mscc_main.c | 21 +- drivers/net/phy/mscc/mscc_ptp.c | 580 +++ 3 files changed, 627 insertions(+), 3 deletions(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index 0881b22dbdac..af3dc82f170a 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -375,8 +375,13 @@ struct vsc8531_private { unsigned long egr_flows; #endif + struct mii_timestamper mii_ts; + bool input_clk_init; struct vsc85xx_ptp *ptp; + /* LOAD/SAVE GPIO pin, used for retrieving or setting time to the PHC. +*/ + struct gpio_desc *load_save; /* For multiple port PHYs; the MDIO address of the base PHY in the * pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are coupled. @@ -387,8 +392,18 @@ struct vsc8531_private { u8 ts_base_phy; /* ts_lock: used for per-PHY timestamping operations. +* phc_lock: used for per-PHY PHC opertations. */ struct mutex ts_lock; + struct mutex phc_lock; +}; + +/* Shared structure between the PHYs of the same package. + * gpio_lock: used for PHC operations. Common for all PHYs as the load/save GPIO + * is shared. + */ +struct vsc85xx_shared_private { + struct mutex gpio_lock; }; #if IS_ENABLED(CONFIG_OF_MDIO) @@ -417,20 +432,34 @@ static inline void vsc8584_config_macsec_intr(struct phy_device *phydev) #if IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) void vsc85xx_link_change_notify(struct phy_device *phydev); +void vsc8584_config_ts_intr(struct phy_device *phydev); int vsc8584_ptp_init(struct phy_device *phydev); +int vsc8584_ptp_probe_once(struct phy_device *phydev); int vsc8584_ptp_probe(struct phy_device *phydev); +irqreturn_t vsc8584_handle_ts_interrupt(struct phy_device *phydev); #else static inline void vsc85xx_link_change_notify(struct phy_device *phydev) { } +static inline void vsc8584_config_ts_intr(struct phy_device *phydev) +{ +} static inline int vsc8584_ptp_init(struct phy_device *phydev) { return 0; } +static inline int vsc8584_ptp_probe_once(struct phy_device *phydev) +{ + return 0; +} static inline int vsc8584_ptp_probe(struct phy_device *phydev) { return 0; } +static inline irqreturn_t vsc8584_handle_ts_interrupt(struct phy_device *phydev) +{ + return IRQ_NONE; +} #endif #endif /* _MSCC_PHY_H_ */ diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 87ddae514627..5535901b9433 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1477,12 +1477,20 @@ static int vsc8584_config_init(struct phy_device *phydev) static irqreturn_t vsc8584_handle_interrupt(struct phy_device *phydev) { + irqreturn_t ret; int irq_status; irq_status = phy_read(phydev, MII_VSC85XX_INT_STATUS); - if (irq_status < 0 || !(irq_status & MII_VSC85XX_INT_MASK_MASK)) + if (irq_status < 0) return IRQ_NONE; + /* Timestamping IRQ does not set a bit in the global INT_STATUS, so +* irq_status would be 0. +*/ + ret = vsc8584_handle_ts_interrupt(phydev); + if (!(irq_status & MII_VSC85XX_INT_MASK_MASK)) + return ret; + if (irq_status & MII_VSC85XX_INT_MASK_EXT) vsc8584_handle_macsec_interrupt(phydev); @@ -1922,6 +1930,7 @@ static int vsc85xx_config_intr(struct phy_device *phydev) if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { vsc8584_config_macsec_intr(phydev); + vsc8584_config_ts_intr(phydev); rc = phy_write(phydev, MII_VSC85XX_INT_MASK, MII_VSC85XX_INT_MASK_MASK); @@ -2035,8 +2044,8 @@ static int vsc8584_probe(struct phy_device *phydev) phydev->priv = vsc8531; vsc8584_get_base_addr(phydev); - devm_phy_package_join(&phydev->mdio.dev, phydev, - vsc8531->base_addr, 0); + devm_phy_package_join(&phydev->mdio.dev, phydev, vsc8531->base_addr, + sizeof(struct vsc85xx_shared_private)); vsc8531->nleds = 4; vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES; @@ -2047,6 +2056,12 @@ static int vsc8584_probe(stru
[PATCH net-next v2 5/8] net: phy: mscc: 1588 block initialization
From: Quentin Schulz This patch adds the first parts of the 1588 support in the MSCC PHY, with registers definition and the 1588 block initialization. Those PHYs are distributed in hardware packages containing multiple times the PHY. The VSC8584 for example is composed of 4 PHYs. With hardware packages, parts of the logic is usually common and one of the PHY has to be used for some parts of the initialization. Following this logic, the 1588 blocks of those PHYs are shared between two PHYs and accessing the registers has to be done using the "base" PHY of the group. This is handled thanks to helpers in the PTP code (and locks). We also need the MDIO bus lock while performing a single read or write to the 1588 registers as the read/write are composed of multiple MDIO transactions (and we don't want other threads updating the page). Co-developed-by: Antoine Tenart Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/Makefile|4 + drivers/net/phy/mscc/mscc.h | 34 + drivers/net/phy/mscc/mscc_main.c | 32 +- drivers/net/phy/mscc/mscc_ptp.c | 1008 ++ drivers/net/phy/mscc/mscc_ptp.h | 477 ++ 5 files changed, 1553 insertions(+), 2 deletions(-) create mode 100644 drivers/net/phy/mscc/mscc_ptp.c create mode 100644 drivers/net/phy/mscc/mscc_ptp.h diff --git a/drivers/net/phy/mscc/Makefile b/drivers/net/phy/mscc/Makefile index 10af42cd9839..d8e22a4eeeff 100644 --- a/drivers/net/phy/mscc/Makefile +++ b/drivers/net/phy/mscc/Makefile @@ -8,3 +8,7 @@ mscc-objs := mscc_main.o ifdef CONFIG_MACSEC mscc-objs += mscc_macsec.o endif + +ifdef CONFIG_NETWORK_PHY_TIMESTAMPING +mscc-objs += mscc_ptp.o +endif diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index 756ec418f4f8..0881b22dbdac 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -133,6 +133,7 @@ enum rgmii_clock_delay { * in the same package. */ #define MSCC_PHY_PAGE_EXTENDED_GPIO 0x0010 /* Extended reg - GPIO */ +#define MSCC_PHY_PAGE_1588 0x1588 /* PTP (1588) */ #define MSCC_PHY_PAGE_TEST 0x2a30 /* Test reg */ #define MSCC_PHY_PAGE_TR 0x52b5 /* Token ring registers */ @@ -373,6 +374,21 @@ struct vsc8531_private { unsigned long ingr_flows; unsigned long egr_flows; #endif + + bool input_clk_init; + struct vsc85xx_ptp *ptp; + + /* For multiple port PHYs; the MDIO address of the base PHY in the +* pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are coupled. +* PHY1 and PHY3 as well. PHY0 and PHY1 are base PHYs for their +* respective pair. +*/ + unsigned int ts_base_addr; + u8 ts_base_phy; + + /* ts_lock: used for per-PHY timestamping operations. +*/ + struct mutex ts_lock; }; #if IS_ENABLED(CONFIG_OF_MDIO) @@ -399,4 +415,22 @@ static inline void vsc8584_config_macsec_intr(struct phy_device *phydev) } #endif +#if IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) +void vsc85xx_link_change_notify(struct phy_device *phydev); +int vsc8584_ptp_init(struct phy_device *phydev); +int vsc8584_ptp_probe(struct phy_device *phydev); +#else +static inline void vsc85xx_link_change_notify(struct phy_device *phydev) +{ +} +static inline int vsc8584_ptp_init(struct phy_device *phydev) +{ + return 0; +} +static inline int vsc8584_ptp_probe(struct phy_device *phydev) +{ + return 0; +} +#endif + #endif /* _MSCC_PHY_H_ */ diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 052a0def6e83..87ddae514627 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1299,11 +1299,29 @@ static void vsc8584_get_base_addr(struct phy_device *phydev) __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); mutex_unlock(&phydev->mdio.bus->mdio_lock); - if (val & PHY_ADDR_REVERSED) + /* In the package, there are two pairs of PHYs (PHY0 + PHY2 and +* PHY1 + PHY3). The first PHY of each pair (PHY0 and PHY1) is +* the base PHY for timestamping operations. +*/ + if (val & PHY_ADDR_REVERSED) { vsc8531->base_addr = phydev->mdio.addr + addr; - else + vsc8531->ts_base_addr = phydev->mdio.addr; + vsc8531->ts_base_phy = addr; + if (addr > 1) { + vsc8531->ts_base_addr += 2; + vsc8531->ts_base_phy += 2; + } + } else { vsc8531->base_addr = phydev->mdio.addr - addr; + vsc8531->ts_base_addr = phydev->mdio.addr; + vsc8531->ts_base_phy = addr; + if (addr > 1) { + vsc8531->ts_base_addr -= 2; + vsc8531->ts_base_phy -= 2; + }
[PATCH net-next v2 3/8] net: phy: mscc: remove the TR CLK disable magic value
From: Quentin Schulz This patch adds a define for the 0x8000 magic value used to perform enable/disable actions on the "token ring clock". The patch is only cosmetic. Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc.h | 1 + drivers/net/phy/mscc/mscc_main.c | 10 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index fbcee5fce7b2..756ec418f4f8 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -252,6 +252,7 @@ enum rgmii_clock_delay { /* Test page Registers */ #define MSCC_PHY_TEST_PAGE_5 5 #define MSCC_PHY_TEST_PAGE_8 8 +#define TR_CLK_DISABLE 0x8000 #define MSCC_PHY_TEST_PAGE_9 9 #define MSCC_PHY_TEST_PAGE_2020 #define MSCC_PHY_TEST_PAGE_2424 diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 5ddc44f87eaf..052a0def6e83 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -629,7 +629,7 @@ static int vsc8531_pre_init_seq_set(struct phy_device *phydev) if (rc < 0) return rc; rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_TEST, - MSCC_PHY_TEST_PAGE_8, 0x8000, 0x8000); + MSCC_PHY_TEST_PAGE_8, TR_CLK_DISABLE, TR_CLK_DISABLE); if (rc < 0) return rc; @@ -1026,7 +1026,7 @@ static int vsc8574_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_PHY_TEST_PAGE_5, 0x1b20); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg |= 0x8000; + reg |= TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR); @@ -1046,7 +1046,7 @@ static int vsc8574_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg &= ~0x8000; + reg &= ~TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); @@ -1196,7 +1196,7 @@ static int vsc8584_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_PHY_TEST_PAGE_5, 0x1f20); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg |= 0x8000; + reg |= TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR); @@ -1225,7 +1225,7 @@ static int vsc8584_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg &= ~0x8000; + reg &= ~TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); -- 2.26.2
[PATCH net-next v2 8/8] MIPS: dts: ocelot: describe the load/save GPIO
From: Quentin Schulz This patch adds a description of the load/save GPIN pin, used in the VSC8584 PHY for timestamping operations. The related pinctrl description is also added. Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts index 33991fd209f5..897de5025d7f 100644 --- a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts +++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts @@ -3,6 +3,7 @@ /dts-v1/; +#include #include #include #include "ocelot.dtsi" @@ -25,6 +26,11 @@ phy_int_pins: phy_int_pins { pins = "GPIO_4"; function = "gpio"; }; + + phy_load_save_pins: phy_load_save_pins { + pins = "GPIO_10"; + function = "ptp2"; + }; }; &mdio0 { @@ -34,27 +40,31 @@ &mdio0 { &mdio1 { status = "okay"; pinctrl-names = "default"; - pinctrl-0 = <&miim1>, <&phy_int_pins>; + pinctrl-0 = <&miim1>, <&phy_int_pins>, <&phy_load_save_pins>; phy7: ethernet-phy@0 { reg = <0>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy6: ethernet-phy@1 { reg = <1>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy5: ethernet-phy@2 { reg = <2>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy4: ethernet-phy@3 { reg = <3>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; }; -- 2.26.2
[PATCH net-next v2 2/8] net: phy: mscc: fix copyright and author information in MACsec
All headers in the MSCC PHY driver have been copied and pasted from the original mscc.c file. However the information is not necessarily correct, as in the MACsec support. Fix this. Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_fc_buffer.h | 2 +- drivers/net/phy/mscc/mscc_mac.h | 2 +- drivers/net/phy/mscc/mscc_macsec.c| 6 +++--- drivers/net/phy/mscc/mscc_macsec.h| 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/mscc/mscc_fc_buffer.h b/drivers/net/phy/mscc/mscc_fc_buffer.h index 3803e826c37d..399e803395a5 100644 --- a/drivers/net/phy/mscc/mscc_fc_buffer.h +++ b/drivers/net/phy/mscc/mscc_fc_buffer.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (C) 2019 Microsemi Corporation + * Copyright (C) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_FC_BUFFER_H_ diff --git a/drivers/net/phy/mscc/mscc_mac.h b/drivers/net/phy/mscc/mscc_mac.h index 59b6837c60b3..8dd38dc6edbf 100644 --- a/drivers/net/phy/mscc/mscc_mac.h +++ b/drivers/net/phy/mscc/mscc_mac.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (c) 2017 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_LINE_MAC_H_ diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index b4d3dc4068e2..c0eeb62cb940 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -1,10 +1,10 @@ // SPDX-License-Identifier: (GPL-2.0 OR MIT) /* - * Driver for Microsemi VSC85xx PHYs + * Driver for Microsemi VSC85xx PHYs - MACsec support * - * Author: Nagaraju Lakkaraju + * Author: Antoine Tenart * License: Dual MIT/GPL - * Copyright (c) 2016 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #include diff --git a/drivers/net/phy/mscc/mscc_macsec.h b/drivers/net/phy/mscc/mscc_macsec.h index d751f2946b79..9c6d25e36de2 100644 --- a/drivers/net/phy/mscc/mscc_macsec.h +++ b/drivers/net/phy/mscc/mscc_macsec.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (c) 2018 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_MACSEC_H_ -- 2.26.2
[PATCH net-next v2 4/8] net: phy: mscc: take into account the 1588 block in MACsec init
This patch takes in account the use of the 1588 block in the MACsec initialization, as a conditional configuration has to be done (when the 1588 block is used). Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_macsec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index c0eeb62cb940..713c62b1d1f0 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -285,7 +285,9 @@ static void vsc8584_macsec_mac_init(struct phy_device *phydev, MSCC_MAC_CFG_PKTINF_CFG_STRIP_PREAMBLE_ENA | MSCC_MAC_CFG_PKTINF_CFG_INSERT_PREAMBLE_ENA | (bank == HOST_MAC ? - MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : 0)); + MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : 0) | +(IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) ? + MSCC_MAC_CFG_PKTINF_CFG_MACSEC_BYPASS_NUM_PTP_STALL_CLKS(0x8) : 0)); val = vsc8584_macsec_phy_read(phydev, bank, MSCC_MAC_CFG_MODE_CFG); val &= ~MSCC_MAC_CFG_MODE_CFG_DISABLE_DIC; -- 2.26.2
[PATCH net-next v2 0/8] net: phy: mscc: PHC and timestamping support
Hello, This series aims at adding support for PHC and timestamping operations in the MSCC PHY driver, for the VSC858x and VSC8575. Those PHYs are capable of timestamping in 1-step and 2-step for both L2 and L4 traffic. As of this series, only IPv4 support was implemented when using L4 mode. This is because of an hardware limitation which prevents us for supporting both IPv4 and IPv6 at the same time. Implementing support for IPv6 should be quite easy (I do have the modifications needed for the hardware configuration) but I did not see a way to retrieve this information in hwtstamp(). What would you suggest? Those PHYs are distributed in hardware packages containing multiple times the PHY. The VSC8584 for example is composed of 4 PHYs. With hardware packages, parts of the logic is usually common and one of the PHY has to be used for some parts of the initialization. Following this logic, the 1588 blocks of those PHYs are shared between two PHYs and accessing the registers has to be done using the "base" PHY of the group. This is handled thanks to helpers in the PTP code (and locks). We also need the MDIO bus lock while performing a single read or write to the 1588 registers as the read/write are composed of multiple MDIO transactions (and we don't want other threads updating the page). To get and set the PHC time, a GPIO has to be used and changes are only retrieved or committed when on a rising edge. The same GPIO is shared by all PHYs, so the granularity of the lock protecting it has to be different from the ones protecting the 1588 registers (the VSC8584 PHY has 2 1588 blocks, and a single load/save pin). Patch 1 extends the recently added helpers to share information between PHYs of the same hardware package; to allow having part of the probe to be shared (in addition to the already supported init part). This will be used when adding support for PHC/TS to initialize locks. Patches 2 and 3 are mostly cosmetic. Patch 4 takes into account the 1588 block in the MACsec initialization, to allow having both the MACsec and 1588 blocks initialized on a running system. Patches 5 and 6 add support for PHC and timestamping operations in the MSCC driver. An initialization of the 1588 block (plus all the registers definition; and helpers) is added first; and then comes a patch to implement the PHC and timestamping API. Patches 7 and 8 add the required hardware description for device trees, to be able to use the load/save GPIO pin on the PCB120 board. To use this on a PCB120 board, two other series are needed and have already been sent upstream (one is merged). There are no dependency between all those series. Thanks! Antoine Since v1: - Removed checks in rxtstamp/txtstamp as skb cannot be NULL here. - Reworked get_ptp_header_rx/get_ptp_header. - Reworked the locking logic between the PHC and timestamping operations. - Fixed a compilation issue on x86 reported by Jakub. Antoine Tenart (5): net: phy: add support for a common probe between shared PHYs net: phy: mscc: fix copyright and author information in MACsec net: phy: mscc: take into account the 1588 block in MACsec init net: phy: mscc: timestamping and PHC support dt-bindings: net: phy: vsc8531: document the load/save GPIO Quentin Schulz (3): net: phy: mscc: remove the TR CLK disable magic value net: phy: mscc: 1588 block initialization MIPS: dts: ocelot: describe the load/save GPIO .../bindings/net/mscc-phy-vsc8531.txt |3 + arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 12 +- drivers/net/phy/mscc/Makefile |4 + drivers/net/phy/mscc/mscc.h | 64 + drivers/net/phy/mscc/mscc_fc_buffer.h |2 +- drivers/net/phy/mscc/mscc_mac.h |2 +- drivers/net/phy/mscc/mscc_macsec.c| 10 +- drivers/net/phy/mscc/mscc_macsec.h|2 +- drivers/net/phy/mscc/mscc_main.c | 63 +- drivers/net/phy/mscc/mscc_ptp.c | 1588 + drivers/net/phy/mscc/mscc_ptp.h | 477 + include/linux/phy.h | 18 +- 12 files changed, 2224 insertions(+), 21 deletions(-) create mode 100644 drivers/net/phy/mscc/mscc_ptp.c create mode 100644 drivers/net/phy/mscc/mscc_ptp.h -- 2.26.2
[PATCH net] net: phy: mscc: fix Serdes configuration in vsc8584_config_init
When converting the MSCC PHY driver to shared PHY packages, the Serdes configuration in vsc8584_config_init was modified to use 'base_addr' instead of 'base' as the port number. But 'base_addr' isn't equal to 'addr' for all PHYs inside the package, which leads to the Serdes still being enabled on those ports. This patch fixes it. Fixes: deb04e9c0ff2 ("net: phy: mscc: use phy_package_shared") Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 7ed0285206d0..24687ac5ee14 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1396,7 +1396,7 @@ static int vsc8584_config_init(struct phy_device *phydev) /* Disable SerDes for 100Base-FX */ ret = vsc8584_cmd(phydev, PROC_CMD_FIBER_MEDIA_CONF | - PROC_CMD_FIBER_PORT(vsc8531->base_addr) | + PROC_CMD_FIBER_PORT(vsc8531->addr) | PROC_CMD_FIBER_DISABLE | PROC_CMD_READ_MOD_WRITE_PORT | PROC_CMD_RST_CONF_PORT | PROC_CMD_FIBER_100BASE_FX); @@ -1405,7 +1405,7 @@ static int vsc8584_config_init(struct phy_device *phydev) /* Disable SerDes for 1000Base-X */ ret = vsc8584_cmd(phydev, PROC_CMD_FIBER_MEDIA_CONF | - PROC_CMD_FIBER_PORT(vsc8531->base_addr) | + PROC_CMD_FIBER_PORT(vsc8531->addr) | PROC_CMD_FIBER_DISABLE | PROC_CMD_READ_MOD_WRITE_PORT | PROC_CMD_RST_CONF_PORT | PROC_CMD_FIBER_1000BASE_X); -- 2.26.2
Re: [PATCH 05/15] crypto: inside-secure - use PCI_IRQ_MSI_TYPES where appropriate
Hello Piotr, Quoting Piotr Stankiewicz (2020-06-02 11:20:19) > Seeing as there is shorthand available to use when asking for any type > of interrupt, or any type of message signalled interrupt, leverage it. > > Signed-off-by: Piotr Stankiewicz > Reviewed-by: Andy Shevchenko Reviewed-by: Antoine Tenart Thanks, Antoine > --- > drivers/crypto/inside-secure/safexcel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/inside-secure/safexcel.c > b/drivers/crypto/inside-secure/safexcel.c > index 2cb53fbae841..1b2faa2a6ab0 100644 > --- a/drivers/crypto/inside-secure/safexcel.c > +++ b/drivers/crypto/inside-secure/safexcel.c > @@ -1567,7 +1567,7 @@ static int safexcel_probe_generic(void *pdev, > ret = pci_alloc_irq_vectors(pci_pdev, > priv->config.rings + 1, > priv->config.rings + 1, > - PCI_IRQ_MSI | PCI_IRQ_MSIX); > + PCI_IRQ_MSI_TYPES); > if (ret < 0) { > dev_err(dev, "Failed to allocate PCI MSI > interrupts\n"); > return ret; > -- > 2.17.2 > -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH net-next] net: phy: mscc: fix PHYs using the vsc8574_probe
PHYs using the vsc8574_probe fail to be initialized and their config_init return -EIO leading to errors like: "could not attach PHY: -5". This is because when the conversion of the MSCC PHY driver to use the shared PHY package helpers was done, the base address retrieval and the base PHY read and write helpers in the driver were modified. In particular, the base address retrieval logic was moved from the config_init to the probe. But the vsc8574_probe was forgotten. This patch fixes it. Fixes: deb04e9c0ff2 ("net: phy: mscc: use phy_package_shared") Signed-off-by: Antoine Tenart --- Hello, While the patch has a Fixes: tag, it's sent to net-next as commit deb04e9c0ff2 only is in net-next. This patch do not have to be backported to any prior version. Thanks! Antoine drivers/net/phy/mscc/mscc_main.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 550acf547ced..7ed0285206d0 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1977,6 +1977,10 @@ static int vsc8574_probe(struct phy_device *phydev) phydev->priv = vsc8531; + vsc8584_get_base_addr(phydev); + devm_phy_package_join(&phydev->mdio.dev, phydev, + vsc8531->base_addr, 0); + vsc8531->nleds = 4; vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES; vsc8531->hw_stats = vsc8584_hw_stats; -- 2.26.2
Re: [PATCH net-next 6/8] net: phy: mscc: timestamping and PHC support
Hello Richard, Quoting Richard Cochran (2020-05-28 16:34:40) > On Wed, May 27, 2020 at 06:41:56PM +0200, Antoine Tenart wrote: > > > +static struct vsc85xx_ptphdr *get_ptp_header(struct sk_buff *skb) > > +{ > > + struct ethhdr *ethhdr = eth_hdr(skb); > > + struct iphdr *iphdr = ip_hdr(skb); > > + struct udphdr *udphdr; > > + __u8 proto; > > + > > + if (ethhdr->h_proto == htons(ETH_P_1588)) > > + return (struct vsc85xx_ptphdr *)(((unsigned char *)ethhdr) + > > + skb_mac_header_len(skb)); > > + > > + if (ethhdr->h_proto != htons(ETH_P_IP)) > > + return NULL; > > + > > + proto = iphdr->protocol; > > + if (proto != IPPROTO_UDP) > > + return NULL; > > + > > + udphdr = udp_hdr(skb); > > + > > + if (udphdr->source != ntohs(PTP_EV_PORT) || > > + udphdr->dest != ntohs(PTP_EV_PORT)) > > + return NULL; > > + > > + return (struct vsc85xx_ptphdr *)(((unsigned char *)udphdr) + > > UDP_HLEN); > > +} > > This looks a lot like get_ptp_header_rx() below. Are you sure you > need two almost identical methods? That's right, good catch. I'll look into merging the two. > > +static void vsc85xx_get_tx_ts(struct vsc85xx_ptp *ptp) > > +{ > > + struct skb_shared_hwtstamps shhwtstamps; > > + struct sk_buff *skb, *first_skb = NULL; > > + struct vsc85xx_ts_fifo fifo; > > + u8 i, skb_sig[16], *p; > > + unsigned long ns; > > + s64 secs; > > + u32 reg; > > + > > +next_in_fifo: > > + memset(&fifo, 0, sizeof(fifo)); > > + p = (u8 *)&fifo; > > + > > + reg = vsc85xx_ts_read_csr(ptp->phydev, PROCESSOR, > > + MSCC_PHY_PTP_EGR_TS_FIFO(0)); > > + if (reg & PTP_EGR_TS_FIFO_EMPTY) > > + goto out; > > + > > + *p++ = reg & 0xff; > > + *p++ = (reg >> 8) & 0xff; > > + > > + /* Reading FIFO6 pops the FIFO item */ > > + for (i = 1; i < 7; i++) { > > + reg = vsc85xx_ts_read_csr(ptp->phydev, PROCESSOR, > > + MSCC_PHY_PTP_EGR_TS_FIFO(i)); > > + *p++ = reg & 0xff; > > + *p++ = (reg >> 8) & 0xff; > > + *p++ = (reg >> 16) & 0xff; > > + *p++ = (reg >> 24) & 0xff; > > + } > > + > > +next_in_queue: > > + skb = skb_dequeue(&ptp->tx_queue); > > + if (!skb || skb == first_skb) > > + goto out; > > + > > + /* Keep the first skb to avoid looping over it again. */ > > + if (!first_skb) > > + first_skb = skb; > > + > > + /* Can't get the signature of the packet, won't ever > > + * be able to have one so let's dequeue the packet. > > + */ > > + if (get_sig(skb, skb_sig) < 0) > > + goto next_in_queue; > > + > > + /* Valid signature but does not match the one of the > > + * packet in the FIFO right now, reschedule it for later > > + * packets. > > + */ > > + if (memcmp(skb_sig, fifo.sig, sizeof(fifo.sig))) { > > + skb_queue_tail(&ptp->tx_queue, skb); > > + goto next_in_queue; > > + } > > + > > + ns = fifo.ns; > > + secs = fifo.secs; > > + > > + memset(&shhwtstamps, 0, sizeof(shhwtstamps)); > > + shhwtstamps.hwtstamp = ktime_set(secs, ns); > > + skb_complete_tx_timestamp(skb, &shhwtstamps); > > + > > +out: > > + /* If other timestamps are available in the FIFO, process them. */ > > + reg = vsc85xx_ts_read_csr(ptp->phydev, PROCESSOR, > > + MSCC_PHY_PTP_EGR_TS_FIFO_CTRL); > > + if (PTP_EGR_FIFO_LEVEL_LAST_READ(reg) > 1) > > + goto next_in_fifo; > > +} > > AFAICT, there is no need for labels and jumps here. Two nested 'for' > loops will do nicely. The inner skb loop can be in a helper function > for clarity. Be sure to use the "safe" iterator over the skbs. Using helper functions for clarity, I could move to using loops. I'll try that and if it improves readability I'll change this for v2. > > +static void vsc85xx_txtstamp(struct mii_timestamper *mii_ts, > > + struct sk_buff *skb, int type) > > +{ > > + struct vsc8531_
Re: [PATCH net-next 5/8] net: phy: mscc: 1588 block initialization
Hello Jakub, Quoting Jakub Kicinski (2020-05-27 19:35:13) > > This doesn't build on my system :S I'll have a look at this and fix it for v2. Thanks for reporting it! Antoine > > In file included from ../drivers/net/phy/mscc/mscc_ptp.c:18: > ../include/linux/unaligned/be_byteshift.h:41:19: error: redefinition of > ‘get_unaligned_be16’ >41 | static inline u16 get_unaligned_be16(const void *p) > | ^~ > In file included from ../arch/x86/include/asm/unaligned.h:9, > from ../include/linux/etherdevice.h:24, > from ../include/linux/if_vlan.h:11, > from ../include/linux/filter.h:22, > from ../include/net/sock.h:59, > from ../include/net/inet_sock.h:22, > from ../include/linux/udp.h:16, > from ../drivers/net/phy/mscc/mscc_ptp.c:17: > ../include/linux/unaligned/access_ok.h:23:28: note: previous definition of > ‘get_unaligned_be16’ was here >23 | static __always_inline u16 get_unaligned_be16(const void *p) > |^~ > In file included from ../drivers/net/phy/mscc/mscc_ptp.c:18: > ../include/linux/unaligned/be_byteshift.h:46:19: error: redefinition of > ‘get_unaligned_be32’ >46 | static inline u32 get_unaligned_be32(const void *p) > | ^~ > In file included from ../arch/x86/include/asm/unaligned.h:9, > from ../include/linux/etherdevice.h:24, > from ../include/linux/if_vlan.h:11, > from ../include/linux/filter.h:22, > from ../include/net/sock.h:59, > from ../include/net/inet_sock.h:22, > from ../include/linux/udp.h:16, > from ../drivers/net/phy/mscc/mscc_ptp.c:17: > ../include/linux/unaligned/access_ok.h:28:28: note: previous definition of > ‘get_unaligned_be32’ was here >28 | static __always_inline u32 get_unaligned_be32(const void *p) > |^~ > In file included from ../drivers/net/phy/mscc/mscc_ptp.c:18: > ../include/linux/unaligned/be_byteshift.h:51:19: error: redefinition of > ‘get_unaligned_be64’ >51 | static inline u64 get_unaligned_be64(const void *p) > | ^~ > In file included from ../arch/x86/include/asm/unaligned.h:9, > from ../include/linux/etherdevice.h:24, > from ../include/linux/if_vlan.h:11, > from ../include/linux/filter.h:22, > from ../include/net/sock.h:59, > from ../include/net/inet_sock.h:22, > from ../include/linux/udp.h:16, > from ../drivers/net/phy/mscc/mscc_ptp.c:17: > ../include/linux/unaligned/access_ok.h:33:28: note: previous definition of > ‘get_unaligned_be64’ was here >33 | static __always_inline u64 get_unaligned_be64(const void *p) > |^~ > In file included from ../drivers/net/phy/mscc/mscc_ptp.c:18: > ../include/linux/unaligned/be_byteshift.h:56:20: error: redefinition of > ‘put_unaligned_be16’ >56 | static inline void put_unaligned_be16(u16 val, void *p) > |^~ > In file included from ../arch/x86/include/asm/unaligned.h:9, > from ../include/linux/etherdevice.h:24, > from ../include/linux/if_vlan.h:11, > from ../include/linux/filter.h:22, > from ../include/net/sock.h:59, > from ../include/net/inet_sock.h:22, > from ../include/linux/udp.h:16, > from ../drivers/net/phy/mscc/mscc_ptp.c:17: > ../include/linux/unaligned/access_ok.h:53:29: note: previous definition of > ‘put_unaligned_be16’ was here >53 | static __always_inline void put_unaligned_be16(u16 val, void *p) > | ^~ > In file included from ../drivers/net/phy/mscc/mscc_ptp.c:18: > ../include/linux/unaligned/be_byteshift.h:61:20: error: redefinition of > ‘put_unaligned_be32’ >61 | static inline void put_unaligned_be32(u32 val, void *p) > |^~ > In file included from ../arch/x86/include/asm/unaligned.h:9, > from ../include/linux/etherdevice.h:24, > from ../include/linux/if_vlan.h:11, > from ../include/linux/filter.h:22, > from ../include/net/sock.h:59, > from ../include/net/inet_sock.h:22, > from ../include/linux/u
[PATCH net-next 7/8] dt-bindings: net: phy: vsc8531: document the load/save GPIO
A new optional property can be used to reference the load/save GPIO, used for PTP hardware clock (PHC) operations. This patch documents it in the binding documentation. Signed-off-by: Antoine Tenart --- Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt index 5ff37c68c941..87a27d775d48 100644 --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt @@ -31,6 +31,8 @@ Optional properties: VSC8531_LINK_100_ACTIVITY (2), VSC8531_LINK_ACTIVITY (0) and VSC8531_DUPLEX_COLLISION (8). +- load-save-gpios : GPIO used for the load/save operation of the PTP + hardware clock (PHC). Table: 1 - Edge rate change @@ -67,4 +69,5 @@ Example: vsc8531,edge-slowdown = <7>; vsc8531,led-0-mode = ; vsc8531,led-1-mode = ; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; -- 2.26.2
[PATCH net-next 6/8] net: phy: mscc: timestamping and PHC support
This patch adds support for PHC and timestamping operations for the MSCC PHY. PTP 1-step and 2-step modes are supported, over Ethernet and UDP. To get and set the PHC time, a GPIO has to be used and changes are only retrieved or committed when on a rising edge. The same GPIO is shared by all PHYs, so the granularity of the lock protecting it has to be different from the ones protecting the 1588 registers (the VSC8584 PHY has 2 1588 blocks, and a single load/save pin). Co-developed-by: Quentin Schulz Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc.h | 17 + drivers/net/phy/mscc/mscc_main.c | 11 +- drivers/net/phy/mscc/mscc_ptp.c | 573 +++ 3 files changed, 600 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index 913fdb65df7d..63d0ed64c93e 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -375,8 +375,13 @@ struct vsc8531_private { unsigned long egr_flows; #endif + struct mii_timestamper mii_ts; + bool input_clk_init; struct vsc85xx_ptp *ptp; + /* LOAD/SAVE GPIO pin, used for retrieving or setting time to the PHC. +*/ + struct gpio_desc *load_save; /* For multiple port PHYs; the MDIO address of the base PHY in the * pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are coupled. @@ -388,12 +393,15 @@ struct vsc8531_private { }; /* Shared structure between the PHYs of the same package. + * phc_lock: used for PHC operations. Common for all PHYs as the load/save GPIO + * is shared. * ts_lock: used for TS operations; the base address used for PHC operation * isn't the same one as the base address for the package (PHC configuration * blocks are grouped by two PHYs whereas the package can have up to four PHYs * in total). */ struct vsc85xx_shared_private { + struct mutex phc_lock; struct mutex ts_lock[2]; }; @@ -423,13 +431,18 @@ static inline void vsc8584_config_macsec_intr(struct phy_device *phydev) #if IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) void vsc85xx_link_change_notify(struct phy_device *phydev); +void vsc8584_config_ts_intr(struct phy_device *phydev); int vsc8584_ptp_init(struct phy_device *phydev); int vsc8584_ptp_probe_once(struct phy_device *phydev); int vsc8584_ptp_probe(struct phy_device *phydev); +irqreturn_t vsc8584_handle_ts_interrupt(struct phy_device *phydev); #else static inline void vsc85xx_link_change_notify(struct phy_device *phydev) { } +static inline void vsc8584_config_ts_intr(struct phy_device *phydev) +{ +} static inline int vsc8584_ptp_init(struct phy_device *phydev) { return 0; @@ -442,6 +455,10 @@ static inline int vsc8584_ptp_probe(struct phy_device *phydev) { return 0; } +static inline irqreturn_t vsc8584_handle_ts_interrupt(struct phy_device *phydev) +{ + return IRQ_NONE; +} #endif #endif /* _MSCC_PHY_H_ */ diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 3d009eced0ef..3ff87f05c46b 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1477,12 +1477,20 @@ static int vsc8584_config_init(struct phy_device *phydev) static irqreturn_t vsc8584_handle_interrupt(struct phy_device *phydev) { + irqreturn_t ret; int irq_status; irq_status = phy_read(phydev, MII_VSC85XX_INT_STATUS); - if (irq_status < 0 || !(irq_status & MII_VSC85XX_INT_MASK_MASK)) + if (irq_status < 0) return IRQ_NONE; + /* Timestamping IRQ does not set a bit in the global INT_STATUS, so +* irq_status would be 0. +*/ + ret = vsc8584_handle_ts_interrupt(phydev); + if (!(irq_status & MII_VSC85XX_INT_MASK_MASK)) + return ret; + if (irq_status & MII_VSC85XX_INT_MASK_EXT) vsc8584_handle_macsec_interrupt(phydev); @@ -1922,6 +1930,7 @@ static int vsc85xx_config_intr(struct phy_device *phydev) if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { vsc8584_config_macsec_intr(phydev); + vsc8584_config_ts_intr(phydev); rc = phy_write(phydev, MII_VSC85XX_INT_MASK, MII_VSC85XX_INT_MASK_MASK); diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c index 279eec253e98..d326bca69ef7 100644 --- a/drivers/net/phy/mscc/mscc_ptp.c +++ b/drivers/net/phy/mscc/mscc_ptp.c @@ -370,6 +370,127 @@ static int vsc85xx_ts_eth_cmp1_sig(struct phy_device *phydev) return 0; } +static struct vsc85xx_ptphdr *get_ptp_header(struct sk_buff *skb) +{ + struct ethhdr *ethhdr = eth_hdr(skb); + struct iphdr *iphdr = ip_hdr(skb); + struct udphdr *udphdr; + __u8 proto; + + if (ethhdr->h_proto == htons(ETH_P_1588)) + return (struct vsc85xx_ptphd
[PATCH net-next 4/8] net: phy: mscc: take into account the 1588 block in MACsec init
This patch takes in account the use of the 1588 block in the MACsec initialization, as a conditional configuration has to be done (when the 1588 block is used). Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_macsec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index c0eeb62cb940..713c62b1d1f0 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -285,7 +285,9 @@ static void vsc8584_macsec_mac_init(struct phy_device *phydev, MSCC_MAC_CFG_PKTINF_CFG_STRIP_PREAMBLE_ENA | MSCC_MAC_CFG_PKTINF_CFG_INSERT_PREAMBLE_ENA | (bank == HOST_MAC ? - MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : 0)); + MSCC_MAC_CFG_PKTINF_CFG_ENABLE_TX_PADDING : 0) | +(IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) ? + MSCC_MAC_CFG_PKTINF_CFG_MACSEC_BYPASS_NUM_PTP_STALL_CLKS(0x8) : 0)); val = vsc8584_macsec_phy_read(phydev, bank, MSCC_MAC_CFG_MODE_CFG); val &= ~MSCC_MAC_CFG_MODE_CFG_DISABLE_DIC; -- 2.26.2
[PATCH net-next 3/8] net: phy: mscc: remove the TR CLK disable magic value
From: Quentin Schulz This patch adds a define for the 0x8000 magic value used to perform enable/disable actions on the "token ring clock". The patch is only cosmetic. Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc.h | 1 + drivers/net/phy/mscc/mscc_main.c | 10 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index f828c917b9f7..54f1fcbdd9cf 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -252,6 +252,7 @@ enum rgmii_clock_delay { /* Test page Registers */ #define MSCC_PHY_TEST_PAGE_5 5 #define MSCC_PHY_TEST_PAGE_8 8 +#define TR_CLK_DISABLE 0x8000 #define MSCC_PHY_TEST_PAGE_9 9 #define MSCC_PHY_TEST_PAGE_2020 #define MSCC_PHY_TEST_PAGE_2424 diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 550acf547ced..40af506d973e 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -629,7 +629,7 @@ static int vsc8531_pre_init_seq_set(struct phy_device *phydev) if (rc < 0) return rc; rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_TEST, - MSCC_PHY_TEST_PAGE_8, 0x8000, 0x8000); + MSCC_PHY_TEST_PAGE_8, TR_CLK_DISABLE, TR_CLK_DISABLE); if (rc < 0) return rc; @@ -1026,7 +1026,7 @@ static int vsc8574_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_PHY_TEST_PAGE_5, 0x1b20); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg |= 0x8000; + reg |= TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR); @@ -1046,7 +1046,7 @@ static int vsc8574_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg &= ~0x8000; + reg &= ~TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); @@ -1196,7 +1196,7 @@ static int vsc8584_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_PHY_TEST_PAGE_5, 0x1f20); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg |= 0x8000; + reg |= TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR); @@ -1225,7 +1225,7 @@ static int vsc8584_config_pre_init(struct phy_device *phydev) phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST); reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8); - reg &= ~0x8000; + reg &= ~TR_CLK_DISABLE; phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg); phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); -- 2.26.2
[PATCH net-next 5/8] net: phy: mscc: 1588 block initialization
From: Quentin Schulz This patch adds the first parts of the 1588 support in the MSCC PHY, with registers definition and the 1588 block initialization. Those PHYs are distributed in hardware packages containing multiple times the PHY. The VSC8584 for example is composed of 4 PHYs. With hardware packages, parts of the logic is usually common and one of the PHY has to be used for some parts of the initialization. Following this logic, the 1588 blocks of those PHYs are shared between two PHYs and accessing the registers has to be done using the "base" PHY of the group. This is handled thanks to helpers in the PTP code (and locks). We also need the MDIO bus lock while performing a single read or write to the 1588 registers as the read/write are composed of multiple MDIO transactions (and we don't want other threads updating the page). Co-developed-by: Antoine Tenart Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/Makefile|4 + drivers/net/phy/mscc/mscc.h | 45 ++ drivers/net/phy/mscc/mscc_main.c | 42 +- drivers/net/phy/mscc/mscc_ptp.c | 1035 ++ drivers/net/phy/mscc/mscc_ptp.h | 477 ++ 5 files changed, 1599 insertions(+), 4 deletions(-) create mode 100644 drivers/net/phy/mscc/mscc_ptp.c create mode 100644 drivers/net/phy/mscc/mscc_ptp.h diff --git a/drivers/net/phy/mscc/Makefile b/drivers/net/phy/mscc/Makefile index 10af42cd9839..d8e22a4eeeff 100644 --- a/drivers/net/phy/mscc/Makefile +++ b/drivers/net/phy/mscc/Makefile @@ -8,3 +8,7 @@ mscc-objs := mscc_main.o ifdef CONFIG_MACSEC mscc-objs += mscc_macsec.o endif + +ifdef CONFIG_NETWORK_PHY_TIMESTAMPING +mscc-objs += mscc_ptp.o +endif diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index 54f1fcbdd9cf..913fdb65df7d 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -133,6 +133,7 @@ enum rgmii_clock_delay { * in the same package. */ #define MSCC_PHY_PAGE_EXTENDED_GPIO 0x0010 /* Extended reg - GPIO */ +#define MSCC_PHY_PAGE_1588 0x1588 /* PTP (1588) */ #define MSCC_PHY_PAGE_TEST 0x2a30 /* Test reg */ #define MSCC_PHY_PAGE_TR 0x52b5 /* Token ring registers */ @@ -373,6 +374,27 @@ struct vsc8531_private { unsigned long ingr_flows; unsigned long egr_flows; #endif + + bool input_clk_init; + struct vsc85xx_ptp *ptp; + + /* For multiple port PHYs; the MDIO address of the base PHY in the +* pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are coupled. +* PHY1 and PHY3 as well. PHY0 and PHY1 are base PHYs for their +* respective pair. +*/ + unsigned int ts_base_addr; + u8 ts_base_phy; +}; + +/* Shared structure between the PHYs of the same package. + * ts_lock: used for TS operations; the base address used for PHC operation + * isn't the same one as the base address for the package (PHC configuration + * blocks are grouped by two PHYs whereas the package can have up to four PHYs + * in total). + */ +struct vsc85xx_shared_private { + struct mutex ts_lock[2]; }; #ifdef CONFIG_OF_MDIO @@ -399,4 +421,27 @@ static inline void vsc8584_config_macsec_intr(struct phy_device *phydev) } #endif +#if IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING) +void vsc85xx_link_change_notify(struct phy_device *phydev); +int vsc8584_ptp_init(struct phy_device *phydev); +int vsc8584_ptp_probe_once(struct phy_device *phydev); +int vsc8584_ptp_probe(struct phy_device *phydev); +#else +static inline void vsc85xx_link_change_notify(struct phy_device *phydev) +{ +} +static inline int vsc8584_ptp_init(struct phy_device *phydev) +{ + return 0; +} +static inline int vsc8584_ptp_probe_once(struct phy_device *phydev) +{ + return 0; +} +static inline int vsc8584_ptp_probe(struct phy_device *phydev) +{ + return 0; +} +#endif + #endif /* _MSCC_PHY_H_ */ diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 40af506d973e..3d009eced0ef 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1299,11 +1299,29 @@ static void vsc8584_get_base_addr(struct phy_device *phydev) __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); mutex_unlock(&phydev->mdio.bus->mdio_lock); - if (val & PHY_ADDR_REVERSED) + /* In the package, there are two pairs of PHYs (PHY0 + PHY2 and +* PHY1 + PHY3). The first PHY of each pair (PHY0 and PHY1) is +* the base PHY for timestamping operations. +*/ + if (val & PHY_ADDR_REVERSED) { vsc8531->base_addr = phydev->mdio.addr + addr; - else + vsc8531->ts_base_addr = phydev->mdio.addr; + vsc8531->ts_base_phy = addr; + if (addr > 1) { + vsc8531->ts_base_addr += 2; +
[PATCH net-next 8/8] MIPS: dts: ocelot: describe the load/save GPIO
From: Quentin Schulz This patch adds a description of the load/save GPIN pin, used in the VSC8584 PHY for timestamping operations. The related pinctrl description is also added. Signed-off-by: Quentin Schulz Signed-off-by: Antoine Tenart --- arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts index 33991fd209f5..897de5025d7f 100644 --- a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts +++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts @@ -3,6 +3,7 @@ /dts-v1/; +#include #include #include #include "ocelot.dtsi" @@ -25,6 +26,11 @@ phy_int_pins: phy_int_pins { pins = "GPIO_4"; function = "gpio"; }; + + phy_load_save_pins: phy_load_save_pins { + pins = "GPIO_10"; + function = "ptp2"; + }; }; &mdio0 { @@ -34,27 +40,31 @@ &mdio0 { &mdio1 { status = "okay"; pinctrl-names = "default"; - pinctrl-0 = <&miim1>, <&phy_int_pins>; + pinctrl-0 = <&miim1>, <&phy_int_pins>, <&phy_load_save_pins>; phy7: ethernet-phy@0 { reg = <0>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy6: ethernet-phy@1 { reg = <1>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy5: ethernet-phy@2 { reg = <2>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; phy4: ethernet-phy@3 { reg = <3>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&gpio>; + load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; }; }; -- 2.26.2
[PATCH net-next 1/8] net: phy: add support for a common probe between shared PHYs
Shared PHYs (PHYs in the same hardware package) may have shared registers and their drivers would usually need to share information. There is currently a way to have a shared (part of the) init, by using phy_package_init_once(). This patch extends the logic to share parts of the probe to allow sharing the initialization of locks or resources retrieval. Signed-off-by: Antoine Tenart --- include/linux/phy.h | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/include/linux/phy.h b/include/linux/phy.h index 8c05d0fb5c00..058219b6441f 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -244,7 +244,8 @@ struct phy_package_shared { }; /* used as bit number in atomic bitops */ -#define PHY_SHARED_F_INIT_DONE 0 +#define PHY_SHARED_F_INIT_DONE 0 +#define PHY_SHARED_F_PROBE_DONE 1 /* * The Bus class for PHYs. Devices which provide access to @@ -1554,14 +1555,25 @@ static inline int __phy_package_write(struct phy_device *phydev, return __mdiobus_write(phydev->mdio.bus, shared->addr, regnum, val); } -static inline bool phy_package_init_once(struct phy_device *phydev) +static inline bool __phy_package_set_once(struct phy_device *phydev, + unsigned int b) { struct phy_package_shared *shared = phydev->shared; if (!shared) return false; - return !test_and_set_bit(PHY_SHARED_F_INIT_DONE, &shared->flags); + return !test_and_set_bit(b, &shared->flags); +} + +static inline bool phy_package_init_once(struct phy_device *phydev) +{ + return __phy_package_set_once(phydev, PHY_SHARED_F_INIT_DONE); +} + +static inline bool phy_package_probe_once(struct phy_device *phydev) +{ + return __phy_package_set_once(phydev, PHY_SHARED_F_PROBE_DONE); } extern struct bus_type mdio_bus_type; -- 2.26.2
[PATCH net-next 2/8] net: phy: mscc: fix copyright and author information in MACsec
All headers in the MSCC PHY driver have been copied and pasted from the original mscc.c file. However the information is not necessarily correct, as in the MACsec support. Fix this. Signed-off-by: Antoine Tenart --- drivers/net/phy/mscc/mscc_fc_buffer.h | 2 +- drivers/net/phy/mscc/mscc_mac.h | 2 +- drivers/net/phy/mscc/mscc_macsec.c| 6 +++--- drivers/net/phy/mscc/mscc_macsec.h| 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/mscc/mscc_fc_buffer.h b/drivers/net/phy/mscc/mscc_fc_buffer.h index 3803e826c37d..399e803395a5 100644 --- a/drivers/net/phy/mscc/mscc_fc_buffer.h +++ b/drivers/net/phy/mscc/mscc_fc_buffer.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (C) 2019 Microsemi Corporation + * Copyright (C) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_FC_BUFFER_H_ diff --git a/drivers/net/phy/mscc/mscc_mac.h b/drivers/net/phy/mscc/mscc_mac.h index 59b6837c60b3..8dd38dc6edbf 100644 --- a/drivers/net/phy/mscc/mscc_mac.h +++ b/drivers/net/phy/mscc/mscc_mac.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (c) 2017 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_LINE_MAC_H_ diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index b4d3dc4068e2..c0eeb62cb940 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -1,10 +1,10 @@ // SPDX-License-Identifier: (GPL-2.0 OR MIT) /* - * Driver for Microsemi VSC85xx PHYs + * Driver for Microsemi VSC85xx PHYs - MACsec support * - * Author: Nagaraju Lakkaraju + * Author: Antoine Tenart * License: Dual MIT/GPL - * Copyright (c) 2016 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #include diff --git a/drivers/net/phy/mscc/mscc_macsec.h b/drivers/net/phy/mscc/mscc_macsec.h index d751f2946b79..9c6d25e36de2 100644 --- a/drivers/net/phy/mscc/mscc_macsec.h +++ b/drivers/net/phy/mscc/mscc_macsec.h @@ -2,7 +2,7 @@ /* * Driver for Microsemi VSC85xx PHYs * - * Copyright (c) 2018 Microsemi Corporation + * Copyright (c) 2020 Microsemi Corporation */ #ifndef _MSCC_PHY_MACSEC_H_ -- 2.26.2
[PATCH net-next 0/8] net: phy: mscc: PHC and timestamping support
Hello, This series aims at adding support for PHC and timestamping operations in the MSCC PHY driver, for the VSC858x and VSC8575. Those PHYs are capable of timestamping in 1-step and 2-step for both L2 and L4 traffic. As of this series, only IPv4 support was implemented when using L4 mode. This is because of an hardware limitation which prevents us for supporting both IPv4 and IPv6 at the same time. Implementing support for IPv6 should be quite easy (I do have the modifications needed for the hardware configuration) but I did not see a way to retrieve this information in hwtstamp(). What would you suggest? Those PHYs are distributed in hardware packages containing multiple times the PHY. The VSC8584 for example is composed of 4 PHYs. With hardware packages, parts of the logic is usually common and one of the PHY has to be used for some parts of the initialization. Following this logic, the 1588 blocks of those PHYs are shared between two PHYs and accessing the registers has to be done using the "base" PHY of the group. This is handled thanks to helpers in the PTP code (and locks). We also need the MDIO bus lock while performing a single read or write to the 1588 registers as the read/write are composed of multiple MDIO transactions (and we don't want other threads updating the page). To get and set the PHC time, a GPIO has to be used and changes are only retrieved or committed when on a rising edge. The same GPIO is shared by all PHYs, so the granularity of the lock protecting it has to be different from the ones protecting the 1588 registers (the VSC8584 PHY has 2 1588 blocks, and a single load/save pin). Patch 1 extends the recently added helpers to share information between PHYs of the same hardware package; to allow having part of the probe to be shared (in addition to the already supported init part). This will be used when adding support for PHC/TS to initialize locks. Patches 2 and 3 are mostly cosmetic. Patch 4 takes into account the 1588 block in the MACsec initialization, to allow having both the MACsec and 1588 blocks initialized on a running system. Patches 5 and 6 add support for PHC and timestamping operations in the MSCC driver. An initialization of the 1588 block (plus all the registers definition; and helpers) is added first; and then comes a patch to implement the PHC and timestamping API. Patches 7 and 8 add the required hardware description for device trees, to be able to use the load/save GPIO pin on the PCB120 board. To use this on a PCB120 board, two other series are needed and have already been sent upstream (one is merged). There are no dependency between all those series. Thanks! Antoine Antoine Tenart (5): net: phy: add support for a common probe between shared PHYs net: phy: mscc: fix copyright and author information in MACsec net: phy: mscc: take into account the 1588 block in MACsec init net: phy: mscc: timestamping and PHC support dt-bindings: net: phy: vsc8531: document the load/save GPIO Quentin Schulz (3): net: phy: mscc: remove the TR CLK disable magic value net: phy: mscc: 1588 block initialization MIPS: dts: ocelot: describe the load/save GPIO .../bindings/net/mscc-phy-vsc8531.txt |3 + arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 12 +- drivers/net/phy/mscc/Makefile |4 + drivers/net/phy/mscc/mscc.h | 63 + drivers/net/phy/mscc/mscc_fc_buffer.h |2 +- drivers/net/phy/mscc/mscc_mac.h |2 +- drivers/net/phy/mscc/mscc_macsec.c| 10 +- drivers/net/phy/mscc/mscc_macsec.h|2 +- drivers/net/phy/mscc/mscc_main.c | 63 +- drivers/net/phy/mscc/mscc_ptp.c | 1608 + drivers/net/phy/mscc/mscc_ptp.h | 477 + include/linux/phy.h | 18 +- 12 files changed, 2243 insertions(+), 21 deletions(-) create mode 100644 drivers/net/phy/mscc/mscc_ptp.c create mode 100644 drivers/net/phy/mscc/mscc_ptp.h -- 2.26.2
Re: [PATCH net-next 0/4] net: phy: mscc-miim: reduce waiting time between MDIO transactions
Hi Andrew, Quoting Andrew Lunn (2020-05-26 19:01:00) > On Tue, May 26, 2020 at 06:22:52PM +0200, Antoine Tenart wrote: > > > > This series aims at reducing the waiting time between MDIO transactions > > when using the MSCC MIIM MDIO controller. > > There are a couple of other things you can look at: > > Can you disable the pre-amble on the MDIO transaction. It requires > that both the bus master and all devices on the bus support it, but > when it is usable, you half the number of bits sent over the wire. > > Can you control the frequency of MDC? 802.3 says 2.5MHz, but many > devices support higher speeds. Again, you need all devices on the bus > to support the speed. > > When accessing raw TDR data for cable tests i also have a lot of PHY > accesses. I implemented both of these for the FEC MDIO bus, and made > it a lot faster. Thanks for the tips! Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH net-next 2/4] net: phy: mscc-miim: remove redundant timeout check
readl_poll_timeout already returns -ETIMEDOUT if the condition isn't satisfied, there's no need to check again the condition after calling it. Remove the redundant timeout check. Signed-off-by: Antoine Tenart --- drivers/net/phy/mdio-mscc-miim.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c index 0b7544f593fb..42119f661452 100644 --- a/drivers/net/phy/mdio-mscc-miim.c +++ b/drivers/net/phy/mdio-mscc-miim.c @@ -43,12 +43,8 @@ static int mscc_miim_wait_ready(struct mii_bus *bus) struct mscc_miim_dev *miim = bus->priv; u32 val; - readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, - !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 1); - if (val & MSCC_MIIM_STATUS_STAT_BUSY) - return -ETIMEDOUT; - - return 0; + return readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, + !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 1); } static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum) -- 2.26.2
[PATCH net-next 4/4] net: phy: mscc-miim: read poll when high resolution timers are disabled
The driver uses a read polling mechanism to check the status of the MDIO bus, to know if it is ready to accept next commands. This polling mechanism uses usleep_delay() under the hood between reads which is fine as long as high resolution timers are enabled. Otherwise the delays will end up to be much longer than expected. This patch fixes this by using udelay() under the hood when CONFIG_HIGH_RES_TIMERS isn't enabled. This increases CPU usage. Signed-off-by: Antoine Tenart --- drivers/net/phy/Kconfig | 3 ++- drivers/net/phy/mdio-mscc-miim.c | 22 +- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 2a32f26ead0b..047c27087b10 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -184,7 +184,8 @@ config MDIO_MSCC_MIIM depends on HAS_IOMEM help This driver supports the MIIM (MDIO) interface found in the network - switches of the Microsemi SoCs + switches of the Microsemi SoCs; it is recommended to switch on + CONFIG_HIGH_RES_TIMERS config MDIO_MVUSB tristate "Marvell USB to MDIO Adapter" diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c index aed9afa1e8f1..11f583fd4611 100644 --- a/drivers/net/phy/mdio-mscc-miim.c +++ b/drivers/net/phy/mdio-mscc-miim.c @@ -39,13 +39,25 @@ struct mscc_miim_dev { void __iomem *phy_regs; }; +/* When high resolution timers aren't built-in: we can't use usleep_range() as + * we would sleep way too long. Use udelay() instead. + */ +#define mscc_readl_poll_timeout(addr, val, cond, delay_us, timeout_us) \ +({ \ + if (!IS_ENABLED(CONFIG_HIGH_RES_TIMERS))\ + readl_poll_timeout_atomic(addr, val, cond, delay_us,\ + timeout_us); \ + readl_poll_timeout(addr, val, cond, delay_us, timeout_us); \ +}) + static int mscc_miim_wait_ready(struct mii_bus *bus) { struct mscc_miim_dev *miim = bus->priv; u32 val; - return readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, - !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 1); + return mscc_readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, + !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, + 1); } static int mscc_miim_wait_pending(struct mii_bus *bus) @@ -53,9 +65,9 @@ static int mscc_miim_wait_pending(struct mii_bus *bus) struct mscc_miim_dev *miim = bus->priv; u32 val; - return readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, - !(val & MSCC_MIIM_STATUS_STAT_PENDING), - 50, 1); + return mscc_readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, + !(val & MSCC_MIIM_STATUS_STAT_PENDING), + 50, 1); } static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum) -- 2.26.2
[PATCH net-next 0/4] net: phy: mscc-miim: reduce waiting time between MDIO transactions
Hello, This series aims at reducing the waiting time between MDIO transactions when using the MSCC MIIM MDIO controller. I'm not sure we need patch 4/4 and we could reasonably drop it from the series. I'm including the patch as it could help to ensure the system is functional with a non optimal configuration. We needed to improve the driver's performances as when using a PHY requiring lots of registers accesses (such as the VSC85xx family), delays would add up and ended up to be quite large which would cause issues such as: a slow initialization of the PHY, and issues when using timestamping operations (this feature will be sent quite soon to the mailing lists). Thanks, Antoine Antoine Tenart (4): net: phy: mscc-miim: use more reasonable delays net: phy: mscc-miim: remove redundant timeout check net: phy: mscc-miim: improve waiting logic net: phy: mscc-miim: read poll when high resolution timers are disabled drivers/net/phy/Kconfig | 3 ++- drivers/net/phy/mdio-mscc-miim.c | 33 +--- 2 files changed, 28 insertions(+), 8 deletions(-) -- 2.26.2
[PATCH net-next 3/4] net: phy: mscc-miim: improve waiting logic
The MSCC MIIM MDIO driver uses a waiting logic to wait for the MDIO bus to be ready to accept next commands. It does so by polling the BUSY status bit which indicates the MDIO bus has completed all pending operations. This can take time, and the controller supports writing the next command as soon as there are no pending commands (which happens while the MDIO bus is busy completing its current command). This patch implements this improved logic by adding an helper to poll the PENDING status bit, and by adjusting where we should wait for the bus to not be busy or to not be pending. Signed-off-by: Antoine Tenart --- drivers/net/phy/mdio-mscc-miim.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c index 42119f661452..aed9afa1e8f1 100644 --- a/drivers/net/phy/mdio-mscc-miim.c +++ b/drivers/net/phy/mdio-mscc-miim.c @@ -16,6 +16,7 @@ #include #define MSCC_MIIM_REG_STATUS 0x0 +#defineMSCC_MIIM_STATUS_STAT_PENDING BIT(2) #defineMSCC_MIIM_STATUS_STAT_BUSY BIT(3) #define MSCC_MIIM_REG_CMD 0x8 #defineMSCC_MIIM_CMD_OPR_WRITE BIT(1) @@ -47,13 +48,23 @@ static int mscc_miim_wait_ready(struct mii_bus *bus) !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 1); } +static int mscc_miim_wait_pending(struct mii_bus *bus) +{ + struct mscc_miim_dev *miim = bus->priv; + u32 val; + + return readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, + !(val & MSCC_MIIM_STATUS_STAT_PENDING), + 50, 1); +} + static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum) { struct mscc_miim_dev *miim = bus->priv; u32 val; int ret; - ret = mscc_miim_wait_ready(bus); + ret = mscc_miim_wait_pending(bus); if (ret) goto out; @@ -82,7 +93,7 @@ static int mscc_miim_write(struct mii_bus *bus, int mii_id, struct mscc_miim_dev *miim = bus->priv; int ret; - ret = mscc_miim_wait_ready(bus); + ret = mscc_miim_wait_pending(bus); if (ret < 0) goto out; -- 2.26.2
[PATCH net-next 1/4] net: phy: mscc-miim: use more reasonable delays
The MSCC MIIM MDIO driver uses delays to read poll a status register. I made multiple tests on a Ocelot PCS120 platform which led me to reduce those delays. The delay in between which the polling function is allowed to sleep is reduced from 100us to 50us which in almost all cases is a good value to succeed at the first retry. The overall delay is also lowered as the prior value was really way to high, 1us is large enough. Signed-off-by: Antoine Tenart --- drivers/net/phy/mdio-mscc-miim.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c index badbc99bedd3..0b7544f593fb 100644 --- a/drivers/net/phy/mdio-mscc-miim.c +++ b/drivers/net/phy/mdio-mscc-miim.c @@ -44,7 +44,7 @@ static int mscc_miim_wait_ready(struct mii_bus *bus) u32 val; readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, - !(val & MSCC_MIIM_STATUS_STAT_BUSY), 100, 25); + !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 1); if (val & MSCC_MIIM_STATUS_STAT_BUSY) return -ETIMEDOUT; -- 2.26.2
[PATCH net-next 0/2] net: mscc: allow forwarding ioctl operations to attached PHYs
Hello, These two patches allow forwarding ioctl to the PHY MII implementation, and support is added for offloading timestamping operations to compatible attached PHYs. Thanks, Antoine Antoine Tenart (2): net: mscc: use the PHY MII ioctl interface when possible net: mscc: allow offloading timestamping operations to the PHY drivers/net/ethernet/mscc/ocelot.c | 23 --- drivers/net/ethernet/mscc/ocelot_board.c | 3 ++- 2 files changed, 14 insertions(+), 12 deletions(-) -- 2.26.2
[PATCH net-next 1/2] net: mscc: use the PHY MII ioctl interface when possible
Allow ioctl to be implemented by the PHY, when a PHY is attached to the Ocelot switch. In case the ioctl is a request to set or get the hardware timestamp, use the Ocelot switch implementation for now. Signed-off-by: Antoine Tenart --- drivers/net/ethernet/mscc/ocelot.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index e621c4c3ee86..2151c08a57c7 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -1204,18 +1204,16 @@ static int ocelot_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) struct ocelot *ocelot = priv->port.ocelot; int port = priv->chip_port; - /* The function is only used for PTP operations for now */ - if (!ocelot->ptp) - return -EOPNOTSUPP; - - switch (cmd) { - case SIOCSHWTSTAMP: - return ocelot_hwstamp_set(ocelot, port, ifr); - case SIOCGHWTSTAMP: - return ocelot_hwstamp_get(ocelot, port, ifr); - default: - return -EOPNOTSUPP; + if (ocelot->ptp) { + switch (cmd) { + case SIOCSHWTSTAMP: + return ocelot_hwstamp_set(ocelot, port, ifr); + case SIOCGHWTSTAMP: + return ocelot_hwstamp_get(ocelot, port, ifr); + } } + + return phy_mii_ioctl(dev->phydev, ifr, cmd); } static const struct net_device_ops ocelot_port_netdev_ops = { -- 2.26.2
[PATCH net-next 2/2] net: mscc: allow offloading timestamping operations to the PHY
This patch adds support for offloading timestamping operations not only to the Ocelot switch (as already supported) but to compatible PHYs. When both the PHY and the Ocelot switch support timestamping operations, the PHY implementation is chosen as the timestamp will happen closer to the medium. Signed-off-by: Antoine Tenart --- drivers/net/ethernet/mscc/ocelot.c | 5 - drivers/net/ethernet/mscc/ocelot_board.c | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index 2151c08a57c7..9cfe1fd98c30 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -1204,7 +1204,10 @@ static int ocelot_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) struct ocelot *ocelot = priv->port.ocelot; int port = priv->chip_port; - if (ocelot->ptp) { + /* If the attached PHY device isn't capable of timestamping operations, +* use our own (when possible). +*/ + if (!phy_has_hwtstamp(dev->phydev) && ocelot->ptp) { switch (cmd) { case SIOCSHWTSTAMP: return ocelot_hwstamp_set(ocelot, port, ifr); diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c index 67a8d61c926a..4a15d2ff8b70 100644 --- a/drivers/net/ethernet/mscc/ocelot_board.c +++ b/drivers/net/ethernet/mscc/ocelot_board.c @@ -189,7 +189,8 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg) skb->offload_fwd_mark = 1; skb->protocol = eth_type_trans(skb, dev); - netif_rx(skb); + if (!skb_defer_rx_timestamp(skb)) + netif_rx(skb); dev->stats.rx_bytes += len; dev->stats.rx_packets++; } while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp)); -- 2.26.2
[PATCH net v2] net: phy: mscc: fix initialization of the MACsec protocol mode
At the very end of the MACsec block initialization in the MSCC PHY driver, the MACsec "protocol mode" is set. This setting should be set based on the PHY id within the package, as the bank used to access the register used depends on this. This was not done correctly, and only the first bank was used leading to the two upper PHYs being unstable when using the VSC8584. This patch fixes it. Fixes: 1bbe0ecc2a1a ("net: phy: mscc: macsec initialization") Signed-off-by: Antoine Tenart --- Since v1: - Resent to net. drivers/net/phy/mscc/mscc.h| 2 ++ drivers/net/phy/mscc/mscc_mac.h| 6 +++--- drivers/net/phy/mscc/mscc_macsec.c | 16 ++-- drivers/net/phy/mscc/mscc_macsec.h | 3 ++- drivers/net/phy/mscc/mscc_main.c | 4 5 files changed, 21 insertions(+), 10 deletions(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index 030bf8b600df..414e3b31bb1f 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -354,6 +354,8 @@ struct vsc8531_private { u64 *stats; int nstats; bool pkg_init; + /* PHY address within the package. */ + u8 addr; /* For multiple port PHYs; the MDIO address of the base PHY in the * package. */ diff --git a/drivers/net/phy/mscc/mscc_mac.h b/drivers/net/phy/mscc/mscc_mac.h index fcb5ba5e5d03..59b6837c60b3 100644 --- a/drivers/net/phy/mscc/mscc_mac.h +++ b/drivers/net/phy/mscc/mscc_mac.h @@ -152,8 +152,8 @@ #define MSCC_MAC_PAUSE_CFG_STATE_PAUSE_STATE BIT(0) #define MSCC_MAC_PAUSE_CFG_STATE_MAC_TX_PAUSE_GEN BIT(4) -#define MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL 0x2 -#define MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE(x) (x) -#define MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE_M GENMASK(2, 0) +#define MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL0x2 +#define MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE(x) (x) +#define MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE_M GENMASK(2, 0) #endif /* _MSCC_PHY_LINE_MAC_H_ */ diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index e99e2cd72a0c..b4d3dc4068e2 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -316,6 +316,8 @@ static void vsc8584_macsec_mac_init(struct phy_device *phydev, /* Must be called with mdio_lock taken */ static int __vsc8584_macsec_init(struct phy_device *phydev) { + struct vsc8531_private *priv = phydev->priv; + enum macsec_bank proc_bank; u32 val; vsc8584_macsec_block_init(phydev, MACSEC_INGR); @@ -351,12 +353,14 @@ static int __vsc8584_macsec_init(struct phy_device *phydev) val |= MSCC_FCBUF_ENA_CFG_TX_ENA | MSCC_FCBUF_ENA_CFG_RX_ENA; vsc8584_macsec_phy_write(phydev, FC_BUFFER, MSCC_FCBUF_ENA_CFG, val); - val = vsc8584_macsec_phy_read(phydev, IP_1588, - MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL); - val &= ~MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE_M; - val |= MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE(4); - vsc8584_macsec_phy_write(phydev, IP_1588, -MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL, val); + proc_bank = (priv->addr < 2) ? PROC_0 : PROC_2; + + val = vsc8584_macsec_phy_read(phydev, proc_bank, + MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL); + val &= ~MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE_M; + val |= MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE(4); + vsc8584_macsec_phy_write(phydev, proc_bank, +MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL, val); return 0; } diff --git a/drivers/net/phy/mscc/mscc_macsec.h b/drivers/net/phy/mscc/mscc_macsec.h index d0783944d106..d751f2946b79 100644 --- a/drivers/net/phy/mscc/mscc_macsec.h +++ b/drivers/net/phy/mscc/mscc_macsec.h @@ -64,7 +64,8 @@ enum macsec_bank { FC_BUFFER = 0x04, HOST_MAC= 0x05, LINE_MAC= 0x06, - IP_1588 = 0x0e, + PROC_0 = 0x0e, + PROC_2 = 0x0f, MACSEC_INGR = 0x38, MACSEC_EGR = 0x3c, }; diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index acddef79f4e8..c8aa6d905d8e 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1347,6 +1347,8 @@ static int vsc8584_config_init(struct phy_device *phydev) else vsc8531->base_addr = phydev->mdio.addr - addr; + vsc8531->addr = addr; + /* Some parts of the init sequence are identical for every PHY in the * package. Some parts are modifying the GPIO register bank which is a * set of registers that are affecting all PHYs, a few
Re: [PATCH net-next] net: phy: mscc: fix initialization of the MACsec protocol mode
Quoting David Miller (2020-05-22 02:31:05) > From: Antoine Tenart > Date: Wed, 20 May 2020 12:03:55 +0200 > > > What's the best way to handle this? I can provide all the patches. > > Resubmit this against 'net' please, then I'll deal with the fallout > when I merge net into net-next. OK, I'll resubmit against net. Thanks! Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH net-next] net: phy: mscc: fix initialization of the MACsec protocol mode
At the very end of the MACsec block initialization in the MSCC PHY driver, the MACsec "protocol mode" is set. This setting should be set based on the PHY id within the package, as the bank used to access the register used depends on this. This was not done correctly, and only the first bank was used leading to the two upper PHYs being unstable when using the VSC8584. This patch fixes it. Fixes: 1bbe0ecc2a1a ("net: phy: mscc: macsec initialization") Signed-off-by: Antoine Tenart --- Hi, This patch fixes a bug there since v5.6, but only applies on top of net-next because commit deb04e9c0ff2 changed the way base addresses are retrieved. To fix earlier versions two different patches have to be made, as the MSCC PHY driver was moved and split up into multiple files by commits da80aa52d074 and fa164e40c53b, between v5.6 and v5.7-rc1. To sum up, due to conflicts three (similar) patches have to be made to fix this issue: one for net-next, one for v5.7-rc1+ and one for v4.6. What's the best way to handle this? I can provide all the patches. Thanks! Antoine drivers/net/phy/mscc/mscc.h| 2 ++ drivers/net/phy/mscc/mscc_mac.h| 6 +++--- drivers/net/phy/mscc/mscc_macsec.c | 16 ++-- drivers/net/phy/mscc/mscc_macsec.h | 3 ++- drivers/net/phy/mscc/mscc_main.c | 2 ++ 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index acdd8ee61a39..f828c917b9f7 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -353,6 +353,8 @@ struct vsc8531_private { const struct vsc85xx_hw_stat *hw_stats; u64 *stats; int nstats; + /* PHY address within the package. */ + u8 addr; /* For multiple port PHYs; the MDIO address of the base PHY in the * package. */ diff --git a/drivers/net/phy/mscc/mscc_mac.h b/drivers/net/phy/mscc/mscc_mac.h index fcb5ba5e5d03..59b6837c60b3 100644 --- a/drivers/net/phy/mscc/mscc_mac.h +++ b/drivers/net/phy/mscc/mscc_mac.h @@ -152,8 +152,8 @@ #define MSCC_MAC_PAUSE_CFG_STATE_PAUSE_STATE BIT(0) #define MSCC_MAC_PAUSE_CFG_STATE_MAC_TX_PAUSE_GEN BIT(4) -#define MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL 0x2 -#define MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE(x) (x) -#define MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE_M GENMASK(2, 0) +#define MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL0x2 +#define MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE(x) (x) +#define MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE_M GENMASK(2, 0) #endif /* _MSCC_PHY_LINE_MAC_H_ */ diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c index e99e2cd72a0c..b4d3dc4068e2 100644 --- a/drivers/net/phy/mscc/mscc_macsec.c +++ b/drivers/net/phy/mscc/mscc_macsec.c @@ -316,6 +316,8 @@ static void vsc8584_macsec_mac_init(struct phy_device *phydev, /* Must be called with mdio_lock taken */ static int __vsc8584_macsec_init(struct phy_device *phydev) { + struct vsc8531_private *priv = phydev->priv; + enum macsec_bank proc_bank; u32 val; vsc8584_macsec_block_init(phydev, MACSEC_INGR); @@ -351,12 +353,14 @@ static int __vsc8584_macsec_init(struct phy_device *phydev) val |= MSCC_FCBUF_ENA_CFG_TX_ENA | MSCC_FCBUF_ENA_CFG_RX_ENA; vsc8584_macsec_phy_write(phydev, FC_BUFFER, MSCC_FCBUF_ENA_CFG, val); - val = vsc8584_macsec_phy_read(phydev, IP_1588, - MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL); - val &= ~MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE_M; - val |= MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE(4); - vsc8584_macsec_phy_write(phydev, IP_1588, -MSCC_PROC_0_IP_1588_TOP_CFG_STAT_MODE_CTL, val); + proc_bank = (priv->addr < 2) ? PROC_0 : PROC_2; + + val = vsc8584_macsec_phy_read(phydev, proc_bank, + MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL); + val &= ~MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE_M; + val |= MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL_PROTOCOL_MODE(4); + vsc8584_macsec_phy_write(phydev, proc_bank, +MSCC_PROC_IP_1588_TOP_CFG_STAT_MODE_CTL, val); return 0; } diff --git a/drivers/net/phy/mscc/mscc_macsec.h b/drivers/net/phy/mscc/mscc_macsec.h index d0783944d106..d751f2946b79 100644 --- a/drivers/net/phy/mscc/mscc_macsec.h +++ b/drivers/net/phy/mscc/mscc_macsec.h @@ -64,7 +64,8 @@ enum macsec_bank { FC_BUFFER = 0x04, HOST_MAC= 0x05, LINE_MAC= 0x06, - IP_1588 = 0x0e, + PROC_0 = 0x0e, + PROC_2 = 0x0f, MACSEC_INGR = 0x38, MACSEC_EGR = 0x3c, }; diff --git a/drivers/net/phy/mscc/mscc_main.
Re: [PATCH v5 0/6] Amazon's Annapurna Labs Alpine v3 device-tree
[Adding arm-soc] Hi Hanna, Sorry for the delay, the series was buried in my mails... Acked-by: Antoine Tenart Arnd, Olof, could you take this series directly as this will be the only Alpine patches for this release (and for a long time)? Thanks! Antoine Quoting Hanna Hawa (2020-03-24 11:49:12) > This series organize the Amazon's Annapurna Labs Alpine device tree > bindings, device tree folder and adds new device tree for Alpine v3. > > Changes since v4: > - > - Re-order nodes in increasing order. > - Add disable to UART nodes. > - Add missing UART nodes (1,2,3) > - Add comments for GIC/UART > - Add io-fabric bus, and move uart nodes into it. > - Fix MSIx range according Alpine function spec > > Changes since v3: > - > - rebased and retested for tag Linux 5.6-rc2 > > Changes since v2: > - > - Move up a level for DT node without mmio regs. > - Drop device_type from serial@fd883000 node. > - Minor change name of PCIe node to: pcie@fbd0 > > Changes since v1: > - > - Rename al,alpine DT binding to amazon,alpine > - Rename al folder to be amazon > - Update maintainers of amazon,alpine DT > - Add missing alpine-v2 DT binding > - Fix yaml schemas for alpine-v3-evp.dts: > - #size-cells:0:0: 0 is not one of [1, 2] > - arch-timer: interrupts: [[1, 13, 8, 1, 14, 8, 1, 11, 8, 1, 10, > 8]] is too short > - Change compatible string of alpine-v3-evp to amazon,al > > Hanna Hawa (5): > dt-bindings: arm: amazon: rename al,alpine DT binding to amazon,al > arm64: dts: amazon: rename al folder to be amazon > dt-bindings: arm: amazon: update maintainers of amazon,al DT bindings > dt-bindings: arm: amazon: add missing alpine-v2 DT binding > dt-bindings: arm: amazon: add Amazon Annapurna Labs Alpine V3 > > Ronen Krupnik (1): > arm64: dts: amazon: add Amazon's Annapurna Labs Alpine v3 support > > .../devicetree/bindings/arm/al,alpine.yaml| 21 - > .../devicetree/bindings/arm/amazon,al.yaml| 33 ++ > MAINTAINERS | 2 +- > arch/arm64/boot/dts/Makefile | 2 +- > arch/arm64/boot/dts/{al => amazon}/Makefile | 1 + > .../boot/dts/{al => amazon}/alpine-v2-evp.dts | 0 > .../boot/dts/{al => amazon}/alpine-v2.dtsi| 0 > arch/arm64/boot/dts/amazon/alpine-v3-evp.dts | 24 ++ > arch/arm64/boot/dts/amazon/alpine-v3.dtsi | 408 ++ > 9 files changed, 468 insertions(+), 23 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/arm/al,alpine.yaml > create mode 100644 Documentation/devicetree/bindings/arm/amazon,al.yaml > rename arch/arm64/boot/dts/{al => amazon}/Makefile (64%) > rename arch/arm64/boot/dts/{al => amazon}/alpine-v2-evp.dts (100%) > rename arch/arm64/boot/dts/{al => amazon}/alpine-v2.dtsi (100%) > create mode 100644 arch/arm64/boot/dts/amazon/alpine-v3-evp.dts > create mode 100644 arch/arm64/boot/dts/amazon/alpine-v3.dtsi > > -- > 2.17.1 > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH net] net: macsec: fix rtnl locking issue
netdev_update_features() must be called with the rtnl lock taken. Not doing so triggers a warning, as ASSERT_RTNL() is used in __netdev_update_features(), the first function called by netdev_update_features(). Fix this. Fixes: c850240b6c41 ("net: macsec: report real_dev features when HW offloading is enabled") Signed-off-by: Antoine Tenart --- drivers/net/macsec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index d4034025c87c..d0d31cb99180 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -2641,11 +2641,12 @@ static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info) if (ret) goto rollback; - rtnl_unlock(); /* Force features update, since they are different for SW MACSec and * HW offloading cases. */ netdev_update_features(dev); + + rtnl_unlock(); return 0; rollback: -- 2.26.2
Re: [PATCH net-next v2 3/3] net: phy: mscc: use phy_package_shared
Hello Michael, Quoting Michael Walle (2020-05-04 23:31:36) > > diff --git a/drivers/net/phy/mscc/mscc_main.c > b/drivers/net/phy/mscc/mscc_main.c > index 5391acdece05..a505286b2195 100644 > --- a/drivers/net/phy/mscc/mscc_main.c > +++ b/drivers/net/phy/mscc/mscc_main.c > -static bool vsc8584_is_pkg_init(struct phy_device *phydev, bool reversed) > +static void vsc8584_get_base_addr(struct phy_device *phydev) > { > - struct mii_bus *bus = phydev->mdio.bus; > - struct vsc8531_private *vsc8531; > - struct phy_device *phy; > - int i, addr; > - > - /* VSC8584 is a Quad PHY */ > - for (i = 0; i < 4; i++) { > - vsc8531 = phydev->priv; > - > - if (reversed) > - addr = vsc8531->base_addr - i; > - else > - addr = vsc8531->base_addr + i; > - > - phy = mdiobus_get_phy(bus, addr); > - if (!phy) > - continue; > + struct vsc8531_private *vsc8531 = phydev->priv; > + u16 val, addr; > > - if ((phy->phy_id & phydev->drv->phy_id_mask) != > - (phydev->drv->phy_id & phydev->drv->phy_id_mask)) > - continue; > + mutex_lock(&phydev->mdio.bus->mdio_lock); > + __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_EXTENDED); > > - vsc8531 = phy->priv; > + addr = __phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_4); > + addr >>= PHY_CNTL_4_ADDR_POS; > > - if (vsc8531 && vsc8531->pkg_init) > - return true; > - } > + val = __phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL); You should restore the page to MSCC_PHY_PAGE_STANDARD here. > + mutex_unlock(&phydev->mdio.bus->mdio_lock); > > - return false; > + if (val & PHY_ADDR_REVERSED) > + vsc8531->base_addr = phydev->mdio.addr + addr; > + else > + vsc8531->base_addr = phydev->mdio.addr - addr; > } Thanks for the series! Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Greetings From Mrs Elodie,
Greetings From Mrs Elodie, Calvary Greetings in the name of the LORD Almighty and Our LORD JESUS CHRIST the giver of every good thing. Good day,i know this letter will definitely come to you as a huge surprise, but I implore you to take the time to go through it carefully as the decision you make will go off a long way to determine my future and continued existence. I am Mrs Elodie Antoine aging widow of 59 years old suffering from long time illness. I have some funds I inherited from my late husband, The sum of (US$4.5 Million Dollars) and I needed a very honest and God fearing who can withdraw this money then use the funds for Charity works. I WISH TO GIVE THIS FUNDS TO YOU FOR CHARITY WORKS. I found your email address from the internet after honest prayers to the LORD to bring me a helper and i decided to contact you if you may be willing and interested to handle these trust funds in good faith before anything happens to me. I accept this decision because I do not have any child who will inherit this money after I die. I want your urgent reply to me so that I will give you the deposit receipt which the COMPANY issued to me as next of kin for immediate transfer of the money to your account in your country, to start the good work of God, I want you to use the 15/percent of the total amount to help yourself in doing the project. I am desperately in keen need of assistance and I have summoned up courage to contact you for this task, you must not fail me and the millions of the poor people in our todays WORLD. This is no stolen money and there are no dangers involved,100% RISK FREE with full legal proof. Please if you would be able to use the funds for the Charity works kindly let me know immediately.I will appreciate your utmost confidentiality and trust in this matter to accomplish my heart desire, as I don't want anything that will jeopardize my last wish. I want you to take 15 percent of the total money for your personal use while 85% of the money will go to charity.I will appreciate your utmost confidentiality and trust in this matter to accomplish my heart desire, as I don't want anything that will jeopardize my last wish. kindly respond for further details. reply to my private E-mail:( elodieantoine76...@yahoo.com ) Thanks and God bless you, Mrs Elodie Antoine
[PATCH net] net: cpsw: fix NULL pointer exception in the probe error path
In certain cases when the probe function fails the error path calls cpsw_remove_dt() before calling platform_set_drvdata(). This is an issue as cpsw_remove_dt() uses platform_get_drvdata() to retrieve the cpsw_common data and leds to a NULL pointer exception. This patches fixes it by calling platform_set_drvdata() earlier in the probe. Fixes: 83a8471ba255 ("net: ethernet: ti: cpsw: refactor probe to group common hw initialization") Reported-by: Maxime Chevallier Signed-off-by: Antoine Tenart --- drivers/net/ethernet/ti/cpsw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 32a89744972d..a46b8b2e44e1 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -2775,6 +2775,7 @@ static int cpsw_probe(struct platform_device *pdev) if (!cpsw) return -ENOMEM; + platform_set_drvdata(pdev, cpsw); cpsw->dev = dev; mode = devm_gpiod_get_array_optional(dev, "mode", GPIOD_OUT_LOW); @@ -2879,7 +2880,6 @@ static int cpsw_probe(struct platform_device *pdev) goto clean_cpts; } - platform_set_drvdata(pdev, cpsw); priv = netdev_priv(ndev); priv->cpsw = cpsw; priv->ndev = ndev; -- 2.21.0
Re: [PATCH net-next v2 6/9] net: macsec: hardware offloading infrastructure
Hi Sabrina, On Tue, Aug 20, 2019 at 04:41:19PM +0200, Sabrina Dubroca wrote: > 2019-08-20, 12:01:40 +0200, Antoine Tenart wrote: > > So it seems the ability to enable or disable the offloading on a given > > interface is the main missing feature. I'll add that, however I'll > > probably (at least at first): > > > > - Have the interface to be fully offloaded or fully handled in s/w (with > > errors being thrown if a given configuration isn't supported). Having > > both at the same time on a given interface would be tricky because of > > the MACsec validation parameter. > > > > - Won't allow to enable/disable the offloading of there are rules in > > place, as we're not sure the same rules would be accepted by the other > > implementation. > > That's probably quite problematic actually, because to do that you > need to be able to resync the state between software and hardware, > particularly packet numbers. So, yeah, we're better off having it > completely blocked until we have a working implementation (if that > ever happens). > > However, that means in case the user wants to set up something that's > not offloadable, they'll have to: > - configure the offloaded version until EOPNOTSUPP > - tear everything down > - restart from scratch without offloading > > That's inconvenient. That's right, the user might have to replay the whole configuration if on rule failed to match the h/w requirements. It's inconvenient, but I think it's better to be safe until we have (if that happens) a working implementation of synchronizing the rules' state. > Talking about packet numbers, can you describe how PN exhaustion is > handled? I couldn't find much about packet numbers at all in the > driver patches (I hope the hw doesn't wrap around from 2^32-1 to 0 on > the same SA). At some point userspace needs to know that we're > getting close to 2^32 and that it's time to re-key. Since the whole > TX path of the software implementation is bypassed, it looks like the > PN (as far as drivers/net/macsec.c is concerned) never increases, so > userspace can't know when to negotiate a new SA. That's a very good point. It actually was on my todo list for the next version (I wanted to discuss the other points first). We would also need to sync the stats at some point. > > I'm not sure if we should allow to mix the implementations on a given > > physical interface (by having two MACsec interfaces attached) as the > > validation would be impossible to do (we would have no idea if a > > packet was correctly handled by the offloading part or just not being > > a MACsec packet in the first place, in Rx). > > That's something that really bothers me with this proposal. Quoting > from the commit message: > > > the packets seen by the networking stack on both the physical and > > MACsec virtual interface are exactly the same That bothers me as well. > If the HW/driver is expected to strip the sectag, I don't see how we > could ever have multiple secy's at the same time and demultiplex > properly between them. That's part of the reason why I chose to have > virtual interfaces: without them, picking the right secy on TX gets > weird. > > AFAICT, it means that any filters (tc, tcpdump) need to be different > between offloaded and non-offloaded cases. > > And it's going to be confusing to the administrator when they look at > tcpdumps expecting to see MACsec frames. Right. I did not see how *not* to strip the sectag in the h/w back then, I'll have another look because that would improve things a lot. @all: do other MACsec offloading hardware allow not to stip the sectag? > I don't see how this implementation handles non-macsec traffic (on TX, > that would be packets sent directly through the real interface, for > example by wpa_supplicant - on RX, incoming MKA traffic for > wpa_supplicant). Unless I missed something, incoming MKA traffic will > end up on the macsec interface as well as the lower interface (not > entirely critical, as long as wpa_supplicant can grab it on the lower > device, but not consistent with the software implementation). That's right, as we have no way to tell if an Rx packet was MACsec or non-MACsec traffic, both will end up on both interfaces. Some h/w may be able to insert a custom header (or may allow not to strip the sectag), but I did not find anything related to this in mine (I'll double check). > How does the driver distinguish traffic that should pass through > unmodified from traffic that the HW needs to encapsulate and encrypt? At least in PHYs, packets go in a classification u
Re: [PATCH net-next v2 5/9] net: phy: add MACsec ops in phy_device
Hi Florian, On Wed, Aug 14, 2019 at 04:15:03PM -0700, Florian Fainelli wrote: > On 8/8/19 7:05 AM, Antoine Tenart wrote: > > This patch adds a reference to MACsec ops in the phy_device, to allow > > PHYs to support offloading MACsec operations. The phydev lock will be > > held while calling those helpers. > > > > Signed-off-by: Antoine Tenart > > --- > > include/linux/phy.h | 10 ++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/include/linux/phy.h b/include/linux/phy.h > > index 462b90b73f93..6947a19587e4 100644 > > --- a/include/linux/phy.h > > +++ b/include/linux/phy.h > > @@ -22,6 +22,10 @@ > > #include > > #include > > > > +#ifdef CONFIG_MACSEC > > +#include > > +#endif > > #if IS_ENABLED(CONFIG_MACSEC) > > > + > > #include > > > > #define PHY_DEFAULT_FEATURES (SUPPORTED_Autoneg | \ > > @@ -345,6 +349,7 @@ struct phy_c45_device_ids { > > * attached_dev: The attached enet driver's device instance ptr > > * adjust_link: Callback for the enet controller to respond to > > * changes in the link state. > > + * macsec_ops: MACsec offloading ops. > > * > > * speed, duplex, pause, supported, advertising, lp_advertising, > > * and autoneg are used like in mii_if_info > > @@ -438,6 +443,11 @@ struct phy_device { > > > > void (*phy_link_change)(struct phy_device *, bool up, bool do_carrier); > > void (*adjust_link)(struct net_device *dev); > > + > > +#if defined(CONFIG_MACSEC) > > + /* MACsec management functions */ > > + const struct macsec_ops *macsec_ops; > > +#endif > > #if IS_ENABLED(CONFIG_MACSEC) > > likewise. I'll fix it. Thanks! Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v2 6/9] net: macsec: hardware offloading infrastructure
Hi Sabrina, On Fri, Aug 16, 2019 at 03:25:00PM +0200, Sabrina Dubroca wrote: > 2019-08-13, 10:58:17 +0200, Antoine Tenart wrote: > > > > As for the need for xmit / handle_frame ops (for a MAC w/ MACsec > > offloading), I'd say the xmit / handle_frame ops of the real net device > > driver could be used as the one of the MACsec virtual interface do not > > do much (regardless of the implementation choice discussed above). > > There's no "handle_frame" op on a real device. macsec_handle_frame is > an rx_handler specificity that grabs packets from a real device and > sends them into a virtual device stacked on top of it. A real device > just hands packets over to the stack via NAPI. Right, so if there is a need for a custom implementation of xmit / hamdle_frame we could let the offloading driver provide it. I'll probably let the first MAC implementation add it, as we agreed not to add an API with no user (and mine is done in the PHY). Thanks! Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com