>-----Original Message-----
>From: Simon Glass <s...@chromium.org>
>Sent: Thursday, January 14, 2021 5:42 PM
>To: Claudiu Manoil <claudiu.man...@nxp.com>
>Cc: Joe Hershberger <joe.hershber...@ni.com>; Bin Meng
><bmeng...@gmail.com>; Michael Walle <mich...@walle.cc>; U-Boot Mailing
>List <u-boot@lists.denx.de>; Vladimir Oltean <vladimir.olt...@nxp.com>;
>Alexandru Marginean <alexandru.margin...@nxp.com>
>Subject: Re: [PATCH 1/5] net: Introduce DSA class for Ethernet switches
>
[...]
>
>Reviewed-by: Simon Glass <s...@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.


Reply via email to