From: Vladimir Oltean <firstname.lastname@example.org> To: Russell King - ARM Linux admin <email@example.com> Cc: Andrew Lunn <firstname.lastname@example.org>, Florian Fainelli <email@example.com>, Heiner Kallweit <firstname.lastname@example.org>, "David S. Miller" <email@example.com>, netdev <firstname.lastname@example.org> Subject: Re: [PATCH net-next 4/5] net: phylink: pcs: add 802.3 clause 22 helpers Date: Wed, 11 Mar 2020 21:59:18 +0200 [thread overview] Message-ID: <CA+h21hqnQd=SdQXiNVW5UPuZug8zcM64DUMRvjojZVgMs-tmBQ@mail.gmail.com> (raw) In-Reply-To: <20200311193223.GR25745@shell.armlinux.org.uk> On Wed, 11 Mar 2020 at 21:32, Russell King - ARM Linux admin <email@example.com> wrote: > > > So, why abuse some other subsystem's datastructure for something that > is entirely separate, potentially making the maintanence of that > subsystem more difficult for the maintainers? I don't get why one > would think this is an acceptable approach. > > What you've said is that you want to use struct phy_device, but you > don't want to publish it into the device model, you don't want to > use mdio accesses, you don't want to use phylib helpers. So, what's > the point of using struct phy_device? I don't see _any_ reason to > do that and make things unnecessarily more difficult for the phylib > maintainers. > So if it's such a big mistake... > > > Sorry, but you need to explain better what you would like to see here. > > > The additions I'm adding are to the SGMII specification; I find your > > > existing definitions to be obscure because they conflate two different > > > bit fields together to produce something for the ethtool linkmodes > > > (which I think is a big mistake.) > > > > I'm saying that there were already LPA_SGMII definitions in there. > > There are 2 "generic" solutions proposed now and yet they cannot agree > > on config_reg definitions. Omitting the fact that you did have a > > chance to point out that big mistake before it got merged, I'm > > wondering why you didn't remove them and add your new ones instead. > > The code rework is minimal. Is it because the definitions are in UAPI? > > If so, isn't it an even bigger mistake to put more stuff in UAPI? Why > > would user space care about the SGMII config_reg? There's no user even > > of the previous SGMII definitions as far as I can tell. > > I don't see it as a big deal - certainly not the kind of fuss you're > making over it. > ...why keep it? I'm all for creating a common interface for configuring this. It just makes me wonder how common it is going to be, if there's already a driver in-tree, from the same PCS hardware vendor, which after the patchset you're proposing is still going to use a different infrastructure. > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up Thanks, -Vladimir
next prev parent reply other threads:[~2020-03-11 19:59 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-11 12:06 [PATCH net-next 0/5] add phylink support for PCS Russell King - ARM Linux admin 2020-03-11 12:07 ` [PATCH net-next 1/5] net: mii: convert mii_lpa_to_ethtool_lpa_x() to linkmode variant Russell King 2020-03-11 12:07 ` [PATCH net-next 2/5] net: mii: add linkmode_adv_to_mii_adv_x() Russell King 2020-03-11 12:07 ` [PATCH net-next 3/5] net: mdiobus: add APIs for modifying a MDIO device register Russell King 2020-03-11 12:07 ` [PATCH net-next 4/5] net: phylink: pcs: add 802.3 clause 22 helpers Russell King 2020-03-11 14:06 ` Vladimir Oltean 2020-03-11 17:09 ` Russell King - ARM Linux admin 2020-03-11 18:44 ` Vladimir Oltean 2020-03-11 19:32 ` Russell King - ARM Linux admin 2020-03-11 19:59 ` Vladimir Oltean [this message] 2020-03-11 20:32 ` Russell King - ARM Linux admin 2020-03-12 12:54 ` Vladimir Oltean 2020-03-12 13:13 ` Russell King - ARM Linux admin 2020-03-12 13:35 ` Vladimir Oltean 2020-03-11 12:07 ` [PATCH net-next 5/5] net: phylink: pcs: add 802.3 clause 45 helpers Russell King 2020-03-11 12:46 ` [PATCH net-next 0/5] add phylink support for PCS Vladimir Oltean 2020-03-11 12:54 ` Russell King - ARM Linux admin 2020-03-11 13:57 ` Vladimir Oltean 2020-03-11 17:05 ` Russell King - ARM Linux admin 2020-03-11 18:16 ` Vladimir Oltean
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='CA+h21hqnQd=SdQXiNVW5UPuZug8zcM64DUMRvjojZVgMs-tmBQ@mail.gmail.com' \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH net-next 4/5] net: phylink: pcs: add 802.3 clause 22 helpers' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).