Hi,

thanks for your comments.

quoted

+
+static inline int get_ecc_chunk_size(struct gpmi_nand_data *this)
+{
+	/* for historical reason */
+	return 512;

Can't we just #define this? Or will there ever be something else possible here ?
I thought this is the only possible behaviour on MXS.

Please keep it here, it maybe changed in the future.
It ever had some code for ONFI nand, but i removed it.

quoted

+}
+
+int common_nfc_set_geometry(struct gpmi_nand_data *this)
+{
+	struct bch_geometry *geo =&this->bch_geometry;
+	struct mtd_info *mtd =&this->mil.mtd;
+	unsigned int metadata_size;
+	unsigned int status_size;
+	unsigned int chunk_data_size_in_bits;
+	unsigned int chunk_ecc_size_in_bits;
+	unsigned int chunk_total_size_in_bits;
+	unsigned int block_mark_chunk_number;
+	unsigned int block_mark_chunk_bit_offset;
+	unsigned int block_mark_bit_offset;
+	int gf_len = 13;/* use GP13 by default */
+
+	/* We only support BCH now. */
+	geo->ecc_algorithm = "BCH";
+
+	/*
+	 * We always choose a metadata size of 10. Don't try to make sense of
+	 * it -- this is really only for historical compatibility.
+	 */

Historical compat or you mean "the chip was designed this way, see datasheet
section x.y.z"? ;-)

Just for historical compatibility.
it's better to keep it as now, there is no need to change it.

quoted

+	geo->metadata_size_in_bytes = 10;
+
+	/* ECC chunks */
+	geo->ecc_chunk_size_in_bytes = get_ecc_chunk_size(this);
+
+	/*
+	 * Compute the total number of ECC chunks in a page. This includes the
+	 * slightly larger chunk at the beginning of the page, which contains
+	 * both data and metadata.
+	 */
+	geo->ecc_chunk_count = mtd->writesize / geo->ecc_chunk_size_in_bytes;
+
+	/*
+	 * We use the same ECC strength for all chunks, including the first one.
+	 */
+	geo->ecc_strength = get_ecc_strength(this);
+	if (!geo->ecc_strength) {
+		pr_info("Page size:%d, OOB:%d\n", mtd->writesize, mtd->oobsize);
+		return -EINVAL;
+	}
+
+	/* Compute the page size, include page and oob. */
+	geo->page_size_in_bytes = mtd->writesize + mtd->oobsize;
+	geo->payload_size_in_bytes = mtd->writesize;
+
+	/*
+	 * In principle, computing the auxiliary buffer geometry is NFC
+	 * version-specific. However, at this writing, all versions share the
+	 * same model, so this code can also be shared.
+	 *
+	 * The auxiliary buffer contains the metadata and the ECC status. The
+	 * metadata is padded to the nearest 32-bit boundary. The ECC status
+	 * contains one byte for every ECC chunk, and is also padded to the
+	 * nearest 32-bit boundary.
+	 */
+	metadata_size = ALIGN(geo->metadata_size_in_bytes, 4);
+	status_size   = ALIGN(geo->ecc_chunk_count, 4);
+
+	geo->auxiliary_size_in_bytes = metadata_size + status_size;
+	geo->auxiliary_status_offset = metadata_size;
+
+	/* Check if we're going to do block mark swapping. */
+	if (!this->swap_block_mark)
+		return 0;
+
+	/*
+	 * If control arrives here, we're doing block mark swapping, so we need
+	 * to compute the byte and bit offsets of the physical block mark within
+	 * the ECC-based view of the page data. In principle, this isn't a
+	 * difficult computation -- but it's very important and it's easy to get
+	 * it wrong, so we do it carefully.
+	 *
+	 * Note that this calculation is simpler because we use the same ECC
+	 * strength for all chunks, including the zero'th one, which contains
+	 * the metadata. The calculation would be slightly more complicated
+	 * otherwise.
+	 *
+	 * We start by computing the physical bit offset of the block mark. We
+	 * then subtract the number of metadata and ECC bits appearing before
+	 * the mark to arrive at its bit offset within the data alone.
+	 */
+
+	/* Compute some important facts about chunk geometry. */
+	chunk_data_size_in_bits = geo->ecc_chunk_size_in_bytes * 8;
+	chunk_ecc_size_in_bits  = geo->ecc_strength * gf_len;
+	chunk_total_size_in_bits = chunk_data_size_in_bits
+					+ chunk_ecc_size_in_bits;
+
+	/* Compute the bit offset of the block mark within the physical page. */
+	block_mark_bit_offset = mtd->writesize * 8;
+
+	/* Subtract the metadata bits. */
+	block_mark_bit_offset -= geo->metadata_size_in_bytes * 8;
+
+	/*
+	 * Compute the chunk number (starting at zero) in which the block mark
+	 * appears.
+	 */
+	block_mark_chunk_number =
+			block_mark_bit_offset / chunk_total_size_in_bits;
+
+	/*
+	 * Compute the bit offset of the block mark within its chunk, and
+	 * validate it.
+	 */
+	block_mark_chunk_bit_offset =
+		block_mark_bit_offset -
+			(block_mark_chunk_number * chunk_total_size_in_bits);
+
+	if (block_mark_chunk_bit_offset>  chunk_data_size_in_bits) {
+		/*
+		 * If control arrives here, the block mark actually appears in
+		 * the ECC bits of this chunk. This wont' work.
+		 */
+		pr_info("Unsupported page geometry : %u:%u\n",
+				mtd->writesize, mtd->oobsize);
+		return -EINVAL;
+	}
+
+	/*
+	 * Now that we know the chunk number in which the block mark appears,
+	 * we can subtract all the ECC bits that appear before it.
+	 */
+	block_mark_bit_offset -=
+			block_mark_chunk_number * chunk_ecc_size_in_bits;
+
+	/*
+	 * We now know the absolute bit offset of the block mark within the
+	 * ECC-based data. We can now compute the byte offset and the bit
+	 * offset within the byte.
+	 */
+	geo->block_mark_byte_offset = block_mark_bit_offset / 8;
+	geo->block_mark_bit_offset  = block_mark_bit_offset % 8;
+
+	return 0;
+}
+
+struct dma_chan *get_dma_chan(struct gpmi_nand_data *this)
+{
+	int chip = this->mil.current_chip;
+
+	BUG_ON(chip<  0);
+	return this->dma_chans[chip];
+}
+
+/* Can we use the upper's buffer directly for DMA? */
+void prepare_data_dma(struct gpmi_nand_data *this, enum dma_data_direction
dr) +{
+	struct mil *mil =&this->mil;
+	struct scatterlist *sgl =&mil->data_sgl;

I still see the "MIL" present -- can't we just merge the library and this ?

the   `mil` is just a data structure to contain the fields now.
Maybe I should change the name of it.

quoted

+	int ret;
+
+	mil->direct_dma_map_ok = true;
+
+	/* first try to map the upper buffer directly */
+	sg_init_one(sgl, mil->upper_buf, mil->upper_len);
+	ret = dma_map_sg(this->dev, sgl, 1, dr);
+	if (ret == 0) {
+		/* We have to use our own DMA buffer. */
+		sg_init_one(sgl, mil->data_buffer_dma, PAGE_SIZE);
+
+		if (dr == DMA_TO_DEVICE)
+			memcpy(mil->data_buffer_dma, mil->upper_buf,
+				mil->upper_len);
+
+		ret = dma_map_sg(this->dev, sgl, 1, dr);
+		BUG_ON(ret == 0);
+
+		mil->direct_dma_map_ok = false;
+	}
+}
+
+/* This will be called after the DMA operation is finished. */
+static void dma_irq_callback(void *param)
+{
+	struct gpmi_nand_data *this = param;
+	struct mil *mil =&this->mil;
+	struct completion *dma_c =&this->dma_done;
+
+	complete(dma_c);
+
+	switch (this->dma_type) {
+	case DMA_FOR_COMMAND:
+		dma_unmap_sg(this->dev,&mil->cmd_sgl, 1, DMA_TO_DEVICE);
+		break;
+
+	case DMA_FOR_READ_DATA:
+		dma_unmap_sg(this->dev,&mil->data_sgl, 1, DMA_FROM_DEVICE);
+		if (mil->direct_dma_map_ok == false)
+			memcpy(mil->upper_buf, mil->data_buffer_dma,
+				mil->upper_len);
+		break;
+
+	case DMA_FOR_WRITE_DATA:
+		dma_unmap_sg(this->dev,&mil->data_sgl, 1, DMA_TO_DEVICE);
+		break;
+
+	case DMA_FOR_READ_ECC_PAGE:
+	case DMA_FOR_WRITE_ECC_PAGE:
+		/* We have to wait the BCH interrupt to finish. */
+		break;
+
+	default:
+		BUG();
+	}
+}
+
+static void show_bch_geometry(struct bch_geometry *geo)
+{
+	pr_info("---------------------------------------\n");
+	pr_info("	BCH Geometry\n");
+	pr_info("---------------------------------------\n");
+	pr_info("ECC Algorithm          : %s\n", geo->ecc_algorithm);
+	pr_info("ECC Strength           : %u\n", geo->ecc_strength);
+	pr_info("Page Size in Bytes     : %u\n", geo->page_size_in_bytes);
+	pr_info("Metadata Size in Bytes : %u\n", geo->metadata_size_in_bytes);
+	pr_info("ECC Chunk Size in Bytes: %u\n", geo->ecc_chunk_size_in_bytes);
+	pr_info("ECC Chunk Count        : %u\n", geo->ecc_chunk_count);
+	pr_info("Payload Size in Bytes  : %u\n", geo->payload_size_in_bytes);
+	pr_info("Auxiliary Size in Bytes: %u\n", geo->auxiliary_size_in_bytes);
+	pr_info("Auxiliary Status Offset: %u\n", geo->auxiliary_status_offset);
+	pr_info("Block Mark Byte Offset : %u\n", geo->block_mark_byte_offset);
+	pr_info("Block Mark Bit Offset  : %u\n", geo->block_mark_bit_offset);
+}

We don't need this.

I just use it for debug.
Why do not need it? :)
I think it's useful to debug.

quoted

+
+int start_dma_without_bch_irq(struct gpmi_nand_data *this,
+				struct dma_async_tx_descriptor *desc)
+{
+	struct completion *dma_c =&this->dma_done;
+	int err;
+
+	init_completion(dma_c);
+
+	desc->callback		= dma_irq_callback;
+	desc->callback_param	= this;
+	dmaengine_submit(desc);
+
+	/* Wait for the interrupt from the DMA block. */
+	err = wait_for_completion_timeout(dma_c, msecs_to_jiffies(1000));
+	err = (!err) ? -ETIMEDOUT : 0;
+	if (err) {
+		pr_info("DMA timeout, last DMA :%d\n", this->last_dma_type);
+		if (gpmi_debug&  GPMI_DEBUG_CRAZY) {
+			struct bch_geometry *geo =&this->bch_geometry;
+
+			gpmi_show_regs(this);
+			show_bch_geometry(geo);
+			panic("-----------DMA FAILED------------------");

No, dev_err() or something.
\

ok, no problem.

Also, I don't like you using pr_ stuff, I think you can use dev_ stuff, can't you?

quoted

+		}
+	}
+	return err;
+}
+
+/*
+ * This function is used in BCH reading or BCH writing pages.
+ * It will wait for the BCH interrupt as long as ONE second.
+ * Actually, we must wait for two interrupts :
+ *	[1] firstly the DMA interrupt and
+ *	[2] secondly the BCH interrupt.
+ *
+ * @this:	Per-device data structure.
+ * @desc:	DMA channel

Does this conform to kerneldoc ?

it's redundant, i will remove the description for the parameters.
But keep the description for the function.

quoted

+ */
+int start_dma_with_bch_irq(struct gpmi_nand_data *this,
+			struct dma_async_tx_descriptor *desc)
+{
+	int err;
+
+	/* Prepare to receive an interrupt from the BCH block. */
+	init_completion(&this->bch_done);
+
+	/* start the DMA */
+	start_dma_without_bch_irq(this, desc);
+
+	/* Wait for the interrupt from the BCH block. */
+	err = wait_for_completion_timeout(&this->bch_done,
+					msecs_to_jiffies(1000));
+	err = (!err) ? -ETIMEDOUT : 0;
+	if (err) {
+		pr_info("BCH timeout!!!\n");

One ! is enough!!!

ok.

quoted

+		if (gpmi_debug&  GPMI_DEBUG_CRAZY) {

GPMI_DEBUG_CRAZY should probably be GPMI_DEBUG_VERBOSE ?

ok, thanks

quoted

+			struct bch_geometry *geo =&this->bch_geometry;
+
+			gpmi_show_regs(this);
+			show_bch_geometry(geo);
+			panic("-----------BCH FAILED------------------");

dev_err()

quoted

+		}
+	}
+	return err;
+}
+
+static int __devinit acquire_register_block(struct gpmi_nand_data *this,
+			const char *resource_name, void **reg_block_base)
+{
+	struct platform_device *pdev = this->pdev;
+	struct resource *r;
+	void *p;
+
+	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, resource_name);
+	if (!r) {
+		pr_info("Can't get resource for %s\n", resource_name);
+		return -ENXIO;
+	}
+
+	/* remap the register block */
+	p = ioremap(r->start, resource_size(r));
+	if (!p) {
+		pr_info("Can't remap %s\n", resource_name);
+		return -ENOMEM;
+	}
+
+	*reg_block_base = p;
+	return 0;
+}
+
+static void release_register_block(struct gpmi_nand_data *this,
+				void *reg_block_base)
+{
+	iounmap(reg_block_base);
+}
+
+static int __devinit acquire_interrupt(struct gpmi_nand_data *this,
+			const char *resource_name,
+			irq_handler_t interrupt_handler, int *lno, int *hno)
+{
+	struct platform_device *pdev = this->pdev;
+	struct resource *r;
+	int err;
+
+	r = platform_get_resource_byname(pdev, IORESOURCE_IRQ, resource_name);
+	if (!r) {
+		pr_info("Can't get resource for %s\n", resource_name);
+		return -ENXIO;
+	}
+
+	BUG_ON(r->start != r->end);
+	err = request_irq(r->start, interrupt_handler, 0, resource_name, this);
+	if (err) {
+		pr_info("Can't own %s\n", resource_name);
+		return err;
+	}
+
+	*lno = r->start;
+	*hno = r->end;
+	return 0;
+}
+

+
+static int __devinit acquire_resources(struct gpmi_nand_data *this)
+{
+	struct resources *r =&this->resources;
+	int error;
+
+	/* Attempt to acquire the GPMI register block. */
+	error = acquire_register_block(this,
+				GPMI_NAND_GPMI_REGS_ADDR_RES_NAME,
+				&r->gpmi_regs);

You're already passing "this", why pass r->gpmi_regs? Please fix globally.

ok, thanks

quoted

+static int gpmi_dev_ready(struct mtd_info *mtd)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct gpmi_nand_data *this = nand->priv;
+	struct mil *mil =&this->mil;
+
+	return gpmi_is_ready(this, mil->current_chip);
+}
+
+static void gpmi_select_chip(struct mtd_info *mtd, int chip)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct gpmi_nand_data *this = nand->priv;
+	struct mil *mil =&this->mil;
+
+	if ((mil->current_chip<  0)&&  (chip>= 0))
+		gpmi_begin(this);
+	else if ((mil->current_chip>= 0)&&  (chip<  0))
+		gpmi_end(this);
+	else
+		;

Do you need this else branch at all?

It needs a warning here.

quoted

+
+	mil->current_chip = chip;
+}
+
+static void gpmi_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct gpmi_nand_data *this = nand->priv;
+	struct mil *mil =&this->mil;
+
+	logio(GPMI_DEBUG_READ);
+	/* save the info in mil{} for future */
+	mil->upper_buf	= buf;
+	mil->upper_len	= len;
+
+	gpmi_read_data(this);
+}
+
+static void gpmi_write_buf(struct mtd_info *mtd, const uint8_t *buf, int
len) +{
+	struct nand_chip *nand = mtd->priv;
+	struct gpmi_nand_data *this = nand->priv;
+	struct mil *mil =&this->mil;
+
+	logio(GPMI_DEBUG_WRITE);
+	/* save the info in mil{} for future */
+	mil->upper_buf	= (uint8_t *)buf;
+	mil->upper_len	= len;
+
+	gpmi_send_data(this);
+}
+
+static uint8_t gpmi_read_byte(struct mtd_info *mtd)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct gpmi_nand_data *this = nand->priv;
+	struct mil *mil =&this->mil;
+	uint8_t *buf = mil->data_buffer_dma;
+
+	gpmi_read_buf(mtd, buf, 1);
+	return buf[0];
+}
+

+static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct gpmi_nand_data *this = nand->priv;
+	int block, ret = 0;
+
+	/* Get block number */
+	block = (int)(ofs>>  nand->bbt_erase_shift);
+	if (nand->bbt)
+		nand->bbt[block>>  2] |= 0x01<<  ((block&  0x03)<<  1);
+
+	/* Do we have a flash based bad block table ? */
+	if (nand->options&  NAND_USE_FLASH_BBT)
+		ret = nand_update_bbt(mtd, ofs);

if (stuff)
	return nand_update_bbt();

I can not return it here, I need to update the ecc_stats too.

stuff from else branch
.
.
.

Besides, please don't declare variables in the middle of code.

why?
it's no harm to declare the variables in the {}.

quoted

+	else {
+		struct mil *mil	=&this->mil;
+		uint8_t *block_mark;
+		int column, page, status, chipnr;
+
+		chipnr = (int)(ofs>>  nand->chip_shift);
+		nand->select_chip(mtd, chipnr);
+
+		column = this->swap_block_mark ? mtd->writesize : 0;
+
+		/* Write the block mark. */
+		block_mark = mil->data_buffer_dma;
+		block_mark[0] = 0; /* bad block marker */
+
+		/* Shift to get page */
+		page = (int)(ofs>>  nand->page_shift);
+
+		nand->cmdfunc(mtd, NAND_CMD_SEQIN, column, page);
+		nand->write_buf(mtd, block_mark, 1);
+		nand->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+
+		status = nand->waitfunc(mtd, nand);
+		if (status&  NAND_STATUS_FAIL)
+			ret = -EIO;
+
+		nand->select_chip(mtd, -1);
+	}
+	if (!ret)
+		mtd->ecc_stats.badblocks++;
+
+	return ret;
+}
+
+static int __devinit nand_boot_set_geometry(struct gpmi_nand_data *this)
+{
+	struct boot_rom_geometry *geometry =&this->rom_geometry;
+
+	/*
+	 * Set the boot block stride size.
+	 *
+	 * In principle, we should be reading this from the OTP bits, since
+	 * that's where the ROM is going to get it. In fact, we don't have any
+	 * way to read the OTP bits, so we go with the default and hope for the
+	 * best.
+	 */
+	geometry->stride_size_in_pages = 64;
+
+	/*
+	 * Set the search area stride exponent.
+	 *
+	 * In principle, we should be reading this from the OTP bits, since
+	 * that's where the ROM is going to get it. In fact, we don't have any
+	 * way to read the OTP bits, so we go with the default and hope for the
+	 * best.
+	 */
+	geometry->search_area_stride_exponent = 2;
+
+	if (gpmi_debug&  GPMI_DEBUG_INIT)
+		pr_info("stride size in page : %d, search areas : %d\n",
+			geometry->stride_size_in_pages,
+			geometry->search_area_stride_exponent);
+	return 0;
+}
+
+static const char  *fingerprint = "STMP";
+static int __devinit mx23_check_transcription_stamp(struct gpmi_nand_data
*this) +{
+	struct boot_rom_geometry *rom_geo =&this->rom_geometry;
+	struct mil *mil =&this->mil;
+	struct mtd_info *mtd =&mil->mtd;
+	struct nand_chip *nand =&mil->nand;
+	unsigned int search_area_size_in_strides;
+	unsigned int stride;
+	unsigned int page;
+	loff_t byte;
+	uint8_t *buffer = nand->buffers->databuf;
+	int saved_chip_number;
+	int found_an_ncb_fingerprint = false;
+
+	/* Compute the number of strides in a search area. */
+	search_area_size_in_strides = 1<<  rom_geo->search_area_stride_exponent;
+
+	/* Select chip 0. */
+	saved_chip_number = mil->current_chip;
+	nand->select_chip(mtd, 0);
+
+	/*
+	 * Loop through the first search area, looking for the NCB fingerprint.
+	 */
+	pr_info("Scanning for an NCB fingerprint...\n");
+
+	for (stride = 0; stride<  search_area_size_in_strides; stride++) {
+		/* Compute the page and byte addresses. */
+		page = stride * rom_geo->stride_size_in_pages;
+		byte = page   * mtd->writesize;
+
+		pr_info("  Looking for a fingerprint in page 0x%x\n", page);

pr_info? Why, who cares, I'd prefer dev_dbg()?

thanks

quoted

+
+		/*
+		 * Read the NCB fingerprint. The fingerprint is four bytes long
+		 * and starts in the 12th byte of the page.
+		 */
+		nand->cmdfunc(mtd, NAND_CMD_READ0, 12, page);
+		nand->read_buf(mtd, buffer, strlen(fingerprint));
+
+		/* Look for the fingerprint. */
+		if (!memcmp(buffer, fingerprint, strlen(fingerprint))) {
+			found_an_ncb_fingerprint = true;
+			break;
+		}
+
+	}
+
+	/* Deselect chip 0. */
+	nand->select_chip(mtd, saved_chip_number);
+
+	if (found_an_ncb_fingerprint)
+		pr_info("  Found a fingerprint\n");
+	else
+		pr_info("  No fingerprint found\n");
+	return found_an_ncb_fingerprint;
+}
+
+/* Writes a transcription stamp. */
+static int __devinit mx23_write_transcription_stamp(struct gpmi_nand_data
*this) +{
+	struct device *dev = this->dev;
+	struct boot_rom_geometry *rom_geo =&this->rom_geometry;
+	struct mil *mil =&this->mil;
+	struct mtd_info *mtd =&mil->mtd;
+	struct nand_chip *nand =&mil->nand;
+	unsigned int block_size_in_pages;
+	unsigned int search_area_size_in_strides;
+	unsigned int search_area_size_in_pages;
+	unsigned int search_area_size_in_blocks;
+	unsigned int block;
+	unsigned int stride;
+	unsigned int page;
+	loff_t       byte;
+	uint8_t      *buffer = nand->buffers->databuf;
+	int saved_chip_number;
+	int status;
+
+	/* Compute the search area geometry. */
+	block_size_in_pages = mtd->erasesize / mtd->writesize;
+	search_area_size_in_strides = 1<<  rom_geo->search_area_stride_exponent;
+	search_area_size_in_pages = search_area_size_in_strides *
+					rom_geo->stride_size_in_pages;
+	search_area_size_in_blocks =
+		  (search_area_size_in_pages + (block_size_in_pages - 1)) /
+				    block_size_in_pages;
+
+	pr_info("-------------------------------------------\n");
+	pr_info("Search Area Geometry\n");
+	pr_info("-------------------------------------------\n");
+	pr_info("Search Area in Blocks : %u\n", search_area_size_in_blocks);
+	pr_info("Search Area in Strides: %u\n", search_area_size_in_strides);
+	pr_info("Search Area in Pages  : %u\n", search_area_size_in_pages);

Maybe if you debug it, yes ... but I certainly don't want such ascii-art in my
log during normal operation.

ok.

quoted

addr_t auxiliary);
+
+/* ONFI or TOGGLE nand */
+bool is_ddr_nand(struct gpmi_nand_data *);
+
+/* for log */
+extern int gpmi_debug;

Why this extern ?

this header can be included by gpmi-nand.c and gpmi-lib.c.

quoted

+#define GPMI_DEBUG_INIT		0x0001
+#define GPMI_DEBUG_READ		0x0002
+#define GPMI_DEBUG_WRITE	0x0004
+#define GPMI_DEBUG_ECC_READ	0x0008
+#define GPMI_DEBUG_ECC_WRITE	0x0010
+#define GPMI_DEBUG_CRAZY	0x0020
+
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) "[ %s : %.3d ] " fmt, __func__, __LINE__
+
+#define logio(level)				\
+		do {				\
+			if (gpmi_debug&  level)	\
+				pr_info("\n");	\
+		} while (0)

Do you really need this ?

not really.

thanks

Huang Shijie