On 23.10.2023 11:20, Zhenhua Huang wrote: > Qualcomm memory dump driver initializes system memory dump table. > Firmware dumps system cache, internal memory, peripheral registers > to DDR as per this table after system crashes and warm resets. The > driver reserves memory, populates ids and sizes for firmware dumping > according to the configuration. > > Signed-off-by: Zhenhua Huang <quic_zhenhuah@xxxxxxxxxxx> > --- [...] > +#define MAX_NUM_ENTRIES 0x150 The number of entries makes more sense as a dec number > +#define QCOM_DUMP_MAKE_VERSION(major, minor) (((major) << 20) | (minor)) > +#define QCOM_DUMP_TABLE_VERSION QCOM_DUMP_MAKE_VERSION(2, 0) I feel like doing this: #define QCOM_DUMP_TABLE_VERSION(major, minor) ((major << 20) | (minor)) ... someval = QCOM_DUMP_TABLE_VERSION(2, 0) would make more sense, since v2.0 seems to be the only supported target.. [...] > + if (phys_addr > phys_end_addr) { > + dev_err_probe(dev, -ENOMEM, "Exceeding allocated region\n"); > + return -ENOMEM; > + } > + } else { > + continue; You can check for the inverse and bail out early, saving yourself a lot of tabs [...] > +MODULE_DESCRIPTION("Memory Dump Driver"); Missing some mention of it being QC specific Konrad