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$(echo .)com \
--cc=agross@kernel$(echo .)org \
--cc=ahalaney@redhat$(echo .)com \
--cc=alexandre.torgue@foss$(echo .)st.com \
--cc=andersson@kernel$(echo .)org \
--cc=andrey.konovalov@linaro$(echo .)org \
--cc=bhupesh.sharma@linaro$(echo .)org \
--cc=bmasney@redhat$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=devicetree@vger$(echo .)kernel.org \
--cc=echanude@redhat$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=hisunil@quicinc$(echo .)com \
--cc=jernej.skrabec@gmail$(echo .)com \
--cc=joabreu@synopsys$(echo .)com \
--cc=jonathanh@nvidia$(echo .)com \
--cc=jsuraj@qti$(echo .)qualcomm.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-kernel@vger$(echo .)kernel.org \
--cc=linux-stm32@st-md-mailman$(echo .)stormreply.com \
--cc=linux@armlinux$(echo .)org.uk \
--cc=mcoquelin.stm32@gmail$(echo .)com \
--cc=mohammad.athari.ismail@intel$(echo .)com \
--cc=mturquette@baylibre$(echo .)com \
--cc=ncai@quicinc$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=peppe.cavallaro@st$(echo .)com \
--cc=richardcochran@gmail$(echo .)com \
--cc=robh+dt@kernel$(echo .)org \
--cc=ruppala@nvidia$(echo .)com \
--cc=samuel@sholland$(echo .)org \
--cc=tee.min.tan@linux$(echo .)intel.com \
--cc=veekhee@apple$(echo .)com \
--cc=vkoul@kernel$(echo .)org \
--cc=wens@csie$(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