Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 11 Apr 2023 19:43:39 +0200
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
Message-ID: <ZDWcSxNivNUHyDOR@corigine.com>
References: <20230331214549.756660-1-ahalaney@redhat.com>
 <20230331214549.756660-9-ahalaney@redhat.com>
 <ZChIbc6TnQyZ/Fiu@corigine.com>
 <20230407173453.hsfhbr66254z57ym@halaney-x13s>
 <20230410212422.2rztlqspw5vjtb4d@halaney-x13s>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230410212422.2rztlqspw5vjtb4d@halaney-x13s>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?CUNX1cJ4HiSl1glHSAfvUlGpO45kduezSJR8Lggx5VImC2DjhG94itkhdhqr?=
 =?us-ascii?Q?EqOr9Bho7YiVUBUbidlWcPcEFtz892UMK9WVW7XRbNSQsq/sFT4DuHjff0oy?=
 =?us-ascii?Q?I7Xo3+sUE9L3c09sgLLuVQ+BR6N/xPXwJwxvqtI+VtfPNNk9zv5y4/74c4Jp?=
 =?us-ascii?Q?4/x0SRIK5shr0JpY2g991uFgAQ4rFiRduzpFM9yQIqGkpyGyKSaBKwA2IKAC?=
 =?us-ascii?Q?fxSaxG5I1liOAcdCUbmz0A6kQaReCDKzmP7njSUwV7AIfql7Xjzjk5JbPZfR?=
 =?us-ascii?Q?OTm0dP0XynVX2lPcDsx9JJewYpyQvEE5LoB/4/Bry5BQUfVwdU/MoD2Qg1Nh?=
 =?us-ascii?Q?uK2U61xC6oOUdkG8YyvbUMtP7NEi0m4atri1ojlYFDv9EsXWbHz3SAcEtAnp?=
 =?us-ascii?Q?HTpyZSZJtqmFLPO6oofRitckn/vSTKs7dl5/kinKH3DOQVNlgrUPHItevKD+?=
 =?us-ascii?Q?0chCM6sWofXPcpcTcZTwYYs33yk7AWDjrmMwmzMRWpQwu4P+Gw0jssg3EFi7?=
 =?us-ascii?Q?bl2M1BzFzinMMWCyeDKtwQQQKX5cx5/PrKvJNwqExODb3RINEx7FuqByKxPd?=
 =?us-ascii?Q?3QXPtTQAxoW0a6fykpb+xUKNyy0QRzlIk4/Db/yajyURdEqmHCfZyPxJQ07Y?=
 =?us-ascii?Q?uoOaWwpSAlxioeiQWhX7cN8SIlyuzO0QO0C4alwKPg6rMesory6rIUdo6p7O?=
 =?us-ascii?Q?squCNIqwO/kW+QC3mQrzo69cQsI/Wx+GmkTWyDTwldU1sAnbC+stf018ZvA9?=
 =?us-ascii?Q?KfpBoCM1cWZKW24r4DURi4Cfo4hETGjOB5K5CukSvgcValqXzmNyxF6AXuN3?=
 =?us-ascii?Q?5qokS4pEP+Lgbbzj/Gx0rRnD0UyLNf50U4DIHrdpR2AJ+85l4ds055pYwmYu?=
 =?us-ascii?Q?yfFOv7AEWeibYbaQlTpqwKAs93BsWscM4gAxgWYmBL7bQ7n0nIL9X68SMYMt?=
 =?us-ascii?Q?2V2UUaRP7g+39WbiSdk/dbER4tKFKhsluIUiqYZB0YtDyJ0JM+5jX347LREA?=
 =?us-ascii?Q?M5kPtp9Vctlde6IP6MCFv54fYeNDxvc6gT33zXwFW0kxbd21rz7eKuRPviUm?=
 =?us-ascii?Q?jO9veo1jBFsLv2pv2ccIGKA25M4XvEHp2qW8VLqdGSq1pcwoj6+VyRU7Ddck?=
 =?us-ascii?Q?GuOQbl7lpys9m1zUSx8UHjW3ElWaI4mVF/86DWjxvVVgatfhCeOKp1heqTws?=
 =?us-ascii?Q?MdekN3l6EyqnpVbJpIib1tsW/hoeTwOI27oxU1OwdzjEyMG5+vu0nD8kXHV4?=
 =?us-ascii?Q?LsJEUSd8Yiy9NI0UoBvfBgkcFT3YX8VPPCItCcffCqkQZDn2k6wpAw/6mLzs?=
 =?us-ascii?Q?yiw0I53hbct8BmGgYqg1bVaGJwpcvnsc53Bag44/iF+Bb2yET3+zzabsnYfg?=
 =?us-ascii?Q?fqgoGjOz1vR4SEorvnCs/xIx9ViU++3HyVkvmcLcJML2fNerG30XDvX0+FHt?=
 =?us-ascii?Q?dNTM6kEY96rvrhcxQ94ajGyI1CGDwB4qYnf54MT2yIcpDsmouxoY5gu/oYEw?=
 =?us-ascii?Q?mL3Lv5QcAeP9VyDW61j6ZtoYEYr/6UE4sYXTkhmhdKNeOtOXlq+E+3zaveCX?=
 =?us-ascii?Q?ictyRYORppVvmabY7O23XqwG/3cF93gXJcmWc3XSQkhBRBgVJzK26zywggxV?=
 =?us-ascii?Q?gG+Qm0xIS8C6ifWdvfZz4mcl0svyPSg0oMlLk82N92oXN0cqcc+uvXznZiyU?=
 =?us-ascii?Q?/hIhbw=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b480b3a7-a92c-4615-8585-08db3ab44f0e
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2023 17:43:50.1006
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: R1rNNDEZTOMkOzp/UmG2MGW5S4laPu9dhOffChMPHCcUowfNm4qgElAZDhG53yjCqQipilbLtNOCp34bTd4Q64jqd+Qr2+p792CGXgAt44k=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR13MB5995
X-Spam-Status: No, score=-0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS
        autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.