All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
To: Tudor.Ambarus@microchip.com
Cc: vigneshr@ti.com, richard@nod.at, j.neuschaefer@gmx.net,
	linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
	miquel.raynal@bootlin.com
Subject: Re: [PATCH] mtd: spi-nor: Simplify loop in spi_nor_read_id()
Date: Fri, 21 Feb 2020 17:22:48 +0100	[thread overview]
Message-ID: <20200221162248.GG2031@latitude> (raw)
In-Reply-To: <5604429.rq6fcmI4QA@localhost.localdomain>


[-- Attachment #1.1: Type: text/plain, Size: 1159 bytes --]

On Wed, Feb 19, 2020 at 08:06:08AM +0000, Tudor.Ambarus@microchip.com wrote:
[...]
> >  static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> >  {
> > -       int                     tmp;
> > +       int                     tmp, i;
> 
> while cleaning this function, would you rename tmp with ret?

Good idea, I'll do that in v2.

> >         u8                      *id = nor->bouncebuf;
> 
> and please drop this tab between u8 and *id

The same with the other variables in this functions? They have tabs
between the type and the pointer star / name as well.

> 
> >         const struct flash_info *info;
> 
> Also, IMO, the definition of variables should be done with the focus of 
> avoiding stack padding. With this in mind, I would first define the pointers 
> and then the ints and smaller types. But there are others than prefer defining 
> the variables in a tree/reverse-tree way, depending of the length of the line. 
> There's no agreement on this, either way if fine, do as you prefer.

I have no preference here. I think I'll keep it as is for now.


Thanks for the review,
Jonathan Neuschäfer

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
To: Tudor.Ambarus@microchip.com
Cc: j.neuschaefer@gmx.net, linux-mtd@lists.infradead.org,
	miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mtd: spi-nor: Simplify loop in spi_nor_read_id()
Date: Fri, 21 Feb 2020 17:22:48 +0100	[thread overview]
Message-ID: <20200221162248.GG2031@latitude> (raw)
In-Reply-To: <5604429.rq6fcmI4QA@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]

On Wed, Feb 19, 2020 at 08:06:08AM +0000, Tudor.Ambarus@microchip.com wrote:
[...]
> >  static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> >  {
> > -       int                     tmp;
> > +       int                     tmp, i;
> 
> while cleaning this function, would you rename tmp with ret?

Good idea, I'll do that in v2.

> >         u8                      *id = nor->bouncebuf;
> 
> and please drop this tab between u8 and *id

The same with the other variables in this functions? They have tabs
between the type and the pointer star / name as well.

> 
> >         const struct flash_info *info;
> 
> Also, IMO, the definition of variables should be done with the focus of 
> avoiding stack padding. With this in mind, I would first define the pointers 
> and then the ints and smaller types. But there are others than prefer defining 
> the variables in a tree/reverse-tree way, depending of the length of the line. 
> There's no agreement on this, either way if fine, do as you prefer.

I have no preference here. I think I'll keep it as is for now.


Thanks for the review,
Jonathan Neuschäfer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-02-21 16:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-18 15:10 [PATCH] mtd: spi-nor: Simplify loop in spi_nor_read_id() Jonathan Neuschäfer
2020-02-18 15:10 ` Jonathan Neuschäfer
2020-02-19  8:06 ` Tudor.Ambarus
2020-02-19  8:06   ` Tudor.Ambarus
2020-02-21 16:22   ` Jonathan Neuschäfer [this message]
2020-02-21 16:22     ` Jonathan Neuschäfer
2020-02-21 16:50     ` Tudor.Ambarus
2020-02-21 16:50       ` Tudor.Ambarus
2020-02-22 21:51       ` Jonathan Neuschäfer
2020-02-22 21:51         ` Jonathan Neuschäfer

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=20200221162248.GG2031@latitude \
    --to=j.neuschaefer@gmx.net \
    --cc=Tudor.Ambarus@microchip.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.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 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.