Hi Claudiu,
On Fri, 15 Jan 2021 at 09:47, Claudiu Manoil claudiu.manoil@nxp.com wrote:
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Thursday, January 14, 2021 5:42 PM To: Claudiu Manoil claudiu.manoil@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; Bin Meng bmeng.cn@gmail.com; Michael Walle michael@walle.cc; U-Boot Mailing List u-boot@lists.denx.de; Vladimir Oltean vladimir.oltean@nxp.com; Alexandru Marginean alexandru.marginean@nxp.com Subject: Re: [PATCH 1/5] net: Introduce DSA class for Ethernet switches
[...]
Reviewed-by: Simon Glass sjg@chromium.org
I don't think it is necessary to have the 'if (!pdev)' checks around the place. We need a way in U-Boot to have checks like that to catch programming errors but to be able to turn them off in production code to reduce size.
I suppose a Kconfig would do it, with:
if (CONFIG_IS_ENABLED(SAFETY) && !pdev) return log_,msg_ref("safety", -ENODEV)
Also note that -ENODEV is used by drive rmodel so it generally isn't safe to return it as a logic error. I think in this case because it never happens, it should be OK.
Thanks for the review, Simon. I thought about using assert(pdev) checks, but during development the simple "if (!pdev)..." proved more friendly. I like your idea about enabling the checks at compile time and disabling them in production. For now, since this SAFETY flag is not implemented, my understanding is that you’re ok with leaving the pdev checks in the code as they are right now and sometime in the future these will be converted to the "SAFETY" construct you mention.
Yes that's fine, you have my review tag.
+Tom Rini what do you think about CONFIG_SAFETY or similar to allow these bug checks to be disabled for code-size reasons?
Regards, Simon