Hi Troy,
On Mon, 30 Jan 2023 at 15:17, Troy Kisky <troykiskybound...@gmail.com> wrote:
>
>
>
> On Mon, Jan 30, 2023 at 11:44 AM Tom Rini <tr...@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 <tr...@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.ki...@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.ki...@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 ?
DM_EVENT controls whether driver model sends out events via
device_notify(). I don't see that this feature is widely enabled yet,
so I don't think we should require it. However perhaps the usage will
increase with time.
>
>
>> > > 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.
Regards,
Simon
