On Wed, Jan 08, 2025 at 07:13:34PM -0800, Tushar Dave wrote: > > Yes, but X in config_acs should copy the FW value not the value > > modified by disable_acs_redir_param > > I see your point. In that case (for the last hunk in my patch) something > like this should work IMO. > > - /* If mask is 0 then we copy the bit from the firmware setting. */ > - caps->ctrl = (caps->ctrl & ~mask) | (caps->fw_ctrl & mask); > - caps->ctrl |= flags; > + /* For unchanged ACS bits 'x' or 'X', copy the bits from the > firmware setting. */ > + if (!acs_mask) > + caps->ctrl = caps->fw_ctrl; > + > + caps->ctrl &= ~mask; > + caps->ctrl |= (flags & mask); > > Wish I can have better condition check instead of 'if (!acs_mask)' but let > me know your thoughts. It should be per-bit surely? I think the original logic was the right idea, just the bit logic had the wrong operators.. Jason
