Hi Claudiu, On Fri, 15 Jan 2021 at 09:47, Claudiu Manoil <[email protected]> wrote: > > >-----Original Message----- > >From: Simon Glass <[email protected]> > >Sent: Thursday, January 14, 2021 5:42 PM > >To: Claudiu Manoil <[email protected]> > >Cc: Joe Hershberger <[email protected]>; Bin Meng > ><[email protected]>; Michael Walle <[email protected]>; U-Boot Mailing > >List <[email protected]>; Vladimir Oltean <[email protected]>; > >Alexandru Marginean <[email protected]> > >Subject: Re: [PATCH 1/5] net: Introduce DSA class for Ethernet switches > > > [...] > > > >Reviewed-by: Simon Glass <[email protected]> > > > >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
