On Thu 04 Apr 2024 at 23:13, Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx> wrote: > Hi Jerome > > Thank you for your feedback > >> playback_only and capture_only have implication on each other. If one is >> set, the other can/must not be set. This leads to conditions which can >> be fairly hard to read and possibly bugs. > (snip) >> Wouldn't it be better to replace those 2 flags with a single bitfield ? >> >> something like: >> >> unsigned int directions; >> #define PLAYBACK_VALID BIT(0) >> #define CAPTURE_VALID BIT(1) > > I think Amadeusz indicated similar idea, and basically I can agree about > it. I've seen it afterward. It is similar indeed but I don't think 'None' or 'Both' should have a dedicated bit. That would be yet another redundance/implication between flags/bits ... so another source of bugs/complexity IMO. > But in this patch-set, I want focus to removing dpcm_xxx flag as 1st > step. So I'm happy to create such patch-set, but I want to handle it as > another patch-set. Fine by me ... at least for the Amlogic part. > > Thank you for your help !! > Thanks for your work ! > Best regards > --- > Renesas Electronics > Ph.D. Kuninori Morimoto -- Jerome
