Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
On Tue, Nov 07, 2017 at 07:23:27PM -0800, Richard Cochran wrote: > The application does join that group on the external (slave) > interface. I'll find out why the delay request mechanism isn't > working... Looking back, I now recall that the series lets the HW embed the time stamps into the protocol buffers. In the case of UDP, this invalidates the checksum unless the HW corrects it. I'll bet that is what is happening... Thanks, Richard
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
On Wed, Nov 08, 2017 at 04:02:26AM +0100, Andrew Lunn wrote: > So i did a quick test. If the application joins 224.0.1.129 on the > slave interface, the switch will pass the packets to the host and to > the application. The application does join that group on the external (slave) interface. I'll find out why the delay request mechanism isn't working... Thanks, Richard
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
> One thing that we're not doing (and probably should be) is > configuring multicast frames to 01:1B:19:00:00:00 to be destined to > the CPU port. So i did a quick test. If the application joins 224.0.1.129 on the slave interface, the switch will pass the packets to the host and to the application. Andrew
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
On Tue, Nov 07, 2017 at 08:56:05PM +, Brandon Streiff wrote: > > Oops, I had "slaveOnly" set in my PC's configuration. So layer2 seems > > to work as expected. > > > > Have you tested UDPv4? It doesn't work. > > I have not. Our usage has been focused on 802.1AS; the ptp4l settings we > use are the following: > > transportSpecific0x1 > ptp_dst_mac 01:80:C2:00:00:0E > p2p_dst_mac 01:80:C2:00:00:0E > network_transportL2 > delay_mechanism P2P > time_stampinghardware > > One thing that we're not doing (and probably should be) is configuring > multicast frames to 01:1B:19:00:00:00 to be destined to the CPU port. > (01:80:C2:00:00:0E is used for management, so the *_mgmt_rsvd2cpu() > functions give us that "for free".) That might be necessary to make 1588 > L2 work properly. I don't know if that would affect 1588 L4, or if > there's anything else missing to make L4 timestamping work from the HW > perspective. Is the application performing a join on the group? If so, on which interface? I've not tested many multicast applications with DSA. It is possible we have bugs. Andrew
RE: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
> Oops, I had "slaveOnly" set in my PC's configuration. So layer2 seems > to work as expected. > > Have you tested UDPv4? It doesn't work. I have not. Our usage has been focused on 802.1AS; the ptp4l settings we use are the following: transportSpecific0x1 ptp_dst_mac 01:80:C2:00:00:0E p2p_dst_mac 01:80:C2:00:00:0E network_transportL2 delay_mechanism P2P time_stampinghardware One thing that we're not doing (and probably should be) is configuring multicast frames to 01:1B:19:00:00:00 to be destined to the CPU port. (01:80:C2:00:00:0E is used for management, so the *_mgmt_rsvd2cpu() functions give us that "for free".) That might be necessary to make 1588 L2 work properly. I don't know if that would affect 1588 L4, or if there's anything else missing to make L4 timestamping work from the HW perspective. -- brandon
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
On Mon, Nov 06, 2017 at 04:04:22PM +0100, Andrew Lunn wrote: > I assume you have tested basic networking? You can ping the other > machines in the network? Yes, I ssh into the device via the external switch port interface. Thanks, Richard
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
On Mon, Nov 06, 2017 at 06:55:46AM -0800, Richard Cochran wrote: > When I run with Layer2 transport and the switch as master, it seems to > work. Any other combination of role + transport fails. Oops, I had "slaveOnly" set in my PC's configuration. So layer2 seems to work as expected. Have you tested UDPv4? It doesn't work. Thanks, Richard
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
On Mon, Nov 06, 2017 at 06:55:46AM -0800, Richard Cochran wrote: > On Sun, Oct 08, 2017 at 11:38:21AM -0400, Richard Cochran wrote: > > I will try to get my hands on some HW, perhaps by the end of October, > > in order to test and complete your driver... > > I now have a 88E6352 to test your series on. Unfortunately, it > doesn't really work. Here is what I did. > > 1. Gave one of the external switch ports an address (ifconfig ext0 >192.168.1.111) Hi Richard I assume you have tested basic networking? You can ping the other machines in the network? With DSA, users sometimes forget to set the DSA master interface up. Then nothing works. Andrew
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
On Sun, Oct 08, 2017 at 11:38:21AM -0400, Richard Cochran wrote: > I will try to get my hands on some HW, perhaps by the end of October, > in order to test and complete your driver... I now have a 88E6352 to test your series on. Unfortunately, it doesn't really work. Here is what I did. 1. Gave one of the external switch ports an address (ifconfig ext0 192.168.1.111) 2. Ran ptp4l with option 'free_running 1'. When I run with Layer2 transport and the switch as master, it seems to work. Any other combination of role + transport fails. | Switch Role | Tranport | Result | |-+--+| | master | UDPv4| no Delay_Resp appear at slave | | master | UDPv6| no Delay_Resp appear at slave | | master | layer2 | seems okay | | slave | layer2 | Announce messages not getting through to host? | Have you tested any of this? Do I need some special switch configuration first? Thanks, Richard
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
On Fri, Sep 29, 2017 at 05:43:23AM -0400, Richard Cochran wrote: > I happy to see this series. I just finished porting an out-of-tree > PHC driver for the Marvell mv88e635x, and I want to mainline it, but I > also have a few uglies. This series looks really good. I won't even post my mine, as that would now be too embarrassing. I will try to get my hands on some HW, perhaps by the end of October, in order to test and complete your driver... Thanks, Richard
RE: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Thursday, September 28, 2017 12:36 PM
>
> I assume ptp already has the core code to use pinctrl and Linux
> standard GPIOs? What does the device tree binding look like? How do
> you specify the GPIOs to use?
>
> What we want to avoid is defining an ABI now, otherwise it is going to
> be hard to swap to pinctrl later.
A ptp_clock_info has an array of struct ptp_pin_desc which defines "pins" with
a name ("Hardware specific human readable pin name"), an index, and a bitmask
of valid functions. The ptp_pin_desc structure is shared with usermode for the
PTP_PIN_GETFUNC and PTP_PIN_SETFUNC ioctls. The pins are also exposed in sysfs
(see Documentation/ABI/testing/sysfs-ptp). The underlying implementation for
configuring the hardware is left up to the PHC driver. I don't see any drivers
today that use the PHC pin API as a layer over pinctrl/gpiochip, but there's no
reason that that couldn't be the case.
For mv88e6xxx, we name the pins using the pattern "mv88e6xxx_gpio%d"; this
appears to be in line with current practice (igb_ptp.c uses "SDP%d", mlx5
driver uses "mlx5_pps%d"). Usermode code appears to be expected to determine
which pin it needs to use. (Our current userspace code, for instance, knows
that it needs to find "mv88e6xxx_gpio8".)
For mv88e6xxx, Device Tree does feel like a better option here for declaring
names, functions, and pin usages. Not all platforms that use the PTP API use
Device Tree though.
-- brandon
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
Brandon, On Thu, Sep 28, 2017 at 10:25:32AM -0500, Brandon Streiff wrote: > - Patch #2: We expose the switch time as a PTP clock but don't support > adjustment (max_adj=0). The driver should implement a cyclecounter/timecounter. > Our platform adjusted a systemwide oscillator > from userspace, so we didn't need adjustment at this layer, but other > PTP clock drivers support this and we probably should too. We don't currently have any way to support this kind of HW or anything like an external VCO. I would like to find a way to do this, but that is a different kettle of fish as it might require changes in the PHC subsystem. For this driver, I think we should get it merged using the cyclecounter/timecounter (as that will benefit lots of users) and worry about the external oscillator later. > Feedback is appreciated. I happy to see this series. I just finished porting an out-of-tree PHC driver for the Marvell mv88e635x, and I want to mainline it, but I also have a few uglies. Unfortunately I am in the middle of a move right now, and so my review of this series might have to wait a bit. However, I am looking forward to comparing notes, and then getting this support in. Thanks, Richard
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
On 09/28/2017 10:36 AM, Andrew Lunn wrote: >> - Patch #3: The GPIO config support is handled in a very simple manner. >> I suspect a longer term goal would be to use pinctrl here. > > I assume ptp already has the core code to use pinctrl and Linux > standard GPIOs? What does the device tree binding look like? How do > you specify the GPIOs to use? > > What we want to avoid is defining an ABI now, otherwise it is going to > be hard to swap to pinctrl later. > >> - Patch #6: the dsa_switch pointer and port index is plumbed from >> dsa_device_ops::rcv so that we can call the correct port_rxtstamp >> method. This involved instrumenting all of the *_tag_rcv functions in >> a way that's kind of a kludge and that I'm not terribly happy with. > > Yes, this is ugly. I will see if i can find a better way to do > this. See my reply in patch 6, I may be missing something, but once dst->rdcv() has been called, skb->dev points to the slave network device which already contains the switch port and switch information in dsa_slave_priv, so that should lift the need for asking the individual taggers' rcv() callback to tell us about it. -- Florian
Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
> - Patch #3: The GPIO config support is handled in a very simple manner. > I suspect a longer term goal would be to use pinctrl here. I assume ptp already has the core code to use pinctrl and Linux standard GPIOs? What does the device tree binding look like? How do you specify the GPIOs to use? What we want to avoid is defining an ABI now, otherwise it is going to be hard to swap to pinctrl later. > - Patch #6: the dsa_switch pointer and port index is plumbed from > dsa_device_ops::rcv so that we can call the correct port_rxtstamp > method. This involved instrumenting all of the *_tag_rcv functions in > a way that's kind of a kludge and that I'm not terribly happy with. Yes, this is ugly. I will see if i can find a better way to do this. Andrew
[PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
This patch series adds support for PTP timestamping through the DSA
framework, as well as an implementation for mv8e6xxx switches.
This implementation was targeted at a National Instruments platform
that uses the Marvell 88E6341 (Topaz). I've tried to enable support on
other Marvell switches where the register interfaces seemed compatible,
but I don't have the hardware to verify their operation myself.
This series probably ties in well with Richard's comment last week
("Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for
traffic shapers") about figuring out proper interfaces for managing
switch-level PTP timestamps.
A couple patches that I expect may need further polishing:
- Patch #2: We expose the switch time as a PTP clock but don't support
adjustment (max_adj=0). Our platform adjusted a systemwide oscillator
from userspace, so we didn't need adjustment at this layer, but other
PTP clock drivers support this and we probably should too.
- Patch #3: The GPIO config support is handled in a very simple manner.
I suspect a longer term goal would be to use pinctrl here.
- Patch #6: the dsa_switch pointer and port index is plumbed from
dsa_device_ops::rcv so that we can call the correct port_rxtstamp
method. This involved instrumenting all of the *_tag_rcv functions in
a way that's kind of a kludge and that I'm not terribly happy with.
This applies to net-next as of 14a0d032f4ec.
Feedback is appreciated.
-- brandon
Brandon Streiff (9):
net: dsa: mv88e6xxx: add accessors for PTP/TAI registers
net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock
net: dsa: mv88e6xxx: add support for GPIO configuration
net: dsa: mv88e6xxx: add support for event capture
net: dsa: forward hardware timestamping ioctls to switch driver
net: dsa: forward timestamping callbacks to switch drivers
ptp: add offset for reserved field to header
net: dsa: mv88e6xxx: add rx/tx timestamping support
net: dsa: mv88e6xxx: add workaround for 6341 timestamping
drivers/net/dsa/mv88e6xxx/Kconfig| 10 +
drivers/net/dsa/mv88e6xxx/Makefile | 2 +
drivers/net/dsa/mv88e6xxx/chip.c | 65 +
drivers/net/dsa/mv88e6xxx/chip.h | 71 +
drivers/net/dsa/mv88e6xxx/global2.c | 244
drivers/net/dsa/mv88e6xxx/global2.h | 59 +++-
drivers/net/dsa/mv88e6xxx/hwtstamp.c | 548 +++
drivers/net/dsa/mv88e6xxx/hwtstamp.h | 171 +++
drivers/net/dsa/mv88e6xxx/ptp.c | 493 +++
drivers/net/dsa/mv88e6xxx/ptp.h | 99 +++
include/linux/ptp_classify.h | 1 +
include/net/dsa.h| 28 +-
net/dsa/dsa.c| 39 ++-
net/dsa/slave.c | 67 -
net/dsa/tag_brcm.c | 6 +-
net/dsa/tag_dsa.c| 6 +-
net/dsa/tag_edsa.c | 6 +-
net/dsa/tag_ksz.c| 6 +-
net/dsa/tag_lan9303.c| 6 +-
net/dsa/tag_mtk.c| 6 +-
net/dsa/tag_qca.c| 6 +-
net/dsa/tag_trailer.c| 6 +-
22 files changed, 1929 insertions(+), 16 deletions(-)
create mode 100644 drivers/net/dsa/mv88e6xxx/hwtstamp.c
create mode 100644 drivers/net/dsa/mv88e6xxx/hwtstamp.h
create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.c
create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.h
--
2.1.4
