[PATCH v9 1/3] MTD : add the common code for GPMI-NAND controller driver
Huang Shijie
b32955 at freescale.com
Mon Aug 22 00:59:11 EDT 2011
Hi,
thanks for your comments.
>> +
>> +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.
>> +}
>> +
>> +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.
>> + 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.
>> + 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.
>> +
>> +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?
>
>> + }
>> + }
>> + 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.
>> + */
>> +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.
>> + if (gpmi_debug& GPMI_DEBUG_CRAZY) {
> GPMI_DEBUG_CRAZY should probably be GPMI_DEBUG_VERBOSE ?
>
ok, thanks
>> + struct bch_geometry *geo =&this->bch_geometry;
>> +
>> + gpmi_show_regs(this);
>> + show_bch_geometry(geo);
>> + panic("-----------BCH FAILED------------------");
> dev_err()
>
>> + }
>> + }
>> + 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
>
>> +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.
>> +
>> + 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 {}.
>> + 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
>> +
>> + /*
>> + * 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.
>> 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.
>> +#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
More information about the linux-arm-kernel
mailing list