Re: [PATCH 1/1 v2] driver:mtd:spi-nor: Add Micron quad I/O support
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/1 v2] driver:mtd:spi-nor: Add Micron quad I/O support
>> 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! just as subject and commit
message described,
it is for enable Micron Quad spi transfer mode.do you read the spi-nor.c file?
please pay attention
to the set_quad_mode() function.by the way,I can add more commit message for
it,but I think
it is redundant,don't need.
>> +static int micron_quad_enable(struct spi_nor *nor) {
>> +int ret, val;
>> +
>> +ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
>> +if (ret < 0) {
>> +dev_err(nor->dev, "error %d reading EVCR\n", ret);
>> +return -EINVAL;
>> +}
>> +
>> +write_enable(nor);
>> +
>> +/* set EVCR ,enable quad I/O */
>> +nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
>> +ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
>> +if (ret < 0) {
>> +dev_err(nor->dev,
>> +"error while writing EVCR register\n");
>> +return -EINVAL;
>Why not just "return ret;" ?
>[...]
Ok,this is good,I will modify it in the next patch version.thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1 v2] driver:mtd:spi-nor: Add Micron quad I/O support
On Sunday, September 28, 2014 at 03:59:42 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.
> Signed-off-by: bean huo
> ---
> v1-v2:modified to that capture wait_till_ready()
> return value,if error,directly return its
> the value.
>
> drivers/mtd/spi-nor/spi-nor.c | 46
> + include/linux/mtd/spi-nor.h |
> 6 ++
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index b5ad6be..0c3b4fd 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -878,6 +878,45 @@ static int spansion_quad_enable(struct spi_nor *nor)
> return 0;
> }
>
> +static int micron_quad_enable(struct spi_nor *nor)
> +{
> + int ret, val;
> +
> + ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
> + if (ret < 0) {
> + dev_err(nor->dev, "error %d reading EVCR\n", ret);
> + return -EINVAL;
> + }
> +
> + write_enable(nor);
> +
> + /* set EVCR ,enable quad I/O */
> + nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
> + ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
> + if (ret < 0) {
> + dev_err(nor->dev,
> + "error while writing EVCR register\n");
> + return -EINVAL;
Why not just "return ret;" ?
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
