Re: [PATCH v3 1/9] driver core: Don't let a device probe until it's ready

[Greg Kroah-Hartman]

On Fri, Apr 03, 2026 at 08:34:20AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Fri, Apr 3, 2026 at 12:04 AM Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > > @@ -458,6 +458,18 @@ struct device_physical_location {
> > >       bool lid;
> > >  };
> > >
> > > +/**
> > > + * enum struct_device_flags - Flags in struct device
> > > + *
> > > + * Should be accessed with thread-safe bitops.
> > > + *
> > > + * @DEV_FLAG_READY_TO_PROBE: If set then device_add() has finished enough
> > > + *           initialization that probe could be called.
> > > + */
> > > +enum struct_device_flags {
> > > +     DEV_FLAG_READY_TO_PROBE,
> > > +};
> >
> > If you are going to want this to be a bit value, please use BIT(X) and
> > not an enum, as that's going to just get confusing over time.
> 
> I don't believe I can do that. BIT(x) is not compatible with the
> atomic bitops API. BIT(x) will turn the bit number (x) into a hex
> value, but the atomic bitops API needs the bit number.

Ah, yeah, sorry, missed that.

> If you wish, I can turn this into something like:
> 
> #define DEV_FLAG_READY_TO_PROBE         0
> #define DEV_FLAG_CAN_MATCH              1
> #define DEV_FLAG_DMA_IOMMU              2
> ...
> 
> ...but that seemed worse (to me) than the enum. Please shout if I
> misunderstood or you disagree.

If you make it a numbered enum, that's fine (I hate unnumbered ones),
and a comment somewhere saying we will "max out" at 63, or have a
DEV_FLAG_MAX_BIT entry in there.

> > Also, none of this manual test_bit()/set_bit() stuff, please give us
> > real "accessors" for this like:
> >         bool device_ready_to_probe(struct device *dev);
> >
> > so that it's obvious what is happening.
> 
> Sure, that matches Rafael's request and is a nice improvement. To keep
> from having to replicate a bunch of boilerplate code, I'll have macros
> define the accessors:
> 
> #define __create_dev_flag_accessors(accessor_name, flag_name) \
> static inline bool dev_##accessor_name(const struct device *dev) { \
>         return test_bit(flag_name, &dev->flags); \
> } \
> static inline void dev_set_##accessor_name(struct device *dev) { \
>         set_bit(flag_name, &dev->flags); \
> } \
> static inline void dev_clear_##accessor_name(struct device *dev) { \
>         clear_bit(flag_name, &dev->flags); \
> } \
> static inline void dev_assign_##accessor_name(struct device *dev, bool
> value) { \
>         assign_bit(flag_name, &dev->flags, value); \
> } \
> static inline bool dev_test_and_set_##accessor_name(struct device *dev) { \
>         return test_and_set_bit(flag_name, &dev->flags); \
> }
> 
> __create_dev_flag_accessors(ready_to_probe, DEV_FLAG_READY_TO_PROBE);
> __create_dev_flag_accessors(can_match, DEV_FLAG_CAN_MATCH);
> __create_dev_flag_accessors(dma_iommu, DEV_FLAG_DMA_IOMMU);
> __create_dev_flag_accessors(dma_skip_sync, DEV_FLAG_DMA_SKIP_SYNC);
> __create_dev_flag_accessors(dma_ops_bypass, DEV_FLAG_DMA_OPS_BYPASS);
> __create_dev_flag_accessors(state_synced, DEV_FLAG_STATE_SYNCED);
> __create_dev_flag_accessors(dma_coherent, DEV_FLAG_DMA_COHERENT);
> __create_dev_flag_accessors(of_node_reused, DEV_FLAG_OF_NODE_REUSED);
> __create_dev_flag_accessors(offline_disabled, DEV_FLAG_OFFLINE_DISABLED);
> __create_dev_flag_accessors(offline, DEV_FLAG_OFFLINE);
> 
> Happy to tweak the above if desired.

Sure, that works, thanks!

greg k-h