From: Simon Horman <simon.horman@corigine.com>
To: Andrew Halaney <ahalaney@redhat.com>
Cc: linux-kernel@vger.kernel.org, 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, vkoul@kernel.org,
bhupesh.sharma@linaro.org, wens@csie.org,
jernej.skrabec@gmail.com, samuel@sholland.org,
mturquette@baylibre.com, peppe.cavallaro@st.com,
alexandre.torgue@foss.st.com, joabreu@synopsys.com,
mcoquelin.stm32@gmail.com, richardcochran@gmail.com,
linux@armlinux.org.uk, veekhee@apple.com,
tee.min.tan@linux.intel.com, mohammad.athari.ismail@intel.com,
jonathanh@nvidia.com, ruppala@nvidia.com, bmasney@redhat.com,
andrey.konovalov@linaro.org, linux-arm-msm@vger.kernel.org,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, ncai@quicinc.com,
jsuraj@qti.qualcomm.com, hisunil@quicinc.com,
echanude@redhat.com
Subject: Re: [PATCH net-next v3 08/12] net: stmmac: Pass stmmac_priv in some callbacks
Date: Tue, 11 Apr 2023 19:43:39 +0200 [thread overview]
Message-ID: <ZDWcSxNivNUHyDOR@corigine.com> (raw)
In-Reply-To: <20230410212422.2rztlqspw5vjtb4d@halaney-x13s>
On Mon, Apr 10, 2023 at 04:24:22PM -0500, Andrew Halaney wrote:
> On Fri, Apr 07, 2023 at 12:34:53PM -0500, Andrew Halaney wrote:
> > On Sat, Apr 01, 2023 at 05:06:21PM +0200, Simon Horman wrote:
> > > On Fri, Mar 31, 2023 at 04:45:45PM -0500, Andrew Halaney wrote:
> > > > Passing stmmac_priv to some of the callbacks allows hwif implementations
> > > > to grab some data that platforms can customize. Adjust the callbacks
> > > > accordingly in preparation of such a platform customization.
> > > >
> > > > Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
> > >
> > > ...
> > >
> > > > #define stmmac_reset(__priv, __args...) \
> > > > @@ -223,59 +240,59 @@ struct stmmac_dma_ops {
> > > > #define stmmac_dma_init(__priv, __args...) \
> > > > stmmac_do_void_callback(__priv, dma, init, __args)
> > > > #define stmmac_init_chan(__priv, __args...) \
> > > > - stmmac_do_void_callback(__priv, dma, init_chan, __args)
> > > > + stmmac_do_void_callback(__priv, dma, init_chan, __priv, __args)
> > >
> > > Hi Andrew,
> > >
> > > Rather than maintaining these macros can we just get rid of them?
> > > I'd be surprised if things aren't nicer with functions in their place [1].
> > >
> > > f.e., we now have (__priv, ..., __priv, ...) due to a generalisation
> > > that seems to take a lot more than it gives.
> > >
> > > [1] https://lore.kernel.org/linux-arm-kernel/ZBst1SzcIS4j+t46@corigine.com/
> > >
> >
> > Thanks for the pointer. I think that makes sense, I'll take that
> > approach for these functions (and maybe in a follow-up series I'll
> > tackle all of them just because the lack of consistency will eat me up).
> >
>
> I tried taking this approach for a spin, and I'm not so sure about it
> now!
>
> 1. Implementing the functions as static inline requires us to know
> about stmmac_priv, but that's getting into circular dependency land
> 2. You could define them in hwif.c, but then they're not inlined
> 3. There's still a good bit of boilerplate that's repeated all over
> with the approach. Ignoring 1 above, you get something like this:
>
> static inline int stmmac_init_chan(struct stmmac_priv *priv,
> void __iomem *ioaddr,
> struct stmmac_dma_cfg *dma_cfg, u32 chan)
> {
> if (priv->hw->dma && priv->hw->dma->init_chan) {
> priv->hw->dma->init_chan(priv, ioaddr, dma_cfg, chan);
> return 0;
> }
> return -EINVAL;
> }
>
> that is then repeated for every function... which is making me actually
> appreciate the macros some for reducing boilerplate.
>
> Am I suffering from a case of holiday brain, and 1-3 above are silly
> points with obvious answers, or do they make you reconsider continuing
> with the current approach in hwif.h?
I'm about to embark to the holiday brain zone.
But before I do I wanted to acknowledge your concerns and that, yes,
it may be easier said than done.
next prev parent reply other threads:[~2023-04-11 17:43 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-31 21:45 [PATCH net-next v3 00/12] Add EMAC3 support for sa8540p-ride Andrew Halaney
2023-03-31 21:45 ` [PATCH net-next v3 01/12] dt-bindings: net: snps,dwmac: Update interrupt-names Andrew Halaney
2023-03-31 21:45 ` [PATCH net-next v3 02/12] dt-bindings: net: snps,dwmac: Add Qualcomm Ethernet ETHQOS compatibles Andrew Halaney
2023-03-31 21:45 ` [PATCH net-next v3 03/12] dt-bindings: net: qcom,ethqos: Convert bindings to yaml Andrew Halaney
2023-03-31 21:45 ` [PATCH net-next v3 04/12] dt-bindings: net: qcom,ethqos: Add Qualcomm sc8280xp compatibles Andrew Halaney
2023-04-02 10:49 ` Krzysztof Kozlowski
2023-03-31 21:45 ` [PATCH net-next v3 05/12] net: stmmac: Remove unnecessary if statement brackets Andrew Halaney
2023-03-31 21:45 ` [PATCH net-next v3 06/12] net: stmmac: Fix DMA typo Andrew Halaney
2023-03-31 21:45 ` [PATCH net-next v3 07/12] net: stmmac: Remove some unnecessary void pointers Andrew Halaney
2023-04-01 14:49 ` Simon Horman
2023-03-31 21:45 ` [PATCH net-next v3 08/12] net: stmmac: Pass stmmac_priv in some callbacks Andrew Halaney
2023-04-01 15:06 ` Simon Horman
2023-04-07 17:34 ` Andrew Halaney
2023-04-10 21:24 ` Andrew Halaney
2023-04-11 17:43 ` Simon Horman [this message]
2023-03-31 21:45 ` [PATCH net-next v3 09/12] net: stmmac: dwmac4: Allow platforms to specify some DMA/MTL offsets Andrew Halaney
2023-04-01 14:58 ` Simon Horman
2023-04-07 17:36 ` Andrew Halaney
2023-03-31 21:45 ` [PATCH net-next v3 10/12] net: stmmac: dwmac-qcom-ethqos: Respect phy-mode and TX delay Andrew Halaney
2023-03-31 21:45 ` [PATCH net-next v3 11/12] net: stmmac: dwmac-qcom-ethqos: Use loopback_en for all speeds Andrew Halaney
2023-03-31 22:06 ` [PATCH net-next v3 00/12] Add EMAC3 support for sa8540p-ride Andrew Halaney
2023-04-01 4:55 ` Jakub Kicinski
2023-04-03 13:01 ` Andrew Halaney
2023-04-03 16:52 ` [PATCH net-next v3 12/12] net: stmmac: dwmac-qcom-ethqos: Add EMAC3 support Andrew Halaney
2023-04-06 8:57 ` Paolo Abeni
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=ZDWcSxNivNUHyDOR@corigine.com \
--to=simon.horman@corigine.com \
--cc=agross@kernel.org \
--cc=ahalaney@redhat.com \
--cc=alexandre.torgue@foss.st.com \
--cc=andersson@kernel.org \
--cc=andrey.konovalov@linaro.org \
--cc=bhupesh.sharma@linaro.org \
--cc=bmasney@redhat.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=echanude@redhat.com \
--cc=edumazet@google.com \
--cc=hisunil@quicinc.com \
--cc=jernej.skrabec@gmail.com \
--cc=joabreu@synopsys.com \
--cc=jonathanh@nvidia.com \
--cc=jsuraj@qti.qualcomm.com \
--cc=konrad.dybcio@linaro.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=linux@armlinux.org.uk \
--cc=mcoquelin.stm32@gmail.com \
--cc=mohammad.athari.ismail@intel.com \
--cc=mturquette@baylibre.com \
--cc=ncai@quicinc.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=peppe.cavallaro@st.com \
--cc=richardcochran@gmail.com \
--cc=robh+dt@kernel.org \
--cc=ruppala@nvidia.com \
--cc=samuel@sholland.org \
--cc=tee.min.tan@linux.intel.com \
--cc=veekhee@apple.com \
--cc=vkoul@kernel.org \
--cc=wens@csie.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;
as well as URLs for NNTP newsgroup(s).