All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Koeller <thomas.koeller@baslerweb.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-mtd@lists.infradead.org, linux-mips@linux-mips.org
Subject: Re: [PATCH] eXcite nand flash driver
Date: Sat, 10 Feb 2007 11:22:04 +0100	[thread overview]
Message-ID: <200702101122.05049.thomas.koeller@baslerweb.com> (raw)
In-Reply-To: <1170949737.3646.29.camel@chaos>

On Donnerstag, 8. Februar 2007, Thomas Gleixner wrote:
> > +/* command and control functions */
> > +static void excite_nand_control(struct mtd_info *mtd, int cmd,
> > +				       unsigned int ctrl)
> > +{
> > +	io_reg_t regs =
> > +	    container_of(mtd, struct excite_nand_drvdata, board_mtd)->regs;
> > +	static void __iomem *tgt = NULL;
> > +
> > +	switch (ctrl) {
> > +	case NAND_CTRL_CHANGE | NAND_CTRL_CLE:
> > +		tgt = regs + EXCITE_NANDFLASH_CMD_BYTE;
> > +		break;
> > +	case NAND_CTRL_CHANGE | NAND_CTRL_ALE:
> > +		tgt = regs + EXCITE_NANDFLASH_ADDR_BYTE;
> > +		break;
> > +	case NAND_CTRL_CHANGE | NAND_NCE:
> > +		tgt = regs + EXCITE_NANDFLASH_DATA_BYTE;
> > +		break;
> > +	}
>
> Err, did this ever work ? I doubt it. From nand_base.c:
>
>                 chip->cmd_ctrl(mtd, page_addr, ctrl);
>                 ctrl &= ~NAND_CTRL_CHANGE;
>                 chip->cmd_ctrl(mtd, page_addr >> 8, ctrl);
>
> So I expect an OOPS happens on a regular base.
>

I guess it is the 'static void __iomem *tgt = NULL' part that worries
you? Think about it, that value is never used.

However, I admit it is somewhat unclean, and therefore I am changing
it. Updated patch follows.

tk

WARNING: multiple messages have this Message-ID (diff)
From: Thomas Koeller <thomas.koeller@baslerweb.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-mips@linux-mips.org, linux-mtd@lists.infradead.org
Subject: Re: [PATCH] eXcite nand flash driver
Date: Sat, 10 Feb 2007 11:22:04 +0100	[thread overview]
Message-ID: <200702101122.05049.thomas.koeller@baslerweb.com> (raw)
In-Reply-To: <1170949737.3646.29.camel@chaos>

On Donnerstag, 8. Februar 2007, Thomas Gleixner wrote:
> > +/* command and control functions */
> > +static void excite_nand_control(struct mtd_info *mtd, int cmd,
> > +				       unsigned int ctrl)
> > +{
> > +	io_reg_t regs =
> > +	    container_of(mtd, struct excite_nand_drvdata, board_mtd)->regs;
> > +	static void __iomem *tgt = NULL;
> > +
> > +	switch (ctrl) {
> > +	case NAND_CTRL_CHANGE | NAND_CTRL_CLE:
> > +		tgt = regs + EXCITE_NANDFLASH_CMD_BYTE;
> > +		break;
> > +	case NAND_CTRL_CHANGE | NAND_CTRL_ALE:
> > +		tgt = regs + EXCITE_NANDFLASH_ADDR_BYTE;
> > +		break;
> > +	case NAND_CTRL_CHANGE | NAND_NCE:
> > +		tgt = regs + EXCITE_NANDFLASH_DATA_BYTE;
> > +		break;
> > +	}
>
> Err, did this ever work ? I doubt it. From nand_base.c:
>
>                 chip->cmd_ctrl(mtd, page_addr, ctrl);
>                 ctrl &= ~NAND_CTRL_CHANGE;
>                 chip->cmd_ctrl(mtd, page_addr >> 8, ctrl);
>
> So I expect an OOPS happens on a regular base.
>

I guess it is the 'static void __iomem *tgt = NULL' part that worries
you? Think about it, that value is never used.

However, I admit it is somewhat unclean, and therefore I am changing
it. Updated patch follows.

tk

  reply	other threads:[~2007-02-10 10:26 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-08  0:57 [PATCH] eXcite nand flash driver Thomas Koeller
2007-02-08  8:20 ` Ulrich Eckhardt
2007-02-08 10:20   ` Thomas Koeller
2007-02-08 13:50 ` Josh Boyer
2007-02-08 13:50   ` Josh Boyer
2007-02-08 13:55   ` Artem Bityutskiy
2007-02-08 13:55     ` Artem Bityutskiy
2007-02-08 15:48 ` Thomas Gleixner
2007-02-08 15:48   ` Thomas Gleixner
2007-02-10 10:22   ` Thomas Koeller [this message]
2007-02-10 10:22     ` Thomas Koeller
2007-02-11 15:44     ` Thomas Gleixner
2007-02-11 15:44       ` Thomas Gleixner
  -- strict thread matches above, loose matches on Subject: below --
2007-02-10 10:21 Thomas Koeller
2007-02-11 15:50 ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200702101122.05049.thomas.koeller@baslerweb.com \
    --to=thomas.koeller@baslerweb.com \
    --cc=linux-mips@linux-mips.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.