CONFIG_IS_ENABLED vs IS_ENABLED

[Troy Kisky]

On Mon, Jan 30, 2023 at 11:44 AM Tom Rini <trini at konsulko.com> wrote:

> On Mon, Jan 30, 2023 at 10:51:03AM -0800, Troy Kisky wrote:
> > Hi Tom
> >
> >
> > On Mon, Jan 30, 2023 at 9:18 AM Tom Rini <trini at konsulko.com> wrote:
> >
> > > On Sat, Jan 28, 2023 at 09:25:54AM -0800, Troy Kisky wrote:
> > > > Thanks Tom,
> > > >
> > > > I cleaned up the PR based on the CI results. Here's my current
> changes.
> > > >
> > > > Author: Troy Kisky <troy.kisky at boundarydevices.com>
> > > > Date:   Fri Jan 27 11:03:11 2023 -0800
> > > >
> > > >     dm: device-internal: use EVENT instead of DM_EVENT, because
> > > > event_notify is built for EVENT
> > > >
> > > >     Signed-off-by: Troy Kisky <troy.kisky at boundarydevices.com>
> > > >
> > > > diff --git a/include/dm/device-internal.h
> b/include/dm/device-internal.h
> > > > index f31c4702086..2e725aa9416 100644
> > > > --- a/include/dm/device-internal.h
> > > > +++ b/include/dm/device-internal.h
> > > > @@ -431,7 +431,7 @@ static inline void devres_release_all(struct
> udevice
> > > > *dev)
> > > >
> > > >  static inline int device_notify(const struct udevice *dev, enum
> event_t
> > > > type)
> > > >  {
> > > > -#if CONFIG_IS_ENABLED(DM_EVENT)
> > > > +#if CONFIG_IS_ENABLED(EVENT)
> > > >         return event_notify(type, &dev, sizeof(dev));
> > > >  #else
> > > >         return 0;
> > >
> > > Given 448e2b6327d0 ("event: Correct dependencies on the EVENT
> > > framework") I'm a little worried about this change here and want to be
> > > extra sure it doesn't break something inadvertently.
> >
> >
> > event_notify is in common/event, and the Makefile has
> > obj-$(CONFIG_$(SPL_TPL_)EVENT) += event.o
> >
> > So, the other option is to change the Makefile line to
> >   obj-$(CONFIG_$(SPL_TPL_)DM_EVENT) += event.o
> >
> > I don't know which is best.
>
> Right, event_notify is part of the general event framework. The function
> above, device_notify, is part of DM_EVENT. This should probably be
> IS_ENABLED and not CONFIG_IS_ENABLED.
>
>
I think DM_EVENT should be globally replaced by  EVENT. It doesn't seem to
control much.
Alternatively, I could add DM_EVENT to my SPL config list.
changing to IS_ENABLED  is what my script did before this patch, and caused
CI to fail.


Simon ?


> >   Aside from that,
> > > are you able to post your series?
> >
> >
> >
> > I was hoping each of the maintainers could run the script and see if the
> > patches for their area make sense.
> > I don't need my sign-off on any of the patches.
>
> Ah.  I'm not sure how likely that is to happen.
>
> >   And incorporate your checking script
> > > in to .gitlab-ci.yml / .azure-pipline.yml ?
> > >
> >
> > I'll post a patch for that as an RFC
>
> OK.
>
> --
> Tom
>