On Thu, 2015-06-18 at 09:49 +0900, Taku Izumi wrote: > This patch adds hardware initialization routine to be > invoked at driver's .probe routine. Trivial notes: Please run all your patches through scripts/checkpatch.pl and fix whatever messages it emits as you think appropriate. > diff --git a/drivers/platform/x86/fjes/fjes_hw.c b/drivers/platform/x86/fjes/fjes_hw.c [] > +/* supported MTU list */ > +u32 fjes_support_mtu[] = { > + FJES_MTU_DEFINE(8 * 1024), > + FJES_MTU_DEFINE(16 * 1024), > + FJES_MTU_DEFINE(32 * 1024), > + FJES_MTU_DEFINE(64 * 1024), > + 0 > +}; Maybe these should be const? > +static u8 *fjes_hw_iomap(struct fjes_hw *hw) > +{ > + u8 *base; > + > + if (!request_mem_region(hw->hw_res.start, hw->hw_res.size, > + fjes_driver_name)) { > + pr_err("request_mem_region failed"); Please make sure all pr_<level> logging messages end with a "\n" so that interleaving by other process threads can't happen. > +static int fjes_hw_setup(struct fjes_hw *hw) > +{ [] > + mem_size = sizeof(struct ep_share_mem_info) * (hw->max_epid); > + buf = kzalloc(mem_size, GFP_KERNEL); kcalloc? [] > + memset((void *)¶m, 0, sizeof(param)); Unnecessary cast > diff --git a/drivers/platform/x86/fjes/fjes_hw.h b/drivers/platform/x86/fjes/fjes_hw.h [] > +#define FJES_DEVICE_RESET_TIMEOUT ((17 + 1) * 3) /* sec */ 48 second timeout for a device reset? > +/* Frame & MTU */ > +struct esmem_frame_t { > + __le32 frame_size; > + u8 frame_data[]; > +}; Using a _t suffix for a struct type can be confusing. [] > + struct _ep_buffer_info_common_t { > + u32 version; > + } common; > + > + struct _ep_buffer_info_v1_t { > diff --git a/drivers/platform/x86/fjes/fjes_regs.h b/drivers/platform/x86/fjes/fjes_regs.h > +/* Interrupt Control registers */ > +#define XSCT_IMS 0x0084 /* Interrupt mas set */ mask > +#define XSCT_IMC 0x0088 /* Interrupt mask clear */ -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html