From: Sumit Garg <sumit.garg@linaro.org> To: Daniel Thompson <daniel.thompson@linaro.org> Cc: linux-arm-kernel@lists.infradead.org, "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" <linux-crypto@vger.kernel.org>, linux-kernel@vger.kernel.org, Jens Wiklander <jens.wiklander@linaro.org>, mpm@selenic.com, Herbert Xu <herbert@gondor.apana.org.au>, Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Arnd Bergmann <arnd@arndb.de>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Bhupesh Sharma <bhsharma@redhat.com>, tee-dev@lists.linaro.org Subject: Re: [PATCH v2 2/4] tee: optee: add TEE bus device enumeration support Date: Fri, 11 Jan 2019 15:21:50 +0530 [thread overview] Message-ID: <CAFA6WYNp5ve=xZ1bQL+HUGK4Pz7dpzMzWy_fuS0_q8NHaZWL4Q@mail.gmail.com> (raw) In-Reply-To: <20190111093902.hfxb67txjhhlegzu@holly.lan> On Fri, 11 Jan 2019 at 15:09, Daniel Thompson <daniel.thompson@linaro.org> wrote: > > On Fri, Jan 11, 2019 at 12:52:19PM +0530, Sumit Garg wrote: > > On Thu, 10 Jan 2019 at 19:49, Daniel Thompson > > <daniel.thompson@linaro.org> wrote: > > > > +static int get_devices(struct tee_context *ctx, u32 session, > > > > + struct tee_shm *device_uuid, u32 *shm_size) > > > > > > Missing const on device_uuid? > > > > > > > I don't think we should have a const for device_uuid here as this is > > shared memory struct pointer which is dynamically allocated and used > > to fetch device UUIDs. > > Agree. Perhaps device_uuid is misnamed though (part of the reason I > misread this is that it is singular so I though it was a single UUID > travelling into the TZ). > Will rename it to device_shm instead. > > > > + rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size); > > > > + if (rc < 0) > > > > + goto out_shm; > > > > + > > > > + device_uuid = tee_shm_get_va(device_shm, 0); > > > > + if (IS_ERR(device_uuid)) { > > > > + pr_err("tee_shm_get_va failed\n"); > > > > + rc = PTR_ERR(device_uuid); > > > > + goto out_shm; > > > > + } > > > > + > > > > + while (idx < shm_size / sizeof(uuid_t)) { > > > > > > This is a very uncommon way to write a for loop ;-). > > > > > > > Ok, will add "num_devices" variable. > > num_devices might add readability but that is not what I meant. > > The most idiomatic way to write somthing that loops for every valid index > value is: > > for (i=0; i < limit; i++) > > You wrote it like this: > > int idx=0; > > /* lots of code between initializer and first use */ > > while (idx < limit) { > /* more code */ > idx++; > } > > Sure, they are equivalent but the idiomatic form is easier to read. > Oh okay, will use "for" loop instead. -Sumit > > Daniel.
WARNING: multiple messages have this Message-ID (diff)
From: Sumit Garg <sumit.garg@linaro.org> To: Daniel Thompson <daniel.thompson@linaro.org> Cc: Mark Rutland <mark.rutland@arm.com>, Herbert Xu <herbert@gondor.apana.org.au>, Arnd Bergmann <arnd@arndb.de>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Bhupesh Sharma <bhsharma@redhat.com>, linux-kernel@vger.kernel.org, tee-dev@lists.linaro.org, Rob Herring <robh+dt@kernel.org>, "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" <linux-crypto@vger.kernel.org>, mpm@selenic.com, Jens Wiklander <jens.wiklander@linaro.org>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 2/4] tee: optee: add TEE bus device enumeration support Date: Fri, 11 Jan 2019 15:21:50 +0530 [thread overview] Message-ID: <CAFA6WYNp5ve=xZ1bQL+HUGK4Pz7dpzMzWy_fuS0_q8NHaZWL4Q@mail.gmail.com> (raw) In-Reply-To: <20190111093902.hfxb67txjhhlegzu@holly.lan> On Fri, 11 Jan 2019 at 15:09, Daniel Thompson <daniel.thompson@linaro.org> wrote: > > On Fri, Jan 11, 2019 at 12:52:19PM +0530, Sumit Garg wrote: > > On Thu, 10 Jan 2019 at 19:49, Daniel Thompson > > <daniel.thompson@linaro.org> wrote: > > > > +static int get_devices(struct tee_context *ctx, u32 session, > > > > + struct tee_shm *device_uuid, u32 *shm_size) > > > > > > Missing const on device_uuid? > > > > > > > I don't think we should have a const for device_uuid here as this is > > shared memory struct pointer which is dynamically allocated and used > > to fetch device UUIDs. > > Agree. Perhaps device_uuid is misnamed though (part of the reason I > misread this is that it is singular so I though it was a single UUID > travelling into the TZ). > Will rename it to device_shm instead. > > > > + rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size); > > > > + if (rc < 0) > > > > + goto out_shm; > > > > + > > > > + device_uuid = tee_shm_get_va(device_shm, 0); > > > > + if (IS_ERR(device_uuid)) { > > > > + pr_err("tee_shm_get_va failed\n"); > > > > + rc = PTR_ERR(device_uuid); > > > > + goto out_shm; > > > > + } > > > > + > > > > + while (idx < shm_size / sizeof(uuid_t)) { > > > > > > This is a very uncommon way to write a for loop ;-). > > > > > > > Ok, will add "num_devices" variable. > > num_devices might add readability but that is not what I meant. > > The most idiomatic way to write somthing that loops for every valid index > value is: > > for (i=0; i < limit; i++) > > You wrote it like this: > > int idx=0; > > /* lots of code between initializer and first use */ > > while (idx < limit) { > /* more code */ > idx++; > } > > Sure, they are equivalent but the idiomatic form is easier to read. > Oh okay, will use "for" loop instead. -Sumit > > Daniel. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-01-11 9:52 UTC|newest] Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-10 12:24 [PATCH v2 0/4] Introduce TEE bus driver framework Sumit Garg 2019-01-10 12:24 ` Sumit Garg 2019-01-10 12:24 ` [PATCH v2 1/4] tee: add bus driver framework for TEE based devices Sumit Garg 2019-01-10 12:24 ` Sumit Garg 2019-01-10 14:06 ` Daniel Thompson 2019-01-10 14:06 ` Daniel Thompson 2019-01-11 6:41 ` Sumit Garg 2019-01-11 6:41 ` Sumit Garg 2019-01-11 6:41 ` Sumit Garg 2019-01-10 12:24 ` [PATCH v2 2/4] tee: optee: add TEE bus device enumeration support Sumit Garg 2019-01-10 12:24 ` Sumit Garg 2019-01-10 14:18 ` Daniel Thompson 2019-01-10 14:18 ` Daniel Thompson 2019-01-11 7:22 ` Sumit Garg 2019-01-11 7:22 ` Sumit Garg 2019-01-11 7:22 ` Sumit Garg 2019-01-11 9:39 ` Daniel Thompson 2019-01-11 9:39 ` Daniel Thompson 2019-01-11 9:39 ` Daniel Thompson 2019-01-11 9:51 ` Sumit Garg [this message] 2019-01-11 9:51 ` Sumit Garg 2019-01-11 9:51 ` Sumit Garg 2019-01-10 12:24 ` [PATCH v2 3/4] tee: add supp_nowait flag in tee_context struct Sumit Garg 2019-01-10 12:24 ` Sumit Garg 2019-01-10 14:23 ` Daniel Thompson 2019-01-10 14:23 ` Daniel Thompson 2019-01-11 7:30 ` Sumit Garg 2019-01-11 7:30 ` Sumit Garg 2019-01-11 7:30 ` Sumit Garg 2019-01-11 9:54 ` Daniel Thompson 2019-01-11 9:54 ` Daniel Thompson 2019-01-11 9:54 ` Daniel Thompson 2019-01-11 9:57 ` Sumit Garg 2019-01-11 9:57 ` Sumit Garg 2019-01-11 9:57 ` Sumit Garg 2019-01-10 12:24 ` [PATCH v2 4/4] hwrng: add OP-TEE based rng driver Sumit Garg 2019-01-10 12:24 ` Sumit Garg 2019-01-10 13:55 ` [Tee-dev] " Joakim Bech 2019-01-10 13:55 ` Joakim Bech 2019-01-11 6:34 ` Sumit Garg 2019-01-11 6:34 ` Sumit Garg 2019-01-11 6:34 ` Sumit Garg 2019-01-10 14:27 ` Daniel Thompson 2019-01-10 14:27 ` Daniel Thompson 2019-01-11 8:40 ` Sumit Garg 2019-01-11 8:40 ` Sumit Garg 2019-01-11 8:40 ` Sumit Garg
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='CAFA6WYNp5ve=xZ1bQL+HUGK4Pz7dpzMzWy_fuS0_q8NHaZWL4Q@mail.gmail.com' \ --to=sumit.garg@linaro.org \ --cc=ard.biesheuvel@linaro.org \ --cc=arnd@arndb.de \ --cc=bhsharma@redhat.com \ --cc=daniel.thompson@linaro.org \ --cc=gregkh@linuxfoundation.org \ --cc=herbert@gondor.apana.org.au \ --cc=jens.wiklander@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-crypto@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mpm@selenic.com \ --cc=robh+dt@kernel.org \ --cc=tee-dev@lists.linaro.org \ /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: linkBe 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.