[DSA] fallback PTP to master port when switch does not support it

[DSA] fallback PTP to master port when switch does not support it

[Matej Zachar]

 Hi,

in my embedded setup I have CPU (master) port with full PTP support connected to the onboard switch (without PTP support) through DSA. As the ioctl and ts_info is passed to the switch driver I made small change to fallback to the master net_device. This however requires that the switch which does not support PTP must not implement .get_ts_info and .port_hwtstamp_get/set from dsa_switch_ops struct.

Do you think this is good approach - I’m happy to work on patch if it makes sense.

I understand that better solution would be to have PTP capable switch, but thats not the situation on this board.

Thank you,
Matej.


diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 65b125bb3b8606e35e5a4a5963c04543266c6114..c78b202e86f3b12d2046de718fd5a1ddcec277cd 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -251,16 +251,25 @@ static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_switch *ds = p->dp->ds;
 	int port = p->dp->index;
+	struct net_device *master = dsa_slave_to_master(dev);
 
 	/* Pass through to switch driver if it supports timestamping */
 	switch (cmd) {
 	case SIOCGHWTSTAMP:
 		if (ds->ops->port_hwtstamp_get)
 			return ds->ops->port_hwtstamp_get(ds, port, ifr);
+
+		if (master->netdev_ops->ndo_do_ioctl)
+			return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
+
 		break;
 	case SIOCSHWTSTAMP:
 		if (ds->ops->port_hwtstamp_set)
 			return ds->ops->port_hwtstamp_set(ds, port, ifr);
+
+		if (master->netdev_ops->ndo_do_ioctl)
+			return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
+
 		break;
 	}
 
@@ -1292,11 +1303,12 @@ static int dsa_slave_get_ts_info(struct net_device *dev,
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_switch *ds = p->dp->ds;
+	struct net_device *master = dsa_slave_to_master(dev);
 
-	if (!ds->ops->get_ts_info)
-		return -EOPNOTSUPP;
+	if (ds->ops->get_ts_info)
+		return ds->ops->get_ts_info(ds, p->dp->index, ts);
 
-	return ds->ops->get_ts_info(ds, p->dp->index, ts);
+	return master->ethtool_ops->get_ts_info(master, ts);
 }
 
 static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,

Re: [DSA] fallback PTP to master port when switch does not support it

[Andrew Lunn]

On Mon, Apr 04, 2022 at 09:28:08PM +0200, Matej Zachar wrote:
>  Hi,
> 
> in my embedded setup I have CPU (master) port with full PTP support
> connected to the onboard switch (without PTP support) through
> DSA. As the ioctl and ts_info is passed to the switch driver I made
> small change to fallback to the master net_device.

Did you try just running PTP on the master device? I'm wondering if
the DSA headers get in the way?

What i don't like about your proposed fallback is that it gives the
impression the slave ports actually support PTP, when they do not. And
maybe you want to run different ports in different modes, one upstream
towards a grand master and one downstream? I suspect the errors you
get are not obvious. Where as if you just run PTP on the master, the
errors would be more obvious.

> This however requires that the switch which does not support PTP
> must not implement .get_ts_info and .port_hwtstamp_get/set from
> dsa_switch_ops struct.

And this is another advantage of just using master directly. You can
even use master when the switch ports do support PTP.

       Andrew

Re: [DSA] fallback PTP to master port when switch does not support it

[Matej Zachar]

Hi Andrew,
thank you for the quick response.

> On 5. 4. 2022, at 0:04, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> 
> Did you try just running PTP on the master device? I'm wondering if
> the DSA headers get in the way?

Yes this is exactly the problem as the master device is used as “conduit interface”
so the switch is dropping the frames as there are no correct DSA headers present.

So running ptp4l on eth0 (master) interface configures the hardware time-stamping correctly but no traffic is received.
running it on lan1 (slave) interface you get the packets but no hardware time-stamping support.

I though this should be in a way similar to the case when you run ptp4l over vlan interfaces (without dsa switch) - it fallbacks to the master port.

> 
> What i don't like about your proposed fallback is that it gives the
> impression the slave ports actually support PTP, when they do not. And
> maybe you want to run different ports in different modes, one upstream
> towards a grand master and one downstream? I suspect the errors you
> get are not obvious. Where as if you just run PTP on the master, the
> errors would be more obvious.
> 

I’m using switch ports in “single port” configuration so there is lan1 lan2 interfaces connected to different network segments.
So it can behave as you described in upstream/downstream configuration as “boundary” PTP clock or as a redundancy where
lan1 & lan2 are connected to physically separate networks - including switches and cables. 

> 
> And this is another advantage of just using master directly. You can
> even use master when the switch ports do support PTP.

I do not see how is that possible as the DSA headers are in the way and packets get dropped by the switch but maybe I am missing something.

My second idea was to check the return values and fallback based on that so the switch driver could still implement
.get_ts_info and .port_hwtstamp_get/set from dsa_switch_ops struct. This was just quick proof to test if it would work over slave interface.

If there is better approach I could explore I’m happy to try.

Matej.

Re: [DSA] fallback PTP to master port when switch does not support it

[Jakub Kicinski]

On Tue, 5 Apr 2022 00:04:30 +0200 Andrew Lunn wrote:
> What i don't like about your proposed fallback is that it gives the
> impression the slave ports actually support PTP, when they do not.

+1, running PTP on the master means there is a non-PTP-aware switch 
in the path, which should not be taken lightly.

Re: [DSA] fallback PTP to master port when switch does not support it

[Vladimir Oltean]

On Tue, Apr 05, 2022 at 12:48:51PM -0700, Jakub Kicinski wrote:
> On Tue, 5 Apr 2022 00:04:30 +0200 Andrew Lunn wrote:
> > What i don't like about your proposed fallback is that it gives the
> > impression the slave ports actually support PTP, when they do not.
>
> +1, running PTP on the master means there is a non-PTP-aware switch
> in the path, which should not be taken lightly.

+2, the change could probably be technically done, and there are aspects
worth discussing, but the goal presented here is questionable and it's
best to not fool ourselves into thinking that the variable queuing delays
of the switch are taken into account when reporting the timestamps,
which they aren't.

I think that by the time you realize that you need PTP hardware
timestamping on switch ports but you have a PTP-unaware switch
integrated *into* your system, you need to go back to the drawing board.

Re: [DSA] fallback PTP to master port when switch does not support it

[Rodolfo Giometti]

On 07/04/22 11:44, Vladimir Oltean wrote:
> On Tue, Apr 05, 2022 at 12:48:51PM -0700, Jakub Kicinski wrote:
>> On Tue, 5 Apr 2022 00:04:30 +0200 Andrew Lunn wrote:
>>> What i don't like about your proposed fallback is that it gives the
>>> impression the slave ports actually support PTP, when they do not.
>>
>> +1, running PTP on the master means there is a non-PTP-aware switch
>> in the path, which should not be taken lightly.
> 
> +2, the change could probably be technically done, and there are aspects
> worth discussing, but the goal presented here is questionable and it's
> best to not fool ourselves into thinking that the variable queuing delays
> of the switch are taken into account when reporting the timestamps,
> which they aren't.
> 
> I think that by the time you realize that you need PTP hardware
> timestamping on switch ports but you have a PTP-unaware switch
> integrated *into* your system, you need to go back to the drawing board.

IMHO this patch is a great hack but what you say sounds good to me.

However we can modify the patch in order to leave the default behavior as-is but 
adding the ability to enable this hack via DTS flag as follow:

                 ports {
                         #address-cells = <1>;
                         #size-cells = <0>;

                         port@0 {
                                 reg = <0>;
                                 label = "lan1";
                                 allow-ptp-fallback;
                         };

                         port@1 {
                                 reg = <1>;
                                 label = "lan2";
                         };

                         ...

                         port@5 {
                                 reg = <5>;
                                 label = "cpu";
                                 ethernet = <&fec>;

                                 fixed-link {
                                         speed = <1000>;
                                         full-duplex;
                                 };
                         };
                 };

Then the code can do as follow:

static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
{
         struct dsa_slave_priv *p = netdev_priv(dev);
         struct dsa_switch *ds = p->dp->ds;
         int port = p->dp->index;
         struct net_device *master = dsa_slave_to_master(dev);

         /* Pass through to switch driver if it supports timestamping */
         switch (cmd) {
         case SIOCGHWTSTAMP:
                 if (ds->ops->port_hwtstamp_get)
                         return ds->ops->port_hwtstamp_get(ds, port, ifr);
                 if (p->dp->allow_ptp_fallback && master->netdev_ops->ndo_do_ioctl)
                         return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
                 break;
         case SIOCSHWTSTAMP:
                 if (ds->ops->port_hwtstamp_set)
                         return ds->ops->port_hwtstamp_set(ds, port, ifr);
                 if (p->dp->allow_ptp_fallback && master->netdev_ops->ndo_do_ioctl)
                         return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
                 break;
         }

         return phylink_mii_ioctl(p->dp->pl, ifr, cmd);
}

In this manner the default behavior is to return error if the switch doesn't 
support the PTP functions, but developers can intentionally enable the PTP 
fallback on specific ports only in order to be able to use PTP on buggy hardware.

Can this solution be acceptable?

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti

Re: [DSA] fallback PTP to master port when switch does not support it

[Vladimir Oltean]

On Wed, May 25, 2022 at 05:00:24PM +0200, Rodolfo Giometti wrote:
> On 07/04/22 11:44, Vladimir Oltean wrote:
> > On Tue, Apr 05, 2022 at 12:48:51PM -0700, Jakub Kicinski wrote:
> > > On Tue, 5 Apr 2022 00:04:30 +0200 Andrew Lunn wrote:
> > > > What i don't like about your proposed fallback is that it gives the
> > > > impression the slave ports actually support PTP, when they do not.
> > > 
> > > +1, running PTP on the master means there is a non-PTP-aware switch
> > > in the path, which should not be taken lightly.
> > 
> > +2, the change could probably be technically done, and there are aspects
> > worth discussing, but the goal presented here is questionable and it's
> > best to not fool ourselves into thinking that the variable queuing delays
> > of the switch are taken into account when reporting the timestamps,
> > which they aren't.
> > 
> > I think that by the time you realize that you need PTP hardware
> > timestamping on switch ports but you have a PTP-unaware switch
> > integrated *into* your system, you need to go back to the drawing board.
> 
> IMHO this patch is a great hack but what you say sounds good to me.

How many Ethernet connections are there between the switch and the host?
One alternative which requires no code changes is to connect one more
switch port and run PTP at your own risk on the attached FEC port
(not DSA master).

What switch driver is it? There are 2 paths to be discussed.
On TX, does the switch forward DSA-untagged packets from the host port? Where to?
On RX, does the switch tag all packets with a DSA header towards the
host? I guess yes, but in that case, be aware that not many Ethernet
controllers can timestamp non-PTP packets. And you need anyway to demote
e.g. HWTSTAMP_FILTER_PTP_V2_EVENT to HWTSTAMP_FILTER_ALL when you pass
the request to the master to account for that, which you are not doing.

> However we can modify the patch in order to leave the default behavior as-is
> but adding the ability to enable this hack via DTS flag as follow:
> 
>                 ports {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         port@0 {
>                                 reg = <0>;
>                                 label = "lan1";
>                                 allow-ptp-fallback;
>                         };
> 
>                         port@1 {
>                                 reg = <1>;
>                                 label = "lan2";
>                         };
> 
>                         ...
> 
>                         port@5 {
>                                 reg = <5>;
>                                 label = "cpu";
>                                 ethernet = <&fec>;
> 
>                                 fixed-link {
>                                         speed = <1000>;
>                                         full-duplex;
>                                 };
>                         };
>                 };
> 
> Then the code can do as follow:
> 
> static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> {
>         struct dsa_slave_priv *p = netdev_priv(dev);
>         struct dsa_switch *ds = p->dp->ds;
>         int port = p->dp->index;
>         struct net_device *master = dsa_slave_to_master(dev);
> 
>         /* Pass through to switch driver if it supports timestamping */
>         switch (cmd) {
>         case SIOCGHWTSTAMP:
>                 if (ds->ops->port_hwtstamp_get)
>                         return ds->ops->port_hwtstamp_get(ds, port, ifr);
>                 if (p->dp->allow_ptp_fallback && master->netdev_ops->ndo_do_ioctl)
>                         return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
>                 break;
>         case SIOCSHWTSTAMP:
>                 if (ds->ops->port_hwtstamp_set)
>                         return ds->ops->port_hwtstamp_set(ds, port, ifr);
>                 if (p->dp->allow_ptp_fallback && master->netdev_ops->ndo_do_ioctl)
>                         return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
>                 break;
>         }
> 
>         return phylink_mii_ioctl(p->dp->pl, ifr, cmd);
> }
> 
> In this manner the default behavior is to return error if the switch doesn't
> support the PTP functions, but developers can intentionally enable the PTP
> fallback on specific ports only in order to be able to use PTP on buggy
> hardware.
> 
> Can this solution be acceptable?

Generally we don't allow policy configuration through the device tree.

Re: [DSA] fallback PTP to master port when switch does not support it

[Rodolfo Giometti]

On 25/05/22 17:55, Vladimir Oltean wrote:
> On Wed, May 25, 2022 at 05:00:24PM +0200, Rodolfo Giometti wrote:
>> On 07/04/22 11:44, Vladimir Oltean wrote:
>>> On Tue, Apr 05, 2022 at 12:48:51PM -0700, Jakub Kicinski wrote:
>>>> On Tue, 5 Apr 2022 00:04:30 +0200 Andrew Lunn wrote:
>>>>> What i don't like about your proposed fallback is that it gives the
>>>>> impression the slave ports actually support PTP, when they do not.
>>>>
>>>> +1, running PTP on the master means there is a non-PTP-aware switch
>>>> in the path, which should not be taken lightly.
>>>
>>> +2, the change could probably be technically done, and there are aspects
>>> worth discussing, but the goal presented here is questionable and it's
>>> best to not fool ourselves into thinking that the variable queuing delays
>>> of the switch are taken into account when reporting the timestamps,
>>> which they aren't.
>>>
>>> I think that by the time you realize that you need PTP hardware
>>> timestamping on switch ports but you have a PTP-unaware switch
>>> integrated *into* your system, you need to go back to the drawing board.
>>
>> IMHO this patch is a great hack but what you say sounds good to me.
> 
> How many Ethernet connections are there between the switch and the host?

It depends how the hardware is designed. However usually the host has an 
ethernet connected to a switch's port named CPU port, so just one.

> One alternative which requires no code changes is to connect one more
> switch port and run PTP at your own risk on the attached FEC port
> (not DSA master).

I see, but here we are talking about of not-so-well designed boards :( whose 
have a switch that can't manage PTP and we still need to have a sort of time 
sync. The trick can be to forward PTP packets to the host's ethernet (and 
viceversa).

> What switch driver is it? There are 2 paths to be discussed.
> On TX, does the switch forward DSA-untagged packets from the host port? Where to?
> On RX, does the switch tag all packets with a DSA header towards the
> host? I guess yes,

Of course it's in charge of the DSA to properly setup the switch in order to 
abstract all switch's ports as host's ethernet ports, so all packets go where 
they have to go.

> but in that case, be aware that not many Ethernet
> controllers can timestamp non-PTP packets. And you need anyway to demote
> e.g. HWTSTAMP_FILTER_PTP_V2_EVENT to HWTSTAMP_FILTER_ALL when you pass
> the request to the master to account for that, which you are not doing.

Mmm... I can't see problems here... can you please explain it?

>> However we can modify the patch in order to leave the default behavior as-is
>> but adding the ability to enable this hack via DTS flag as follow:
>>
>>                  ports {
>>                          #address-cells = <1>;
>>                          #size-cells = <0>;
>>
>>                          port@0 {
>>                                  reg = <0>;
>>                                  label = "lan1";
>>                                  allow-ptp-fallback;
>>                          };
>>
>>                          port@1 {
>>                                  reg = <1>;
>>                                  label = "lan2";
>>                          };
>>
>>                          ...
>>
>>                          port@5 {
>>                                  reg = <5>;
>>                                  label = "cpu";
>>                                  ethernet = <&fec>;
>>
>>                                  fixed-link {
>>                                          speed = <1000>;
>>                                          full-duplex;
>>                                  };
>>                          };
>>                  };
>>
>> Then the code can do as follow:
>>
>> static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>> {
>>          struct dsa_slave_priv *p = netdev_priv(dev);
>>          struct dsa_switch *ds = p->dp->ds;
>>          int port = p->dp->index;
>>          struct net_device *master = dsa_slave_to_master(dev);
>>
>>          /* Pass through to switch driver if it supports timestamping */
>>          switch (cmd) {
>>          case SIOCGHWTSTAMP:
>>                  if (ds->ops->port_hwtstamp_get)
>>                          return ds->ops->port_hwtstamp_get(ds, port, ifr);
>>                  if (p->dp->allow_ptp_fallback && master->netdev_ops->ndo_do_ioctl)
>>                          return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
>>                  break;
>>          case SIOCSHWTSTAMP:
>>                  if (ds->ops->port_hwtstamp_set)
>>                          return ds->ops->port_hwtstamp_set(ds, port, ifr);
>>                  if (p->dp->allow_ptp_fallback && master->netdev_ops->ndo_do_ioctl)
>>                          return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
>>                  break;
>>          }
>>
>>          return phylink_mii_ioctl(p->dp->pl, ifr, cmd);
>> }
>>
>> In this manner the default behavior is to return error if the switch doesn't
>> support the PTP functions, but developers can intentionally enable the PTP
>> fallback on specific ports only in order to be able to use PTP on buggy
>> hardware.
>>
>> Can this solution be acceptable?
> 
> Generally we don't allow policy configuration through the device tree.

I agree, but here we have to signal an hardware that can't do something and 
(IMHO) the device tree is a good point to address it. :)

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti

Re: [DSA] fallback PTP to master port when switch does not support it

[Vladimir Oltean]

On Thu, May 26, 2022 at 09:45:39AM +0200, Rodolfo Giometti wrote:
> On 25/05/22 17:55, Vladimir Oltean wrote:
> > On Wed, May 25, 2022 at 05:00:24PM +0200, Rodolfo Giometti wrote:
> > > On 07/04/22 11:44, Vladimir Oltean wrote:
> > > > On Tue, Apr 05, 2022 at 12:48:51PM -0700, Jakub Kicinski wrote:
> > > > > On Tue, 5 Apr 2022 00:04:30 +0200 Andrew Lunn wrote:
> > > > > > What i don't like about your proposed fallback is that it gives the
> > > > > > impression the slave ports actually support PTP, when they do not.
> > > > > 
> > > > > +1, running PTP on the master means there is a non-PTP-aware switch
> > > > > in the path, which should not be taken lightly.
> > > > 
> > > > +2, the change could probably be technically done, and there are aspects
> > > > worth discussing, but the goal presented here is questionable and it's
> > > > best to not fool ourselves into thinking that the variable queuing delays
> > > > of the switch are taken into account when reporting the timestamps,
> > > > which they aren't.
> > > > 
> > > > I think that by the time you realize that you need PTP hardware
> > > > timestamping on switch ports but you have a PTP-unaware switch
> > > > integrated *into* your system, you need to go back to the drawing board.
> > > 
> > > IMHO this patch is a great hack but what you say sounds good to me.
> > 
> > How many Ethernet connections are there between the switch and the host?
> 
> It depends how the hardware is designed. However usually the host has an
> ethernet connected to a switch's port named CPU port, so just one.
> 
> > One alternative which requires no code changes is to connect one more
> > switch port and run PTP at your own risk on the attached FEC port
> > (not DSA master).
> 
> I see, but here we are talking about of not-so-well designed boards :( whose
> have a switch that can't manage PTP and we still need to have a sort of time
> sync. The trick can be to forward PTP packets to the host's ethernet (and
> viceversa).
> 
> > What switch driver is it? There are 2 paths to be discussed.
> > On TX, does the switch forward DSA-untagged packets from the host port? Where to?
> > On RX, does the switch tag all packets with a DSA header towards the
> > host? I guess yes,
> 
> Of course it's in charge of the DSA to properly setup the switch in order to
> abstract all switch's ports as host's ethernet ports, so all packets go
> where they have to go.

It's a trick alright, but whom is it tricking? The user sees that netdev
X has a PHC and reports hardware timestamping, so he has no way of
knowing that packets aren't timestamped at the point where the MAC of
said netdev puts the frame's SFD on the wire. There's a reason why
hardware timestamping is taken at that point and not earlier.
Run an iperf3 at the same time as your PTP timestamping and you'll see why, too.
Or a shaper on the switch port.

Is hardware timestamping on the master better than no hardware
timestamping at all? Probably, maybe, sometimes. Does it work except
when it's sunny outside? No. Is it a lie to present a braindead solution
to the user in the exact same way as a proper solution would look? Yes,
no one will look at the device tree and say "oh, yes, it has the
allow-ptp-fallback property, I'm well aware that this is a hack".

My point is that not all hacks are meant to be part of the mainline
kernel, _especially_ if they are disingenuous.

> > but in that case, be aware that not many Ethernet
> > controllers can timestamp non-PTP packets. And you need anyway to demote
> > e.g. HWTSTAMP_FILTER_PTP_V2_EVENT to HWTSTAMP_FILTER_ALL when you pass
> > the request to the master to account for that, which you are not doing.
> 
> Mmm... I can't see problems here... can you please explain it?

HWTSTAMP_FILTER_PTP_V2_EVENT means "please timestamp PTP event packets,
L2 and L4". DSA-tagged PTP packets are not PTP packets as far as the
master is concerned, because the DSA header obfuscates the real
encapsulated protocol. So when you forward the request to the master,
you need to tell it to timestamp everything (and it may not be able to
do that).

By the way there's another problem with this approach. Your PTP packets
probably reach the DSA master by flooding. If the switch is going to
participate in the PTP network, it means that SYNC packets coming from
an external station A acting as GM will be flooded not only to the DSA
master, but also to the other stations B. But ptp4l running on the local
DSA switch as a boundary clock also generates SYNC packets of its own on
the ports that are in the MASTER state.
By flooding the SYNC packets of A, station B effectively sees SYNC
packets from multiple masters and gets confused about whom it should track.

The expected behavior is to install trap-to-CPU rules for PTP packets
when timestamping is enabled on a switch port. See commit 96ca08c05838
("net: mscc: ocelot: set up traps for PTP packets") for example.
You are not doing that, because you just forward the timestamping ioctls
to the master. So PTP support will be very buggy.

> > > However we can modify the patch in order to leave the default behavior as-is
> > > but adding the ability to enable this hack via DTS flag as follow:
> > > 
> > >                  ports {
> > >                          #address-cells = <1>;
> > >                          #size-cells = <0>;
> > > 
> > >                          port@0 {
> > >                                  reg = <0>;
> > >                                  label = "lan1";
> > >                                  allow-ptp-fallback;
> > >                          };
> > > 
> > >                          port@1 {
> > >                                  reg = <1>;
> > >                                  label = "lan2";
> > >                          };
> > > 
> > >                          ...
> > > 
> > >                          port@5 {
> > >                                  reg = <5>;
> > >                                  label = "cpu";
> > >                                  ethernet = <&fec>;
> > > 
> > >                                  fixed-link {
> > >                                          speed = <1000>;
> > >                                          full-duplex;
> > >                                  };
> > >                          };
> > >                  };
> > > 
> > > Then the code can do as follow:
> > > 
> > > static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> > > {
> > >          struct dsa_slave_priv *p = netdev_priv(dev);
> > >          struct dsa_switch *ds = p->dp->ds;
> > >          int port = p->dp->index;
> > >          struct net_device *master = dsa_slave_to_master(dev);
> > > 
> > >          /* Pass through to switch driver if it supports timestamping */
> > >          switch (cmd) {
> > >          case SIOCGHWTSTAMP:
> > >                  if (ds->ops->port_hwtstamp_get)
> > >                          return ds->ops->port_hwtstamp_get(ds, port, ifr);
> > >                  if (p->dp->allow_ptp_fallback && master->netdev_ops->ndo_do_ioctl)
> > >                          return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
> > >                  break;
> > >          case SIOCSHWTSTAMP:
> > >                  if (ds->ops->port_hwtstamp_set)
> > >                          return ds->ops->port_hwtstamp_set(ds, port, ifr);
> > >                  if (p->dp->allow_ptp_fallback && master->netdev_ops->ndo_do_ioctl)
> > >                          return master->netdev_ops->ndo_do_ioctl(master, ifr, cmd);
> > >                  break;
> > >          }
> > > 
> > >          return phylink_mii_ioctl(p->dp->pl, ifr, cmd);
> > > }
> > > 
> > > In this manner the default behavior is to return error if the switch doesn't
> > > support the PTP functions, but developers can intentionally enable the PTP
> > > fallback on specific ports only in order to be able to use PTP on buggy
> > > hardware.
> > > 
> > > Can this solution be acceptable?
> > 
> > Generally we don't allow policy configuration through the device tree.
> 
> I agree, but here we have to signal an hardware that can't do something and
> (IMHO) the device tree is a good point to address it. :)

You already know the driver can't do it because it doesn't implement the
timestamping ioctls. In fact you didn't answer my previous question of
what hardware this is. I'm curious if PTP support is really absent.

Re: [DSA] fallback PTP to master port when switch does not support it

[Andrew Lunn]

> However we can modify the patch in order to leave the default behavior as-is
> but adding the ability to enable this hack via DTS flag as follow:

The fact you called it a hack suggests you know it is not likely to be
accepted.

> 
>                 ports {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         port@0 {
>                                 reg = <0>;
>                                 label = "lan1";
>                                 allow-ptp-fallback;
>                         };

As Vladimir said, this is configuration, not a description of the
hardware.

	Andrew