public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: bpqw <bpqw@micron.com>
Cc: "dwmw2@infradead.org" <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	"shijie8@gmail.com" <shijie8@gmail.com>,
	"geert+renesas@glider.be" <geert+renesas@glider.be>,
	"grmoore@altera.com" <grmoore@altera.com>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1 v2] driver:mtd:spi-nor: Add Micron quad I/O support
Date: Mon, 29 Sep 2014 20:57:31 +0200	[thread overview]
Message-ID: <201409292057.31274.marex@denx.de> (raw)
In-Reply-To: <A765B125120D1346A63912DDE6D8B6315DCF74@NTXXIAMBX02.xacn.micron.com>

On Monday, September 29, 2014 at 02:30:04 AM, bpqw wrote:
> >> For Micron spi norflash,you can enable Quad spi transfer by clear
> >> EVCR(Enhanced Volatile Configuration Register) Quad I/O protocol bit.
> >
> >OK, this information is nice and all, but what does this patch do? I can't
> >learn this information from the commit message as it is, can I ? And ,
> >the purpose of the commit message is exactly to summarize the change the
> >patch implements.
> 
> you don't understand what purpose of this patch!

Well, I dare to say, reacting to feedback like you just did won't make you many 
allies around here.

> just as subject and commit
> message described, it is for enable Micron Quad spi transfer mode.

I understand the subject part. The commit message, on the other hand, just 
states that it is possible to frob with a certain register to achieve a certain 
effect ; the commit message does not state what this patch does or how is the 
patch useful.

Does this patch enable the bit or does it disable the bit ? I cannot tell 
without looking into the code , I really have no clue just by reading the 
subject and the commit message.

> do you
> read the spi-nor.c file?

No, I didn't even look at the code.

> please pay attention to the set_quad_mode()
> function.

No, what set_quad_mode_function() are you talking about ...

> by the way,I can add more commit message for it,but I think it is
> redundant,don't need.

The commit message shall state what the patch does in the first place, what the 
hardware can do is ortogonal to that. The commit message can be as short as:

The hardware supports 4-bit I/O when bit FOO is set in register BAR. This patch 
adds function that sets bit FOO in register BAR to enable 4-bit I/O if condition 
BAZ and QUUX are met.

Then I do not even have to look at the code if I want to just get the high-level 
overview of what the patch does. If I want to know the details, I will look into 
the code.

Do you know what I'm getting at ?

[...]

Best regards,
Marek Vasut

  reply	other threads:[~2014-09-30  3:04 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-25  6:20 [PATCH 1/1] driver:mtd:spi-nor: Add Micron quad I/O support bpqw
2014-09-25 10:11 ` Marek Vasut
2014-09-26  8:39   ` bpqw
2014-09-26  8:46     ` Marek Vasut
2014-09-28  1:59       ` [PATCH 1/1 v2] " bpqw
2014-09-28 22:43         ` Marek Vasut
2014-09-29  0:30           ` bpqw
2014-09-29 18:57             ` Marek Vasut [this message]
2014-09-30  2:47       ` [PATCH 1/1 v3] " Bean Huo 霍斌斌 (beanhuo)
2014-09-30 13:38         ` Marek Vasut
2014-10-01 14:24           ` Bean Huo 霍斌斌 (beanhuo)
2014-10-01 14:32             ` Marek Vasut
2014-10-01 14:28       ` bpqw
2014-10-01 14:33         ` Marek Vasut
2014-10-04  5:55           ` bpqw
2014-10-16  1:53           ` bpqw
2014-10-17  0:37       ` Bean Huo 霍斌斌 (beanhuo)
2014-10-20  1:24       ` bpqw
2014-10-23  0:58       ` Bean Huo 霍斌斌 (beanhuo)
2014-10-24  0:31       ` Bean Huo 霍斌斌 (beanhuo)
2014-10-27  0:09       ` [PATCH 1/1 v3] driver:mtd:spi-nor: Add quad I/O support for Micron spi nor bpqw
2014-10-30 14:31         ` bpqw
2014-11-12 20:59         ` Jagan Teki
2014-11-04 13:25       ` [PATCH 1/1 v4] " bpqw
2014-11-05 11:26         ` Brian Norris
2014-11-06  2:56           ` bpqw
2014-11-06  3:09       ` [V5 PATCH 1/1] " bpqw
     [not found]         ` <54613259.4070903@opensource.altera.com>
2014-11-11 19:41           ` Graham Moore
2014-11-11 21:55             ` Jagan Teki
2014-11-12  1:19               ` bpqw
2014-11-12  0:58             ` bpqw
2014-11-13 16:26               ` Graham Moore
2014-11-14  2:06                 ` bpqw
2014-11-26  4:06         ` Brian Norris
2014-11-26 16:08           ` bpqw
2014-11-26 21:12         ` Brian Norris
2014-11-27  5:55           ` bpqw
2014-11-27  9:14             ` Brian Norris
2014-11-30 16:11               ` Bean Huo 霍斌斌 (beanhuo)

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=201409292057.31274.marex@denx.de \
    --to=marex@denx.de \
    --cc=bpqw@micron.com \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=geert+renesas@glider.be \
    --cc=grmoore@altera.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=shijie8@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox