Re: [PATCH 4/7] net: dsa: b53: Add PHC clock support
From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2021-11-06 02:33:00
Also in:
lkml
On 11/4/2021 6:31 AM, Martin Kaistra wrote:
The BCM53128 switch has an internal clock, which can be used for timestamping. Add support for it. The 32-bit free running clock counts nanoseconds. In order to account for the wrap-around at 999999999 (0x3B9AC9FF) while using the cycle counter infrastructure, we need to set a 30bit mask and use the overflow_point property. Enable the Broadsync HD timestamping feature in b53_ptp_init() for PTPv2 Ethertype (0x88f7). Signed-off-by: Martin Kaistra <redacted> ---
[snip]
+int b53_ptp_init(struct b53_device *dev)
+{
+ mutex_init(&dev->ptp_mutex);
+
+ INIT_DELAYED_WORK(&dev->overflow_work, b53_ptp_overflow_check);
+
+ /* Enable BroadSync HD for all ports */
+ b53_write16(dev, B53_BROADSYNC_PAGE, B53_BROADSYNC_EN_CTRL1, 0x00ff);Can you do this for all enabled user ports instead of each port, that way it is clera that this register is supposed to be a bitmask of ports for which you desire PTP timestamping to be enabled?
+ + /* Enable BroadSync HD Time Stamping Reporting (Egress) */ + b53_write8(dev, B53_BROADSYNC_PAGE, B53_BROADSYNC_TS_REPORT_CTRL, 0x01);
Can you add a define for this bit in b53_regs.h and name it: #define TSRPT_PKT_EN BIT(0) which will enable timestamp reporting towards the IMP port.
+ + /* Enable BroadSync HD Time Stamping for PTPv2 ingress */ + + /* MPORT_CTRL0 | MPORT0_TS_EN */ + b53_write16(dev, B53_ARLCTRL_PAGE, 0x0e, (1 << 15) | 0x01);
Please add a definition for 0x0e which is the multi-port control register and is 16-bit wide. Bit 15 is MPORT0_TS_EN and it will ensure that packets matching multiport 0 (address or ethertype) will be timestamped. and then add a macro or generic definitions that are applicable to all multiport control registers, something like: #define MPORT_CTRL_DIS_FORWARD 0 #define MPORT_CTRL_CMP_ADDR 1 #define MPORT_CTRL_CMP_ETYPE 2 #define MPORT_CTRL_CMP_ADDR_ETYPE 3 #define MPORT_CTRL_SHIFT(x) ((x) << 2) #define MPORT_CTRL_MASK 0x3
+ /* Forward to IMP port 8 */ + b53_write64(dev, B53_ARLCTRL_PAGE, 0x18, (1 << 8));
0x18 is the multiport vector N register so we would want a macro to define the multiprot vector being used (up to 6 of them), and this is a 32-bit register, not a 64-bit register. The 8 here should be checked against the actual CPU port index number, it is 8 for you, it could be 5 for someone else, or 7, even.
+ /* PTPv2 Ether Type */ + b53_write64(dev, B53_ARLCTRL_PAGE, 0x10, (u64)0x88f7 << 48);
Use ETH_P_1588 here and 0x10 deserves a define which is the multiport address N register. Likewise, we need a base offset of 0x10 and then a macro to address the 6 multiports that exists.
+ + /* Setup PTP clock */ + dev->ptp_clock_info.owner = THIS_MODULE; + snprintf(dev->ptp_clock_info.name, sizeof(dev->ptp_clock_info.name), + dev_name(dev->dev)); + + dev->ptp_clock_info.max_adj = 1000000000ULL; + dev->ptp_clock_info.n_alarm = 0; + dev->ptp_clock_info.n_pins = 0; + dev->ptp_clock_info.n_ext_ts = 0; + dev->ptp_clock_info.n_per_out = 0; + dev->ptp_clock_info.pps = 0;
memset the structure ahead of time so you only need explicit initialization where needed?
+ dev->ptp_clock_info.adjfine = b53_ptp_adjfine; + dev->ptp_clock_info.adjtime = b53_ptp_adjtime; + dev->ptp_clock_info.gettime64 = b53_ptp_gettime; + dev->ptp_clock_info.settime64 = b53_ptp_settime; + dev->ptp_clock_info.enable = b53_ptp_enable; + + dev->ptp_clock = ptp_clock_register(&dev->ptp_clock_info, dev->dev); + if (IS_ERR(dev->ptp_clock)) + return PTR_ERR(dev->ptp_clock); + + /* The switch provides a 32 bit free running counter. Use the Linux + * cycle counter infrastructure which is suited for such scenarios. + */ + dev->cc.read = b53_ptp_read; + dev->cc.mask = CYCLECOUNTER_MASK(30); + dev->cc.overflow_point = 999999999; + dev->cc.mult = (1 << 28); + dev->cc.shift = 28; + + b53_write32(dev, B53_BROADSYNC_PAGE, B53_BROADSYNC_TIMEBASE_ADJ1, 40);
You are writing the default value of the register, is that of any use? -- Florian