Re: [PATCH v10 1/6] net: wwan: t9xx: Add PCIe core

["Wu. JackBB (GSM)" <JackBB_Wu@compal.com>]

Hi Loic,

Thanks for the review.

On Wed, Oct 14, 2026 at 8:49 PM Loic Poulain
<loic.poulain@oss.qualcomm.com> wrote:
> > +#include <linux/dma-mapping.h>
> > +#include <linux/dmapool.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/sched.h>
> > +#include <linux/slab.h>
> > +#include <linux/spinlock.h>
>
> Looks like a lot of headers for such a small header file. Could you
> trim the unnecessary includes? For example, spinlock.h doesn't appear
> to be used.

Will fix in v11.  All seven go: the header only uses BIT() and u32, so
<linux/bits.h> and <linux/types.h> are left, plus a forward declaration
for struct device.

> > +#define SET_HW_BITS(dest, chs, mhccif, dev)		\
> > +	({						\
> > +		if ((chs) & (dev))					\
> > +			(dest) |= FIELD_PREP(mhccif, 1);		\
> > +	})
>
> A static inline helper would be cleaner.

Will fix in v11:

  static inline u32 mtk_pci_map_evt_bit(u32 val, u32 from, u32 to)
  {
	return (val & from) ? to : 0;
  }

Every mask involved is a single bit, so FIELD_PREP(mask, 1) was just
mask.  The same helper also replaces the FIELD_PREP(FIELD_GET()) pairs
in mtk_pci_ext_d2h_evt_chs(), which is this translation in the other
direction.

Thanks,
Jack