On Wed, Jun 28, 2023 at 3:35 PM Mukesh Ojha <quic_mojha@xxxxxxxxxxx> wrote: > > Add qcom_minidump_smem module in a preparation to remove smem > based minidump specific code from driver/remoteproc/qcom_common.c > and provide needed exported API, this abstract minidump specific > data layout from qualcomm's remoteproc driver. ... > +#include <linux/kernel.h> Why? Missing headers: err.h export.h string.h types.h byteorder/generic.h > +#include <linux/module.h> > +#include <linux/io.h> Can you have them ordered? ... > + * Return: On success, it returns iomapped base segment address, otherwise NULL on error. IO mapped (or MMIO?) ... > + return ioremap((unsigned long)le64_to_cpu(subsystem->regions_baseptr), Why casting? > + *seg_cnt * sizeof(struct minidump_region)); Something from overflow.h? ... > + /* If it is not valid region, skip it */ if region is not valid, skip it ... > +#ifndef _QCOM_MINIDUMP_H_ > +#define _QCOM_MINIDUMP_H_ This header uses EINVAL, IS_ENABLED() and (to some extent) dma_addr_t. So do you need respective headers to be included? > +#endif /* _QCOM_MINIDUMP_H_ */ -- With Best Regards, Andy Shevchenko