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

[Doug Anderson]

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.

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.


> 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.

-Doug