Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Received-SPF: Pass (esa3.microchip.iphmx.com: domain of
  Tudor.Ambarus@microchip.com designates 198.175.253.82 as
  permitted sender) identity=mailfrom;
  client-ip=198.175.253.82; receiver=esa3.microchip.iphmx.com;
  envelope-from="Tudor.Ambarus@microchip.com";
  x-sender="Tudor.Ambarus@microchip.com";
  x-conformance=spf_only; x-record-type="v=spf1";
  x-record-text="v=spf1 mx a:ushub1.microchip.com
  a:smtpout.microchip.com -exists:%{i}.spf.microchip.iphmx.com
  include:servers.mcsv.net include:mktomail.com
  include:spf.protection.outlook.com ~all"
Received-SPF: None (esa3.microchip.iphmx.com: no sender
  authenticity information available from domain of
  postmaster@email.microchip.com) identity=helo;
  client-ip=198.175.253.82; receiver=esa3.microchip.iphmx.com;
  envelope-from="Tudor.Ambarus@microchip.com";
  x-sender="postmaster@email.microchip.com";
  x-conformance=spf_only
IronPort-SDR: FG0giC8yO/zxEicxGN5bdtY8IGoFWrSm2JPVl7hEQyj551DemTPRlX8nejnWvwuqI8Y+XNE7eG
 oP/b91TJPfUEP/zdy7hPU2Xg5BNDh3VU+PwSytngA2BSRwKtfiv8xAMpNtvvBQNDkmyA+VLZRn
 GgXmE5z8YF4yQ6Uei3X+607u20PiOytLBkJt3i5lfjT+lkqb6UwtJuCXcGfm9MKBIpkloDXun3
 RIOOvKjIkeahHMOX/VKtKYip88ytMI3j5247e6FAtg/gp6ToT4IR2IpAv5A6VWF9eRNZvD793i
 NmQ=
From:   <Tudor.Ambarus@microchip.com>
To:     <j.neuschaefer@gmx.net>
CC:     <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()
Thread-Topic: [PATCH] mtd: spi-nor: Simplify loop in spi_nor_read_id()
Thread-Index: AQHV5vtxJe3NG8eq40Gy2BJBFopNWA==
Date:   Wed, 19 Feb 2020 08:06:08 +0000
Message-ID: <5604429.rq6fcmI4QA@localhost.localdomain>
References: <20200218151034.24744-1-j.neuschaefer@gmx.net>
In-Reply-To: <20200218151034.24744-1-j.neuschaefer@gmx.net>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: microchip.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="iso-8859-1"
Content-ID: <483678ED2FE2CC458C339AA72BBE6CF6@namprd11.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 607fd1e8-6fff-48cc-c38e-08d7b51293be
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Feb 2020 08:06:08.8096
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 5Yu2dMYTB5nj/qBviLlnM5XehZrjxB7nzGYmwPtW8BUGMtf9m6FLQ76HX6hKNvX8ujs6wIsraBXO/6GKe9NN0AcJmmq5ZnSyza3k23kbVhQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4175
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi, Jonathan,

On Tuesday, February 18, 2020 5:10:34 PM EET Jonathan Neusch=E4fer wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know th=
e
> content is safe
>=20
> - Don't use `tmp` for two purposes (return value, loop counter)
> - Name the loop counter `i`, as is convention
> - Return the pointer variable that the if conditions leading up to the
>   return statement already operate on, rather than a different
>   expression that evaluates to the same pointer
>=20
> Signed-off-by: Jonathan Neusch=E4fer <j.neuschaefer@gmx.net>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>=20
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.=
c
> index 4fc632ec18fe..c491572d5267 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2711,7 +2711,7 @@ static const struct flash_info spi_nor_ids[] =3D {
>=20
>  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?

>         u8                      *id =3D nor->bouncebuf;

and please drop this tab between u8 and *id

>         const struct flash_info *info;

Also, IMO, the definition of variables should be done with the focus of=20
avoiding stack padding. With this in mind, I would first define the pointer=
s=20
and then the ints and smaller types. But there are others than prefer defin=
ing=20
the variables in a tree/reverse-tree way, depending of the length of the li=
ne.=20
There's no agreement on this, either way if fine, do as you prefer.

>=20
> @@ -2732,11 +2732,11 @@ static const struct flash_info
> *spi_nor_read_id(struct spi_nor *nor) return ERR_PTR(tmp);
>         }
>=20
> -       for (tmp =3D 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
> -               info =3D &spi_nor_ids[tmp];
> +       for (i =3D 0; i < ARRAY_SIZE(spi_nor_ids) - 1; i++) {
> +               info =3D &spi_nor_ids[i];
>                 if (info->id_len) {
>                         if (!memcmp(info->id, id, info->id_len))
> -                               return &spi_nor_ids[tmp];
> +                               return info;

Looks good,

Cheers,
ta