quoted

Hi Mattias

Vladimer pointed you towards the qca driver, in a comment for your
RFC. qca already has support for switch commands via Ethernet frames.

The point he was trying to make is that you should look at that
code. The concept of executing a command via an Ethernet frame, and
expecting a reply via an Ethernet frame is generic. The format of
those frames is specific to the switch. We want the generic parts to
look the same for all switches. If possible, we want to implement it
once in the dsa core, so all switch drivers share it. Less code,
better tested code, less bugs, easier maintenance.

Take a look at qca_tagger_data. Please use the same mechanism with

This I can do which makes sense.

quoted

mv88e6xxx. But also look deeper. What else can be shared? You need a

I can also make a generic dsa_eth_tx_timeout routine which
handles the sending and receiving of cmd frames.

quoted

buffer to put the request in, you need to send it, you need to wait

The skb is the buffer and it's up to the driver to decode it properly?

Yes, the tagger layer passes the skb, which contains the complete
Ethernet frame, plus additional meta data. But you need to watch out
for the life cycle of that skb:

        /* Ethernet mgmt read/write packet */
        if (pk_type == QCA_HDR_RECV_TYPE_RW_REG_ACK) {
                if (likely(tagger_data->rw_reg_ack_handler))
                        tagger_data->rw_reg_ack_handler(ds, skb);
                return NULL;
        }

returning NULL means the DSA core will free the skb when the call to
qca_tag_rcv() returns. So either you need to take a copy of the data,
clone the skb, or change this code somehow. See what makes most sense
for generic code.

I've looked into the qca driver and that uses a small static buffer
for replies, no buffer lifetime cycle.

quoted

for the reply, you need to pass the results to the driver, you need to
free that buffer afterwards. That should all be common. Look at these
parts in the qca driver. Can you make them generic, move them into the
DSA core? Are there other parts which could be shared?

I cannot change the qca code as I have no way of verifying that the
resulting code works.

You can change it. You can at least compile test your change. The QCA
developers are also quite active, and so will probably help out
testing whatever you change. And ideally, you want lots of simple,
obviously correct changes, which i can review and say look correct.

Changing code which you cannot test is very normal in Linux, do your
best, and ask for testing.

      Andrew