On Thu, 12 Jun 2025 21:33:44 +0200 Anders Roxell <anders.roxell@xxxxxxxxxx> wrote: > On 2025-06-12 17:21, Johannes Berg wrote: > > > > > > diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info.c b/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info.c > > > index cb36baac14da..1854d071aff2 100644 > > > --- a/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info.c > > > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info.c > > > @@ -204,9 +204,10 @@ int iwl_pcie_ctxt_info_init(struct iwl_trans *trans, > > > > > > WARN_ON(RX_QUEUE_CB_SIZE(iwl_trans_get_num_rbds(trans)) > 12); > > > control_flags = IWL_CTXT_INFO_TFD_FORMAT_LONG; > > > - control_flags |= > > > - u32_encode_bits(RX_QUEUE_CB_SIZE(iwl_trans_get_num_rbds(trans)), > > > - IWL_CTXT_INFO_RB_CB_SIZE); > > > + /* This should just be u32_encode_bits() but gcc-8 and gcc-9 fail to build */ > > > + control_flags |= FIELD_PREP(IWL_CTXT_INFO_RB_CB_SIZE, > > > + RX_QUEUE_CB_SIZE(iwl_trans_get_num_rbds(trans)) & > > > + FIELD_MAX(IWL_CTXT_INFO_RB_CB_SIZE)); > > > > > > > The coding style is really confusing now though - what are arguments to > > the macro and all that. > > Would it help if I indent like this? > > diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info.c b/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info.c > index cb36baac14da..5bb81ed7db79 100644 > --- a/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info.c > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info.c > @@ -204,9 +204,10 @@ int iwl_pcie_ctxt_info_init(struct iwl_trans *trans, > > WARN_ON(RX_QUEUE_CB_SIZE(iwl_trans_get_num_rbds(trans)) > 12); > control_flags = IWL_CTXT_INFO_TFD_FORMAT_LONG; > - control_flags |= > - u32_encode_bits(RX_QUEUE_CB_SIZE(iwl_trans_get_num_rbds(trans)), > - IWL_CTXT_INFO_RB_CB_SIZE); > + /* This should just be u32_encode_bits() but gcc-8 and gcc-9 fail to build */ > + control_flags |= FIELD_PREP(IWL_CTXT_INFO_RB_CB_SIZE, > + RX_QUEUE_CB_SIZE(iwl_trans_get_num_rbds(trans)) & > + FIELD_MAX(IWL_CTXT_INFO_RB_CB_SIZE)); I would always indent the 'continued' part of an arithmetic expression. Otherwise it looks (in this case) like another argument to FIELD_PREP(). (Not helped by the coding style requiring the '&' on the end of the line rather than the start of the continuation line.) David > control_flags |= u32_encode_bits(rb_size, IWL_CTXT_INFO_RB_SIZE); > ctxt_info->control.control_flags = cpu_to_le32(control_flags); > > > Cheers, > Anders >