On Mon, Nov 16, 2020 at 10:21:14AM +0100, Christian Eggers wrote:

On Friday, 13 November 2020, 17:56:34 CET, Christian Eggers wrote:

quoted

On Friday, 13 November 2020, 00:02:54 CET, Vladimir Oltean wrote:

quoted

On Thu, Nov 12, 2020 at 04:35:29PM +0100, Christian Eggers wrote:

quoted

Parts of ksz_common.h (struct ksz_device) will be required in
net/dsa/tag_ksz.c soon. So move the relevant parts into a new header
file.

Signed-off-by: Christian Eggers <ceggers@arri.de>
---

I had to skip ahead to see what you're going to use struct ksz_port and

struct ksz_device for. It looks like you need:
    struct ksz_port::tstamp_rx_latency_ns
    struct ksz_device::ptp_clock_lock
    struct ksz_device::ptp_clock_time

Not more.

I have tried to put these members into separate structs:

include/linux/dsa/ksz_common.h:
struct ksz_port_ptp_shared {
        u16 tstamp_rx_latency_ns;   /* rx delay from wire to tstamp unit */
};

struct ksz_device_ptp_shared {
        spinlock_t ptp_clock_lock; /* for ptp_clock_time */
        /* approximated current time, read once per second from hardware */
        struct timespec64 ptp_clock_time;
};

drivers/net/dsa/microchip/ksz_common.h:
...
#include <linux/dsa/ksz_common.h>
...
struct ksz_port {
...
#if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ9477_PTP)
        struct ksz_port_ptp_shared ptp_shared;  /* shared with tag_ksz.c */
        u16 tstamp_tx_latency_ns;       /* tx delay from tstamp unit to wire */
        struct hwtstamp_config tstamp_config;
        struct sk_buff *tstamp_tx_xdelay_skb;
        unsigned long tstamp_state;
#endif
};
...
struct ksz_device {
...
#if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ9477_PTP)
        struct ptp_clock *ptp_clock;
        struct ptp_clock_info ptp_caps;
        struct mutex ptp_mutex;
        struct ksz_device_ptp_shared ptp_shared;   /* shared with tag_ksz.c */
#endif
};

The problem with such technique is, that I still need to dereference
struct ksz_device in tag_ksz.c:

static void ksz9477_rcv_timestamp(struct sk_buff *skb, u8 *tag,
                                  struct net_device *dev, unsigned int port)
{
...
        struct dsa_switch *ds = dev->dsa_ptr->ds;
        struct ksz_device *ksz = ds->priv;
        struct ksz_port *prt = &ksz->ports[port];
...
}

As struct dsa_switch::priv is already occupied by the pointer to
struct ksz_device, I see no way accessing the ptp specific device/port
information in tag_ksz.c.

There is a dp->priv that you could use to hold a reference to your
PTP-specific substructure (struct ksz_port_ptp_shared) of
struct ksz_port.

Then, in that PTP-specific per-port substructure, you could hold another
pointer to a common struct ksz_device_ptp_shared.