On Mon, Jul 29, 2019 at 3:03 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote: > > On Mon, Jul 29, 2019 at 11:43 PM Saravana Kannan <saravanak@xxxxxxxxxx> wrote: > > > > On Mon, Jul 29, 2019 at 2:25 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote: > > > > > > On Mon, Jul 29, 2019 at 10:47 PM Saravana Kannan <saravanak@xxxxxxxxxx> wrote: > > > > > > > > Rafael, > > > > > > > > This is the fix you need. Or something link this. > > > > > > > > I had asked you to reject DL_FLAG_MANAGED as an input flag if you are > > > > marking it as internal (in the comments). But looks like you were also > > > > trying to check for "undefined" bit positions. However, the check > > > > isn't correct because DL_MANAGED_FLAGS doesn't include (rightfully so) > > > > DL_FLAG_PM_RUNTIME and DL_FLAG_RPM_ACTIVE . > > > > > > > > I tried to write a DL_FLAG_EXTERNAL to include all the external flags, > > > > but that felt like a maintenance headache that's not worth carrying. I > > > > think it's simpler to just error out when internal flags being passed > > > > in and ignore any undefined bit positions. > > > > > > Well, IMO it is better to prevent people from passing unrecognized > > > flags to device_link_add() at all, even if that means some extra > > > effort when adding new flags. > > > > It isn't so much the extra effort that's a concern, but people might > > miss updating whatever grouping macro we use. > > > > > > > > I'll post an alternative fix shortly. > > > > You might want to move the MANAGED_FLAGs and other grouping macros > > into the header file then? So that if someone is adding new flags, > > it'll be less likely they'll forget to update the grouping macro? > > They would need to update device_link_add() itself, so updating a > thing next to it does't seem to be so much of an issue. > > Moreover, the "grouping macro" is not relevant for any users of the > API, just for device_link_add() itself, so I'm not sure how much > better it is to have it in the header. > > And, of course, if anyone forgets to update the "grouping macro", they > will find that the new flags are rejected immediately. Sounds good. -Saravana
