[PATCH v10 1/6] net: wwan: t9xx: Add PCIe core
Wu. JackBB (GSM)
JackBB_Wu at compal.com
Thu Oct 15 02:12:17 PDT 2026
Hi Loic,
Thanks for the review.
On Wed, Oct 14, 2026 at 8:49 PM Loic Poulain
<loic.poulain at 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
More information about the Linux-mediatek
mailing list