From: "Russell King (Oracle)" <linux@armlinux•org.uk>
To: Lei Wei <quic_leiwei@quicinc•com>
Cc: Luo Jie <quic_luoj@quicinc•com>,
agross@kernel•org, andersson@kernel•org,
konrad.dybcio@linaro•org, davem@davemloft•net,
edumazet@google•com, kuba@kernel•org, pabeni@redhat•com,
robh+dt@kernel•org, krzysztof.kozlowski+dt@linaro•org,
conor+dt@kernel•org, corbet@lwn•net, catalin.marinas@arm•com,
will@kernel•org, p.zabel@pengutronix•de, shannon.nelson@amd•com,
anthony.l.nguyen@intel•com, jasowang@redhat•com,
brett.creeley@amd•com, rrameshbabu@nvidia•com,
joshua.a.hay@intel•com, arnd@arndb•de, geert+renesas@glider•be,
neil.armstrong@linaro•org, dmitry.baryshkov@linaro•org,
nfraprado@collabora•com, m.szyprowski@samsung•com,
u-kumar1@ti•com, jacob.e.keller@intel•com, andrew@lunn•ch,
netdev@vger•kernel.org, linux-arm-msm@vger•kernel.org,
devicetree@vger•kernel.org, linux-kernel@vger•kernel.org,
linux-doc@vger•kernel.org, linux-arm-kernel@lists•infradead.org,
ryazanov.s.a@gmail•com, ansuelsmth@gmail•com,
quic_kkumarcs@quicinc•com, quic_suruchia@quicinc•com,
quic_soni@quicinc•com, quic_pavir@quicinc•com,
quic_souravp@quicinc•com, quic_linchen@quicinc•com
Subject: Re: [PATCH net-next 18/20] net: ethernet: qualcomm: Add PPE MAC support for phylink
Date: Mon, 22 Jan 2024 17:36:44 +0000 [thread overview]
Message-ID: <Za6nrICG8gjwTsJ9@shell.armlinux.org.uk> (raw)
In-Reply-To: <fc9c3e08-a83c-4748-89e4-8b7b0c62da7f@quicinc.com>
On Mon, Jan 22, 2024 at 11:01:26PM +0800, Lei Wei wrote:
>
>
> On 1/10/2024 8:18 PM, Russell King (Oracle) wrote:
> > On Wed, Jan 10, 2024 at 07:40:30PM +0800, Luo Jie wrote:
> > > @@ -352,6 +1230,12 @@ static int ppe_port_maxframe_set(struct ppe_device *ppe_dev,
> > > }
> > > static struct ppe_device_ops qcom_ppe_ops = {
> > > + .phylink_setup = ppe_phylink_setup,
> > > + .phylink_destroy = ppe_phylink_destroy,
> > > + .phylink_mac_config = ppe_phylink_mac_config,
> > > + .phylink_mac_link_up = ppe_phylink_mac_link_up,
> > > + .phylink_mac_link_down = ppe_phylink_mac_link_down,
> > > + .phylink_mac_select_pcs = ppe_phylink_mac_select_pcs,
> > > .set_maxframe = ppe_port_maxframe_set,
> > > };
> >
> > Why this extra layer of abstraction? If you need separate phylink
> > operations, why not implement separate phylink_mac_ops structures?
> >
>
> This PPE driver will serve as the base driver for higher level drivers
> such as the ethernet DMA (EDMA) driver and the DSA switch driver.
Why not have the higher level drivers provide a pointer to the
appropriate phylink_mac_ops structure? Having extra levels of
indirection makes my future maintenance of phylink harder (I'm already
bugged by DSA doing this, and it's a right pain.)
For example, if one of your higher level drivers needs the mac_prepare
or mac_finish functionality, you have to add a shim, extra function
pointers and so on.
If I need to add an extra parameter to a method, then I have to fix
up your shim layer _as well_ as all the called methods - in other
words, it adds extra maintenance burden.
It also makes detecting whether an implementation provides something
or not harder - see the problems when mac_select_pcs() was introduced
and rather than testing to see whether the method is populated, we
have to call the method with a dummy value to discover whether the
sub-driver implements it or not. Honestly, I would really like to get
rid of DSA's phylink_mac_ops shim layer.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2024-01-22 17:37 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-10 11:40 [PATCH net-next 00/20] net: ethernet: Add qcom PPE driver Luo Jie
2024-01-10 11:40 ` [PATCH net-next 01/20] Documentation: networking: qcom PPE driver documentation Luo Jie
2024-01-10 11:40 ` [PATCH net-next 02/20] dt-bindings: net: qcom,ppe: Add bindings yaml file Luo Jie
2024-01-10 12:22 ` Krzysztof Kozlowski
2024-01-22 13:55 ` Jie Luo
2024-01-22 14:25 ` Andrew Lunn
2024-01-10 13:01 ` Rob Herring
2024-01-10 11:40 ` [PATCH net-next 03/20] net: ethernet: qualcomm: Add qcom PPE driver Luo Jie
2024-01-10 11:40 ` [PATCH net-next 04/20] net: ethernet: qualcomm: Add PPE buffer manager configuration Luo Jie
2024-01-10 11:40 ` [PATCH net-next 05/20] net: ethernet: qualcomm: Add PPE queue management config Luo Jie
2024-01-10 11:40 ` [PATCH net-next 06/20] net: ethernet: qualcomm: Add PPE TDM config Luo Jie
2024-01-10 11:40 ` [PATCH net-next 07/20] net: ethernet: qualcomm: Add PPE port scheduler resource Luo Jie
2024-01-10 11:40 ` [PATCH net-next 08/20] net: ethernet: qualcomm: Add PPE scheduler config Luo Jie
2024-01-10 11:40 ` [PATCH net-next 09/20] net: ethernet: qualcomm: Add PPE queue config Luo Jie
2024-01-10 11:40 ` [PATCH net-next 10/20] net: ethernet: qualcomm: Add PPE service code config Luo Jie
2024-01-10 11:40 ` [PATCH net-next 11/20] net: ethernet: qualcomm: Add PPE port control config Luo Jie
2024-01-10 11:40 ` [PATCH net-next 12/20] net: ethernet: qualcomm: Add PPE RSS hash config Luo Jie
2024-01-10 11:40 ` [PATCH net-next 13/20] net: ethernet: qualcomm: Export PPE function set_maxframe Luo Jie
2024-01-10 11:40 ` [PATCH net-next 14/20] net: ethernet: qualcomm: Add PPE AC(admission control) function Luo Jie
2024-01-10 11:40 ` [PATCH net-next 15/20] net: ethernet: qualcomm: Add PPE debugfs counters Luo Jie
2024-01-10 11:40 ` [PATCH net-next 16/20] net: ethernet: qualcomm: Add PPE L2 bridge initialization Luo Jie
2024-01-10 11:40 ` [PATCH net-next 17/20] net: ethernet: qualcomm: Add PPE UNIPHY support for phylink Luo Jie
2024-01-10 12:09 ` Russell King (Oracle)
2024-01-22 14:37 ` Lei Wei
2024-01-10 11:40 ` [PATCH net-next 18/20] net: ethernet: qualcomm: Add PPE MAC " Luo Jie
2024-01-10 12:18 ` Russell King (Oracle)
2024-01-22 15:01 ` Lei Wei
2024-01-22 17:36 ` Russell King (Oracle) [this message]
2024-01-10 11:40 ` [PATCH net-next 19/20] net: ethernet: qualcomm: Add PPE MAC functions Luo Jie
2024-01-10 11:40 ` [PATCH net-next 20/20] arm64: defconfig: Enable qcom PPE driver Luo Jie
2024-01-10 12:24 ` [PATCH net-next 00/20] net: ethernet: Add " Krzysztof Kozlowski
2024-01-10 15:44 ` Simon Horman
2024-01-12 15:49 ` Jie Luo
2024-01-10 22:24 ` Jakub Kicinski
2024-01-11 15:49 ` Jie Luo
2024-01-12 17:56 ` Christian Marangi
2024-01-17 15:25 ` Jie Luo
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=Za6nrICG8gjwTsJ9@shell.armlinux.org.uk \
--to=linux@armlinux$(echo .)org.uk \
--cc=agross@kernel$(echo .)org \
--cc=andersson@kernel$(echo .)org \
--cc=andrew@lunn$(echo .)ch \
--cc=ansuelsmth@gmail$(echo .)com \
--cc=anthony.l.nguyen@intel$(echo .)com \
--cc=arnd@arndb$(echo .)de \
--cc=brett.creeley@amd$(echo .)com \
--cc=catalin.marinas@arm$(echo .)com \
--cc=conor+dt@kernel$(echo .)org \
--cc=corbet@lwn$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=devicetree@vger$(echo .)kernel.org \
--cc=dmitry.baryshkov@linaro$(echo .)org \
--cc=edumazet@google$(echo .)com \
--cc=geert+renesas@glider$(echo .)be \
--cc=jacob.e.keller@intel$(echo .)com \
--cc=jasowang@redhat$(echo .)com \
--cc=joshua.a.hay@intel$(echo .)com \
--cc=konrad.dybcio@linaro$(echo .)org \
--cc=krzysztof.kozlowski+dt@linaro$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-arm-msm@vger$(echo .)kernel.org \
--cc=linux-doc@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=m.szyprowski@samsung$(echo .)com \
--cc=neil.armstrong@linaro$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nfraprado@collabora$(echo .)com \
--cc=p.zabel@pengutronix$(echo .)de \
--cc=pabeni@redhat$(echo .)com \
--cc=quic_kkumarcs@quicinc$(echo .)com \
--cc=quic_leiwei@quicinc$(echo .)com \
--cc=quic_linchen@quicinc$(echo .)com \
--cc=quic_luoj@quicinc$(echo .)com \
--cc=quic_pavir@quicinc$(echo .)com \
--cc=quic_soni@quicinc$(echo .)com \
--cc=quic_souravp@quicinc$(echo .)com \
--cc=quic_suruchia@quicinc$(echo .)com \
--cc=robh+dt@kernel$(echo .)org \
--cc=rrameshbabu@nvidia$(echo .)com \
--cc=ryazanov.s.a@gmail$(echo .)com \
--cc=shannon.nelson@amd$(echo .)com \
--cc=u-kumar1@ti$(echo .)com \
--cc=will@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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