On Thu, Sep 07, 2017 at 09:09:30PM +0000, tristram...@microchip.com wrote:
> From: Tristram Ha <tristram...@microchip.com>
>
> The KSZ tail tag code can be used by different KSZ switch drivers.
>
> Signed-off-by: Tristram Ha <tristram...@microchip.com>
> ---
> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c index 010ca0a..d5faf14
> 100644
> --- a/net/dsa/tag_ksz.c
> +++ b/net/dsa/tag_ksz.c
> @@ -13,35 +13,21 @@
> #include <linux/slab.h>
> #include <net/dsa.h>
> #include "dsa_priv.h"
> -
> -/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
> - *
> ---------------------------------------------------------------------------
> - *
> DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
> - *
> ---------------------------------------------------------------------------
> - * tag0 : Prioritization (not used now)
> - * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
> - *
> - * For Egress (KSZ -> Host), 1 byte is added before FCS.
> - *
> ---------------------------------------------------------------------------
> - * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
> - *
> ---------------------------------------------------------------------------
> - * tag0 : zero-based value represents port
> - * (eg, 0x00=port1, 0x02=port3, 0x06=port7)
> - */
> -
> -#define KSZ_INGRESS_TAG_LEN 2
> -#define KSZ_EGRESS_TAG_LEN 1
> +#include "../../drivers/net/dsa/microchip/ksz_priv.h"
No.
Move the header file to somewhere under include/linux. You can split
it into private and public parts if you want, and just put the public
bits in include/linux.
> static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct dsa_slave_priv *p = netdev_priv(dev);
> + struct dsa_switch *ds = p->dp->ds;
> + struct ksz_device *swdev = ds->priv;
> struct sk_buff *nskb;
> + int len;
> int padlen;
> - u8 *tag;
>
> padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
>
> - if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
> + len = swdev->dev_ops->get_tx_len(swdev);
This is ugly. We have a clean separation between a switch driver and a
tag driver. Here you are mixing them together. Don't. Look at the
mailing lists for what Florian and I suggested to Pavel. It will also
solve your include file issue above.
Andrew
