[PATCH 1/5] net: Introduce DSA class for Ethernet switches

[Simon Glass]

Hi Claudiu,

On Fri, 15 Jan 2021 at 09:47, Claudiu Manoil <claudiu.manoil at nxp.com> wrote:
>
> >-----Original Message-----
> >From: Simon Glass <sjg at chromium.org>
> >Sent: Thursday, January 14, 2021 5:42 PM
> >To: Claudiu Manoil <claudiu.manoil at nxp.com>
> >Cc: Joe Hershberger <joe.hershberger at ni.com>; Bin Meng
> ><bmeng.cn at gmail.com>; Michael Walle <michael at walle.cc>; U-Boot Mailing
> >List <u-boot at lists.denx.de>; Vladimir Oltean <vladimir.oltean at nxp.com>;
> >Alexandru Marginean <alexandru.marginean at nxp.com>
> >Subject: Re: [PATCH 1/5] net: Introduce DSA class for Ethernet switches
> >
> [...]
> >
> >Reviewed-by: Simon Glass <sjg at 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