Re: [PATCH v2 1/4] clk: move core flags into a new enum for kernel docs

[Stephen Boyd]

Quoting Brian Masney (2026-03-27 06:30:37)
> > +enum clk_core_flags {
> > +     CLK_SET_RATE_GATE               = BIT(0),
> > +     CLK_SET_PARENT_GATE             = BIT(1),
> > +     CLK_SET_RATE_PARENT             = BIT(2),
> > +     CLK_IGNORE_UNUSED               = BIT(3),
> > +     /* unused */
> > +     /* unused */
> > +     CLK_GET_RATE_NOCACHE            = BIT(6),
> > +     CLK_SET_RATE_NO_REPARENT        = BIT(7),
> > +     CLK_GET_ACCURACY_NOCACHE        = BIT(8),
> > +     CLK_RECALC_NEW_RATES            = BIT(9),
> > +     CLK_SET_RATE_UNGATE             = BIT(10),
> > +     CLK_IS_CRITICAL                 = BIT(11),
> > +     CLK_OPS_PARENT_ENABLE           = BIT(12),
> > +     CLK_DUTY_CYCLE_PARENT           = BIT(13),
> > +};
> 
> I just checked Sashiko [1] for this series and it has this comment:
> 
>     Could converting these unsigned long bitmasks to an enum create a silent
>     type-safety trap if flags ever reach BIT(31)?
>     
>     The flags fields in the clock framework are explicitly designed to be 64-bit
>     on 64-bit architectures (unsigned long). The BIT() macro evaluates to an
>     unsigned long. When defined as macros, operations like clearing flags
>     produce a 64-bit inverted mask, perfectly preserving the upper 32 bits.
>     
>     By moving these flags into an enum, their types are implicitly downgraded to
>     int, since the current maximum flag BIT(13) fits in a 32-bit signed integer.
>     If the flags ever grow to include BIT(31), the enumerator value will overflow
>     a signed 32-bit int, causing the compiler to type it as an unsigned int.
>     
>     Applying a bitwise NOT to this unsigned int will produce an unsigned int.
>     When this is bitwise ANDed with the 64-bit unsigned long flags variable,
>     the unsigned mask will zero-extend to 64 bits, silently clearing all upper
>     32 bits (bits 32-63) of the flags field.
>     
>     Would it be safer to keep them as #define macros and use a DOC: block
>     to properly document the flags using kernel-doc without breaking type safety?
> 

Don't break type safety. I hope we don't get to the point that we have
more than 32 flags, but you never know. Why do we need an enum? Just to
make it easier to grab in docs? Presumably a doc block would work just
as well and not put down a land mine to be discovered later.