public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: Tristram.Ha@microchip.com
Cc: f.fainelli@gmail.com, andrew@lunn.ch,
	UNGLinuxDriver@microchip.com, netdev@vger.kernel.org
Subject: Re: [PATCH RFC 5/6] net: dsa: microchip: Update tag_ksz.c to access switch driver
Date: Sun, 9 Dec 2018 10:17:20 +0100	[thread overview]
Message-ID: <20181209091720.GE7561@amd> (raw)
In-Reply-To: <SN1PR11MB04469EC6A485465166B3C00DECA90@SN1PR11MB0446.namprd11.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]

On Thu 2018-12-06 20:00:26, Tristram.Ha@microchip.com wrote:
> > >>> Update tag_ksz.c to access switch driver's tail tagging operations.
> > >>
> > >> Hi Tristram
> > >>
> > >> Humm, i'm not sure we want this, the tagging spit into two places.  I
> > >> need to take a closer look at the previous patch, to see why it cannot
> > >> be done here.
> > >
> > > O.K, i think i get what is going on.
> > >
> > > I would however implement it differently.
> > >
> > > One net/dsa/tag_X.c file can export two dsa_device_ops structures,
> > > allowing you to share common code for the two taggers. You could call
> > > these DSA_TAG_PROTO_KSZ_1_BYTE, and DSA_TAG_PROTO_KSZ_2_BYTE,
> > and the
> > > .get_tag_protocol call would then return the correct one for the
> > > switch.
> > 
> > Agreed, that is what is done by net/dsa/tag_brcm.c because there are two
> > formats for the Broadcom tag:
> > 
> > - TAG_BRCM: the 4-bytes Broadcom tag is between MAC SA and Ethertype
> > - TAG_BRCM_PREPEND: the 4-bytes Broadcom tag is before the MAC DA
> >
> 
> I did try to implement this way.  But the other switches do not have the same
> format even though the length is the same.  Then I need to change the following
> files for any new KSZ switch: include/linux/dsa.h, net/dsa/dsa.c, net/dsa/dsa_priv.h,
> and finally net/dsa/tag_ksz.c.
> 
> Even then it will not work if Microchip wants to add 1588 PTP capability to the switches.
> 
> For KSZ9477 the length of the tail tag changes when the PTP function is enabled.
> Typically this function is either enabled or disabled all the time, but if users want to
> change that during normal operation to see how the switch behaves, the transmit
> function completely stops working correctly.

I'd be careful about locking. Seems like dsa was designed with "tag
format is static", and you want to change it dynamically...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

  parent reply	other threads:[~2018-12-09  9:17 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-03 23:34 [PATCH RFC 0/6] net: dsa: microchip: Modify KSZ9477 DSA driver to support different tail tag formats Tristram.Ha
2018-12-03 23:34 ` [PATCH RFC 1/6] net: dsa: microchip: Prepare PHY for proper advertisement Tristram.Ha
2018-12-05 17:40   ` Andrew Lunn
2018-12-06 20:31     ` Tristram.Ha
2018-12-03 23:34 ` [PATCH RFC 2/6] net: dsa: microchip: Add MIB counter reading support Tristram.Ha
2018-12-05 17:53   ` Andrew Lunn
2018-12-06 20:16     ` Tristram.Ha
2018-12-06 20:42       ` Andrew Lunn
2018-12-03 23:34 ` [PATCH RFC 3/6] net: dsa: microchip: Break ksz_priv.h into two files Tristram.Ha
2018-12-03 23:34 ` [PATCH RFC 4/6] net: dsa: microchip: Each switch driver has its own tail tagging operations Tristram.Ha
2018-12-03 23:34 ` [PATCH RFC 5/6] net: dsa: microchip: Update tag_ksz.c to access switch driver Tristram.Ha
2018-12-05 18:00   ` Andrew Lunn
2018-12-05 18:18     ` Andrew Lunn
2018-12-05 18:52       ` Florian Fainelli
2018-12-06 20:00         ` Tristram.Ha
2018-12-06 20:41           ` Andrew Lunn
2018-12-07  3:16           ` Richard Cochran
2018-12-09  9:17           ` Pavel Machek [this message]
2018-12-11 23:59             ` Tristram.Ha
2018-12-12  8:18               ` Andrew Lunn
2018-12-03 23:34 ` [PATCH RFC 6/6] net: dsa: microchip: Add switch offload forwarding support Tristram.Ha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181209091720.GE7561@amd \
    --to=pavel@ucw.cz \
    --cc=Tristram.Ha@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=f.fainelli@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox