Re: [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller
On 30-Jun-14 10:03, Zhou Wang wrote:
> Signed-off-by: Zhou Wang
> ---
> arch/arm/boot/dts/hip04.dtsi | 31 +++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
> index abb42ca..e63fc61 100644
> --- a/arch/arm/boot/dts/hip04.dtsi
> +++ b/arch/arm/boot/dts/hip04.dtsi
> @@ -386,5 +386,36 @@
> interrupts = <0 389 4>;
> };
> };
> +
> + nand: nand@402 {
> + compatible = "hisilicon,504-nfc";
In PATCH 2/3 and 3/3 you are using "hisilicon,nfc504".
> + reg = <0x402 0x1>, <0x500 0x1000>;
> + interrupts = <0 379 4>;
> + nand-bus-width = <8>;
> + nand-ecc-mode = "none";
> + hisi,nand-ecc-bits = <0>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition@0 {
> + label = "nand_text";
> + reg = <0x 0x0040>;
> + };
> +
> + partition@0040 {
> + label = "nand_monitor";
> + reg = <0x0040 0x0040>;
> + };
> +
> + partition@0080 {
> + label = "nand_kernel";
> + reg = <0x0080 0x0080>;
> + };
> +
> + partition@0100 {
> + label = "nand_fs";
> + reg = <0x0100 0x1f00>;
> + };
> + };
> };
> };
>
--
Jerome
--
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: [Tee-dev] [PATCHv3 2/3] optee: use uuid for sysfs driver entry
On 5/25/20 1:52 PM, Maxim Uvarov wrote: > Optee device names for sysfs needed to be unique s/Optee/OP-TEE/ s/needed/need/ > and it's better if they will mean something. UUID for name > looks like good solution: > /sys/bus/tee/devices/optee-clnt- How about mentioning it is the UUID of the Trusted Application on the TEE side? > > Signed-off-by: Maxim Uvarov > --- > drivers/tee/optee/device.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Thanks, -- Jerome
Re: [Tee-dev] [PATCHv3 2/3] optee: use uuid for sysfs driver entry
On 5/25/20 3:36 PM, Maxim Uvarov wrote: > On Mon, 25 May 2020 at 15:10, Jerome Forissier wrote: >> >> >> >> On 5/25/20 1:52 PM, Maxim Uvarov wrote: >>> Optee device names for sysfs needed to be unique >> >> s/Optee/OP-TEE/ >> s/needed/need/ >> >>> and it's better if they will mean something. UUID for name >>> looks like good solution: >>> /sys/bus/tee/devices/optee-clnt- >> >> How about mentioning it is the UUID of the Trusted Application on the >> TEE side? >> > > Jerome, do you think optee-ta- is more suitable here? Yes, a bit better I think. More "self explanatory"... kind of :) Is it possible to have several devices bound to the same TA? I think nothing forbids this although we may not have any use case for now... -- Jerome
[PATCH 1/1] tee: optee: do not check memref size on return from Secure World
When Secure World returns, it may have changed the size attribute of the
memory references passed as [in/out] parameters. The GlobalPlatform TEE
Internal Core API specification does not restrict the values that this
size can take. In particular, Secure World may increase the value to be
larger than the size of the input buffer to indicate that it needs more.
Therefore, the size check in optee_from_msg_param() is incorrect and
needs to be removed. This fixes a number of failed test cases in the
GlobalPlatform TEE Initial Configuratiom Test Suite v2_0_0_0-2017_06_09
when OP-TEE is compiled without dynamic shared memory support
(CFG_CORE_DYN_SHM=n).
Suggested-by: Jens Wiklander
Signed-off-by: Jerome Forissier
---
drivers/tee/optee/core.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 319a1e701163..ddb8f9ecf307 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -79,16 +79,6 @@ int optee_from_msg_param(struct tee_param *params, size_t
num_params,
return rc;
p->u.memref.shm_offs = mp->u.tmem.buf_ptr - pa;
p->u.memref.shm = shm;
-
- /* Check that the memref is covered by the shm object */
- if (p->u.memref.size) {
- size_t o = p->u.memref.shm_offs +
- p->u.memref.size - 1;
-
- rc = tee_shm_get_pa(shm, o, NULL);
- if (rc)
- return rc;
- }
break;
case OPTEE_MSG_ATTR_TYPE_RMEM_INPUT:
case OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT:
--
2.25.1
Re: [PATCH v8 2/4] KEYS: trusted: Introduce TEE based Trusted Keys
On 1/21/21 1:02 AM, Jarkko Sakkinen via OP-TEE wrote: > On Wed, Jan 20, 2021 at 12:53:28PM +0530, Sumit Garg wrote: >> On Wed, 20 Jan 2021 at 07:01, Jarkko Sakkinen wrote: >>> >>> On Tue, Jan 19, 2021 at 12:30:42PM +0200, Jarkko Sakkinen wrote: On Fri, Jan 15, 2021 at 11:32:31AM +0530, Sumit Garg wrote: > On Thu, 14 Jan 2021 at 07:35, Jarkko Sakkinen wrote: >> >> On Wed, Jan 13, 2021 at 04:47:00PM +0530, Sumit Garg wrote: >>> Hi Jarkko, >>> >>> On Mon, 11 Jan 2021 at 22:05, Jarkko Sakkinen wrote: On Tue, Nov 03, 2020 at 09:31:44PM +0530, Sumit Garg wrote: > Add support for TEE based trusted keys where TEE provides the > functionality > to seal and unseal trusted keys using hardware unique key. > > Refer to Documentation/tee.txt for detailed information about TEE. > > Signed-off-by: Sumit Garg I haven't yet got QEMU environment working with aarch64, this produces just a blank screen: ./output/host/usr/bin/qemu-system-aarch64 -M virt -cpu cortex-a53 -smp 1 -kernel output/images/Image -initrd output/images/rootfs.cpio -serial stdio My BuildRoot fork for TPM and keyring testing is located over here: https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/buildroot-tpmdd.git/ The "ARM version" is at this point in aarch64 branch. Over time I will define tpmdd-x86_64 and tpmdd-aarch64 boards and everything will be then in the master branch. To create identical images you just need to $ make tpmdd_defconfig && make Can you check if you see anything obviously wrong? I'm eager to test this patch set, and in bigger picture I really need to have ready to run aarch64 environment available. >>> >>> I would rather suggest you to follow steps listed here [1] as to test >>> this feature on Qemu aarch64 we need to build firmwares such as TF-A, >>> OP-TEE, UEFI etc. which are all integrated into OP-TEE Qemu build >>> system [2]. And then it would be easier to migrate them to your >>> buildroot environment as well. >>> >>> [1] >>> https://lists.trustedfirmware.org/pipermail/op-tee/2020-May/27.html >>> [2] >>> https://optee.readthedocs.io/en/latest/building/devices/qemu.html#qemu-v8 >>> >>> -Sumit >> >> Can you provide 'keyctl_change'? Otherwise, the steps are easy to follow. >> > > $ cat keyctl_change > diff --git a/common.mk b/common.mk > index aeb7b41..663e528 100644 > --- a/common.mk > +++ b/common.mk > @@ -229,6 +229,7 @@ BR2_PACKAGE_OPTEE_TEST_SDK ?= > $(OPTEE_OS_TA_DEV_KIT_DIR) > BR2_PACKAGE_OPTEE_TEST_SITE ?= $(OPTEE_TEST_PATH) > BR2_PACKAGE_STRACE ?= y > BR2_TARGET_GENERIC_GETTY_PORT ?= $(if > $(CFG_NW_CONSOLE_UART),ttyAMA$(CFG_NW_CONSOLE_UART),ttyAMA0) > +BR2_PACKAGE_KEYUTILS := y > > # All BR2_* variables from the makefile or the environment are appended > to > # ../out-br/extra.conf. All values are quoted "..." except y and n. > diff --git a/kconfigs/qemu.conf b/kconfigs/qemu.conf > index 368c18a..832ab74 100644 > --- a/kconfigs/qemu.conf > +++ b/kconfigs/qemu.conf > @@ -20,3 +20,5 @@ CONFIG_9P_FS=y > CONFIG_9P_FS_POSIX_ACL=y > CONFIG_HW_RANDOM=y > CONFIG_HW_RANDOM_VIRTIO=y > +CONFIG_TRUSTED_KEYS=y > +CONFIG_ENCRYPTED_KEYS=y > >> After I've successfully tested 2/4, I'd suggest that you roll out one >> more >> version and CC the documentation patch to Elaine and Mini, and clearly >> remark in the commit message that TEE is a standard, with a link to the >> specification. >> > > Sure, I will roll out the next version after your testing. Thanks, I'll try this at instant, and give my feedback. >>> >>> I bump into this: >>> >>> $ make run-only >>> ln -sf /home/jarkko/devel/tpm/optee/build/../out-br/images/rootfs.cpio.gz >>> /home/jarkko/devel/tpm/optee/build/../out/bin/ >>> ln: failed to create symbolic link >>> '/home/jarkko/devel/tpm/optee/build/../out/bin/': No such file or directory >>> make: *** [Makefile:194: run-only] Error 1 >>> >> >> Could you check if the following directory tree is built after >> executing the below command? >> >> $ make -j`nproc` >> CFG_IN_TREE_EARLY_TAS=trusted_keys/f04a0fe7-1f5d-4b9b-abf7-619b85b4ce8c >> >> $ tree out/bin/ >> out/bin/ >> ├── bl1.bin -> >> /home/sumit/build/optee/build/../trusted-firmware-a/build/qemu/release/bl1.bin >> ├── bl2.bin -> >> /home/sumit/build/optee/build/../trusted-firmware-a/build/qemu/release/bl2.bin >> ├── bl31.bin -> >> /home/sumit/build/optee/build/../trusted-firmware-a/build/qemu/release/bl31.bin >> ├── bl32.bin -> >> /home/sumit/build/optee/build/../optee_os/out/arm/core/tee-header_v2.bi
Re: [PATCH v8 2/4] KEYS: trusted: Introduce TEE based Trusted Keys
On 1/21/21 4:24 PM, Jarkko Sakkinen wrote: > On Thu, Jan 21, 2021 at 05:07:42PM +0200, Jarkko Sakkinen wrote: >> On Thu, Jan 21, 2021 at 09:44:07AM +0100, Jerome Forissier wrote: >>> >>> >>> On 1/21/21 1:02 AM, Jarkko Sakkinen via OP-TEE wrote: >>>> On Wed, Jan 20, 2021 at 12:53:28PM +0530, Sumit Garg wrote: >>>>> On Wed, 20 Jan 2021 at 07:01, Jarkko Sakkinen wrote: >>>>>> >>>>>> On Tue, Jan 19, 2021 at 12:30:42PM +0200, Jarkko Sakkinen wrote: >>>>>>> On Fri, Jan 15, 2021 at 11:32:31AM +0530, Sumit Garg wrote: >>>>>>>> On Thu, 14 Jan 2021 at 07:35, Jarkko Sakkinen >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> On Wed, Jan 13, 2021 at 04:47:00PM +0530, Sumit Garg wrote: >>>>>>>>>> Hi Jarkko, >>>>>>>>>> >>>>>>>>>> On Mon, 11 Jan 2021 at 22:05, Jarkko Sakkinen >>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> On Tue, Nov 03, 2020 at 09:31:44PM +0530, Sumit Garg wrote: >>>>>>>>>>>> Add support for TEE based trusted keys where TEE provides the >>>>>>>>>>>> functionality >>>>>>>>>>>> to seal and unseal trusted keys using hardware unique key. >>>>>>>>>>>> >>>>>>>>>>>> Refer to Documentation/tee.txt for detailed information about TEE. >>>>>>>>>>>> >>>>>>>>>>>> Signed-off-by: Sumit Garg >>>>>>>>>>> >>>>>>>>>>> I haven't yet got QEMU environment working with aarch64, this >>>>>>>>>>> produces >>>>>>>>>>> just a blank screen: >>>>>>>>>>> >>>>>>>>>>> ./output/host/usr/bin/qemu-system-aarch64 -M virt -cpu cortex-a53 >>>>>>>>>>> -smp 1 -kernel output/images/Image -initrd >>>>>>>>>>> output/images/rootfs.cpio -serial stdio >>>>>>>>>>> >>>>>>>>>>> My BuildRoot fork for TPM and keyring testing is located over here: >>>>>>>>>>> >>>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/buildroot-tpmdd.git/ >>>>>>>>>>> >>>>>>>>>>> The "ARM version" is at this point in aarch64 branch. Over time I >>>>>>>>>>> will >>>>>>>>>>> define tpmdd-x86_64 and tpmdd-aarch64 boards and everything will be >>>>>>>>>>> then >>>>>>>>>>> in the master branch. >>>>>>>>>>> >>>>>>>>>>> To create identical images you just need to >>>>>>>>>>> >>>>>>>>>>> $ make tpmdd_defconfig && make >>>>>>>>>>> >>>>>>>>>>> Can you check if you see anything obviously wrong? I'm eager to >>>>>>>>>>> test this >>>>>>>>>>> patch set, and in bigger picture I really need to have ready to run >>>>>>>>>>> aarch64 environment available. >>>>>>>>>> >>>>>>>>>> I would rather suggest you to follow steps listed here [1] as to test >>>>>>>>>> this feature on Qemu aarch64 we need to build firmwares such as TF-A, >>>>>>>>>> OP-TEE, UEFI etc. which are all integrated into OP-TEE Qemu build >>>>>>>>>> system [2]. And then it would be easier to migrate them to your >>>>>>>>>> buildroot environment as well. >>>>>>>>>> >>>>>>>>>> [1] >>>>>>>>>> https://lists.trustedfirmware.org/pipermail/op-tee/2020-May/27.html >>>>>>>>>> [2] >>>>>>>>>> https://optee.readthedocs.io/en/latest/building/devices/qemu.html#qemu-v8 >>>>>>>>>> >>>>>>>>>> -Sumit >>>>>>>>> >>>>>>>>> Can you provide 'keyct
[PATCH] tee: optee: update optee_msg.h and optee_smc.h to dual license
The files optee_msg.h and optee_smc.h (under drivers/tee/optee) contain information originating from the OP-TEE OS project [1] [2], where the licensing terms are BSD 2-Clause. Therefore, apply a dual license to those files. [1] https://github.com/OP-TEE/optee_os/blob/master/core/include/optee_msg.h [2] https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/include/sm/optee_smc.h Signed-off-by: Jerome Forissier --- drivers/tee/optee/optee_msg.h | 26 ++ drivers/tee/optee/optee_smc.h | 26 ++ 2 files changed, 4 insertions(+), 48 deletions(-) diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h index 3050490..795bc19 100644 --- a/drivers/tee/optee/optee_msg.h +++ b/drivers/tee/optee/optee_msg.h @@ -1,28 +1,6 @@ +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */ /* - * Copyright (c) 2015-2016, Linaro Limited - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. + * Copyright (c) 2015-2019, Linaro Limited */ #ifndef _OPTEE_MSG_H #define _OPTEE_MSG_H diff --git a/drivers/tee/optee/optee_smc.h b/drivers/tee/optee/optee_smc.h index bbf0cf0..c72122d 100644 --- a/drivers/tee/optee/optee_smc.h +++ b/drivers/tee/optee/optee_smc.h @@ -1,28 +1,6 @@ +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */ /* - * Copyright (c) 2015-2016, Linaro Limited - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. + * Copyright (c) 2015-2019, Linaro Limited */ #ifndef OPTEE_SMC_H #define OPTEE_SMC_H -- 2.7.4
[PATCH v2] Documentation: dt: Add binding for /secure-chosen/stdout-path
Some platforms may use a single device tree to describe two address
spaces, as described in d9f43babb998 ("Documentation: dt: Add bindings
for Secure-only devices"). For these platforms it makes sense to define
a secure counterpart of /chosen, namely: /secure-chosen. This new node
is meant to be used by the secure firmware to pass data to the secure
OS. Only the stdout-path property is supported for now.
Signed-off-by: Jerome Forissier
---
Notes:
Sending this again, slightly modified. Previous submission was in March
2017 [1]. Since then, OP-TEE has implemented this binding for platforms
that use DT [2] (fallback to /chosen/stdout-path to be implemented in
[3]). A patch for QEMU has been proposed [4], to which the maintainer
responded "Are the DT bindings upstream yet?" ;-)
[1] https://patchwork.kernel.org/patch/9602401/
[2] https://github.com/OP-TEE/optee_os/commit/4dc31c52544a
[3] https://github.com/OP-TEE/optee_os/pull/2569
[4] https://patchwork.ozlabs.org/patch/979345/
Changes since v1:
- Use "should" instead of "may" ("...the Secure OS should use the value
of /chosen/stdout-path...").
Documentation/devicetree/bindings/arm/secure.txt | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/arm/secure.txt
b/Documentation/devicetree/bindings/arm/secure.txt
index e31303fb233a..f27bbff2c780 100644
--- a/Documentation/devicetree/bindings/arm/secure.txt
+++ b/Documentation/devicetree/bindings/arm/secure.txt
@@ -32,7 +32,8 @@ describe the view of Secure world using the standard
bindings. These
secure- bindings only need to be used where both the Secure and Normal
world views need to be described in a single device tree.
-Valid Secure world properties:
+Valid Secure world properties
+-
- secure-status : specifies whether the device is present and usable
in the secure world. The combination of this with "status" allows
@@ -51,3 +52,19 @@ Valid Secure world properties:
status = "disabled"; secure-status = "okay"; /* S-only */
status = "disabled"; /* disabled in both */
status = "disabled"; secure-status = "disabled"; /* disabled in both */
+
+The secure-chosen node
+--
+
+Similar to the /chosen node which serves as a place for passing data
+between firmware and the operating system, the /secure-chosen node may
+be used to pass data to the Secure OS. Only the properties defined
+below may appear in the /secure-chosen node.
+
+- stdout-path : specifies the device to be used by the Secure OS for
+ its console output. The syntax is the same as for /chosen/stdout-path.
+ If the /secure-chosen node exists but the stdout-path property is not
+ present, the Secure OS should not perform any console output. If
+ /secure-chosen does not exist, the Secure OS should use the value of
+ /chosen/stdout-path instead (that is, use the same device as the
+ Normal world OS).
--
2.15.1
Re: [PATCH 2/2] clk: imx: imx7d: remove clks_init_on array
On 08/31/2018 03:29 AM, Stephen Boyd wrote: > Quoting Peng Fan (2018-08-12 18:15:41) >> Hi Anson, >> > -Original Message- > From: Anson Huang > Sent: 2018年8月8日 12:39 > To: shawn...@kernel.org; s.ha...@pengutronix.de; > ker...@pengutronix.de; Fabio Estevam ; > mturque...@baylibre.com; sb...@kernel.org; > linux-arm-ker...@lists.infradead.org; > linux-...@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: dl-linux-imx > Subject: [PATCH 2/2] clk: imx: imx7d: remove clks_init_on array > > Clock framework will enable those clocks registered with > CLK_IS_CRITICAL flag, so no need to have clks_init_on array during > clock initialization now. Will it be more flexible to parse dts saying "critical-clocks = " or "init-on-arrary=" and enable those clocks? >>> >>> Parsing the clocks arrays from dtb is another way of enabling critical >>> clocks, but >>> for current i.MX6/7 platforms, we implement it in same way as most of other >>> SoCs, currently I did NOT see any necessity of putting them in dtb, just >>> adding >>> flag during clock registering is more simple, if there is any special >>> requirement >>> for different clocks set to be enabled, then we can add support to enable >>> the >>> method of parsing critical-clocks from dtb. Just my two cents. >> >> Thinking about OP-TEE want to use one device, but it's clocks are registered >> by Linux, because there is no module in Linux side use it, it will shutdown >> the clock, >> which cause OP-TEE could not access the device. >> >> Then people have to modify clk code to add CLK_IS_CRITICAL flag to make sure >> the clocks are not shutdown by Linux. >> >> However adding a new property in clk node and let driver code parse the dts, >> there is no need to modify clk driver code when OP-TEE needs another device >> clock. >> > > If OP-TEE needs linux to keep things on then why can't the OP-TEE driver > in Linux probe, acquire clocks, and keep the clks enabled forever? Sounds reasonable, but how could this be done without introducing platform-specific stuff in the OP-TEE driver? > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
[PATCH] checkpatch: add --typedefsfile
When using checkpatch on out-of-tree code, it may occur that some
project-specific types are used, which will cause spurious warnings.
Add the --typedefsfile option as a way to extend the known types and
deal with this issue.
Signed-off-by: Jerome Forissier
---
scripts/checkpatch.pl | 56 ++-
1 file changed, 37 insertions(+), 19 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index baa3c7b..eb55f5f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -55,6 +55,7 @@ my $spelling_file = "$D/spelling.txt";
my $codespell = 0;
my $codespellfile = "/usr/share/codespell/dictionary.txt";
my $conststructsfile = "$D/const_structs.checkpatch";
+my $typedefsfile = "";
my $color = 1;
my $allow_c99_comments = 1;
@@ -113,6 +114,7 @@ Options:
--codespellUse the codespell dictionary for spelling/typos
(default:/usr/share/codespell/dictionary.txt)
--codespellfileUse this codespell dictionary
+ --typedefsfile Read additional types from this file
--colorUse colors when output is STDOUT (default: on)
-h, --help, --version display this help and exit
@@ -208,6 +210,7 @@ GetOptions(
'test-only=s' => \$tst_only,
'codespell!'=> \$codespell,
'codespellfile=s' => \$codespellfile,
+ 'typedefsfile=s'=> \$typedefsfile,
'color!'=> \$color,
'h|help'=> \$help,
'version' => \$help
@@ -629,28 +632,43 @@ if ($codespell) {
$misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix;
+sub read_words {
+ my ($wordsRef, $file) = @_;
+
+ if (open(my $words, '<', $file)) {
+ while (<$words>) {
+ my $line = $_;
+
+ $line =~ s/\s*\n?$//g;
+ $line =~ s/^\s*//g;
+
+ next if ($line =~ m/^\s*#/);
+ next if ($line =~ m/^\s*$/);
+ if ($line =~ /\s/) {
+ print("$file: '$line' invalid - ignored\n");
+ next;
+ }
+
+ $$wordsRef .= '|' if ($$wordsRef ne "");
+ $$wordsRef .= $line;
+ }
+ close($file);
+ return 1;
+ }
+
+ return 0;
+}
+
my $const_structs = "";
-if (open(my $conststructs, '<', $conststructsfile)) {
- while (<$conststructs>) {
- my $line = $_;
+read_words(\$const_structs, $conststructsfile)
+or warn "No structs that should be const will be found - file
'$conststructsfile': $!\n";
- $line =~ s/\s*\n?$//g;
- $line =~ s/^\s*//g;
-
- next if ($line =~ m/^\s*#/);
- next if ($line =~ m/^\s*$/);
- if ($line =~ /\s/) {
- print("$conststructsfile: '$line' invalid - ignored\n");
- next;
- }
-
- $const_structs .= '|' if ($const_structs ne "");
- $const_structs .= $line;
- }
- close($conststructsfile);
-} else {
- warn "No structs that should be const will be found - file
'$conststructsfile': $!\n";
+my $typeOtherTypedefs = "";
+if (length($typedefsfile)) {
+ read_words(\$typeOtherTypedefs, $typedefsfile)
+ or warn "No additional types will be considered - file
'$typedefsfile': $!\n";
}
+$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne "");
sub build_types {
my $mods = "(?x: \n" . join("|\n ", (@modifierList,
@modifierListFile)) . "\n)";
--
2.7.4
Re: [PATCH] checkpatch: add --typedefsfile
On 04/21/2017 08:31 AM, Jerome Forissier wrote: > On 04/20/2017 06:49 PM, Joe Perches wrote: >> On Thu, 2017-04-20 at 17:39 +0200, Jerome Forissier wrote: >>> When using checkpatch on out-of-tree code, it may occur that some >>> project-specific types are used, which will cause spurious warnings. >>> Add the --typedefsfile option as a way to extend the known types and >>> deal with this issue. >> >> I'm not opposed to the addition. >> What out-of-tree project is this for? > > OP-TEE [1]. We run a Travis job on all pull requests [2], and checkpatch > is part of that. The typical false warning we get on a regular basis is > with some pointers to functions returning TEE_Result [3], which is a > typedef from the GlobalPlatform APIs. We consider it is acceptable to > use GP types in the OP-TEE core implementation, that's why this patch > would be helpful for us. > > [1] https://github.com/OP-TEE/optee_os > [2] https://travis-ci.org/OP-TEE/optee_os/builds > [3] https://travis-ci.org/OP-TEE/optee_os/builds/193355335#L1733 Ping?
[PATCH] checkpatch: don't match against empty $const_structs
When $conststructsfile does not exist or is empty, we may get false
warnings such as:
WARNING: struct should normally be const
Fix that by not running the string match if $const_structs is empty.
Signed-off-by: Jerome Forissier
---
scripts/checkpatch.pl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a8368d1..722a319 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6010,7 +6010,8 @@ sub process {
}
# check for various structs that are normally const (ops, kgdb, device_tree)
- if ($line !~ /\bconst\b/ &&
+ if ($const_structs ne "" &&
+ $line !~ /\bconst\b/ &&
$line =~ /\bstruct\s+($const_structs)\b/) {
WARN("CONST_STRUCT",
"struct $1 should normally be const\n" .
--
2.7.4
Re: [PATCH] checkpatch: don't match against empty $const_structs
On 10/20/2016 10:16 AM, Joe Perches wrote:
> On Thu, 2016-10-20 at 10:12 +0200, Jerome Forissier wrote:
>> When $conststructsfile does not exist or is empty, we may get false
>> warnings such as:
>> WARNING: struct should normally be const
>>
>> Fix that by not running the string match if $const_structs is empty.
>
> Why should $const_structs ever be empty?
When running out-of-tree, i.e., using checkpatch.pl on non-kernel sources.
>
>> Signed-off-by: Jerome Forissier
>> ---
>> scripts/checkpatch.pl | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index a8368d1..722a319 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -6010,7 +6010,8 @@ sub process {
>> }
>>
>> # check for various structs that are normally const (ops, kgdb, device_tree)
>> -if ($line !~ /\bconst\b/ &&
>> +if ($const_structs ne "" &&
>> +$line !~ /\bconst\b/ &&
>> $line =~ /\bstruct\s+($const_structs)\b/) {
>> WARN("CONST_STRUCT",
>> "struct $1 should normally be const\n" .
>>
[PATCH] checkpatch: don't try to get maintained status when --no-tree is given
Fixes the following warning:
Use of uninitialized value $root in concatenation (.) or string at
/path/to/checkpatch.pl line 764.
Signed-off-by: Jerome Forissier
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a8368d1..aa694bc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -761,7 +761,7 @@ sub seed_camelcase_file {
sub is_maintained_obsolete {
my ($filename) = @_;
- return 0 if (!(-e "$root/scripts/get_maintainer.pl"));
+ return 0 if (!$tree || !(-e "$root/scripts/get_maintainer.pl"));
my $status = `perl $root/scripts/get_maintainer.pl --status --nom --nol
--nogit --nogit-fallback -f $filename 2>&1`;
--
2.7.4
[PATCH] tee: add forward declaration for struct device
tee_drv.h references struct device, but does not include device.h nor platform_device.h. Therefore, if tee_drv.h is included by some file that does not pull device.h nor platform_device.h beforehand, we have a compile warning. Fix this by adding a forward declaration. Signed-off-by: Jerome Forissier --- include/linux/tee_drv.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h index 8614713..07bd226 100644 --- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -29,6 +29,7 @@ #define TEE_SHM_DMA_BUFBIT(1) /* Memory with dma-buf handle */ #define TEE_SHM_EXT_DMA_BUFBIT(2) /* Memory with dma-buf handle */ +struct device; struct tee_device; struct tee_shm; struct tee_shm_pool; -- 2.7.4
Re: [PATCH] checkpatch: add --typedefsfile
On 04/20/2017 06:49 PM, Joe Perches wrote:
> On Thu, 2017-04-20 at 17:39 +0200, Jerome Forissier wrote:
>> When using checkpatch on out-of-tree code, it may occur that some
>> project-specific types are used, which will cause spurious warnings.
>> Add the --typedefsfile option as a way to extend the known types and
>> deal with this issue.
>
> I'm not opposed to the addition.
> What out-of-tree project is this for?
OP-TEE [1]. We run a Travis job on all pull requests [2], and checkpatch
is part of that. The typical false warning we get on a regular basis is
with some pointers to functions returning TEE_Result [3], which is a
typedef from the GlobalPlatform APIs. We consider it is acceptable to
use GP types in the OP-TEE core implementation, that's why this patch
would be helpful for us.
[1] https://github.com/OP-TEE/optee_os
[2] https://travis-ci.org/OP-TEE/optee_os/builds
[3] https://travis-ci.org/OP-TEE/optee_os/builds/193355335#L1733
Thanks,
--
Jerome
>
>> Signed-off-by: Jerome Forissier
>> ---
>> scripts/checkpatch.pl | 56
>> ++-
>> 1 file changed, 37 insertions(+), 19 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index baa3c7b..eb55f5f 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -55,6 +55,7 @@ my $spelling_file = "$D/spelling.txt";
>> my $codespell = 0;
>> my $codespellfile = "/usr/share/codespell/dictionary.txt";
>> my $conststructsfile = "$D/const_structs.checkpatch";
>> +my $typedefsfile = "";
>> my $color = 1;
>> my $allow_c99_comments = 1;
>>
>> @@ -113,6 +114,7 @@ Options:
>>--codespellUse the codespell dictionary for spelling/typos
>> (default:/usr/share/codespell/dictionary.txt)
>>--codespellfileUse this codespell dictionary
>> + --typedefsfile Read additional types from this file
>>--colorUse colors when output is STDOUT (default: on)
>>-h, --help, --version display this help and exit
>>
>> @@ -208,6 +210,7 @@ GetOptions(
>> 'test-only=s' => \$tst_only,
>> 'codespell!'=> \$codespell,
>> 'codespellfile=s' => \$codespellfile,
>> +'typedefsfile=s'=> \$typedefsfile,
>> 'color!'=> \$color,
>> 'h|help'=> \$help,
>> 'version' => \$help
>> @@ -629,28 +632,43 @@ if ($codespell) {
>>
>> $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix;
>>
>> +sub read_words {
>> +my ($wordsRef, $file) = @_;
>> +
>> +if (open(my $words, '<', $file)) {
>> +while (<$words>) {
>> +my $line = $_;
>> +
>> +$line =~ s/\s*\n?$//g;
>> +$line =~ s/^\s*//g;
>> +
>> +next if ($line =~ m/^\s*#/);
>> +next if ($line =~ m/^\s*$/);
>> +if ($line =~ /\s/) {
>> +print("$file: '$line' invalid - ignored\n");
>> +next;
>> +}
>> +
>> +$$wordsRef .= '|' if ($$wordsRef ne "");
>> +$$wordsRef .= $line;
>> +}
>> +close($file);
>> +return 1;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> my $const_structs = "";
>> -if (open(my $conststructs, '<', $conststructsfile)) {
>> -while (<$conststructs>) {
>> -my $line = $_;
>> +read_words(\$const_structs, $conststructsfile)
>> +or warn "No structs that should be const will be found - file
>> '$conststructsfile': $!\n";
>>
>> -$line =~ s/\s*\n?$//g;
>> -$line =~ s/^\s*//g;
>> -
>> -next if ($line =~ m/^\s*#/);
>> -next if ($line =~ m/^\s*$/);
>> -if ($line =~ /\s/) {
>> -print("$conststructsfile: '$line' invalid - ignored\n");
>> -next;
>> -}
>> -
>> -$const_structs .= '|' if ($const_structs ne "");
>> -$const_structs .= $line;
>> -}
>> -close($conststructsfile);
>> -} else {
>> -warn "No structs that should be const will be found - file
>> '$conststructsfile': $!\n";
>> +my $typeOtherTypedefs = "";
>> +if (length($typedefsfile)) {
>> +read_words(\$typeOtherTypedefs, $typedefsfile)
>> +or warn "No additional types will be considered - file
>> '$typedefsfile': $!\n";
>> }
>> +$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne "");
>>
>> sub build_types {
>> my $mods = "(?x: \n" . join("|\n ", (@modifierList,
>> @modifierListFile)) . "\n)";
[PATCH] mmc: dw_mmc: k3: add MMC_CAP_CMD23
Enables RPMB support for the on-board eMMC of the HiKey board as well
as for eMMC modules connected to the microSD slot.
Signed-off-by: Jerome Forissier
---
drivers/mmc/host/dw_mmc-k3.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
index 63c2e2e..8e9d886 100644
--- a/drivers/mmc/host/dw_mmc-k3.c
+++ b/drivers/mmc/host/dw_mmc-k3.c
@@ -32,6 +32,12 @@ struct k3_priv {
struct regmap *reg;
};
+static unsigned long dw_mci_hi6220_caps[] = {
+ MMC_CAP_CMD23,
+ MMC_CAP_CMD23,
+ 0
+};
+
static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
{
int ret;
@@ -126,6 +132,7 @@ static void dw_mci_hi6220_set_ios(struct dw_mci *host,
struct mmc_ios *ios)
}
static const struct dw_mci_drv_data hi6220_data = {
+ .caps = dw_mci_hi6220_caps,
.switch_voltage = dw_mci_hi6220_switch_voltage,
.set_ios= dw_mci_hi6220_set_ios,
.parse_dt = dw_mci_hi6220_parse_dt,
--
2.7.4
Re: [PATCH] tee: add forward declaration for struct device
[+Arnd] Ping? Thanks, -- Jerome On 05/31/2017 01:21 PM, Jerome Forissier wrote: > tee_drv.h references struct device, but does not include device.h nor > platform_device.h. Therefore, if tee_drv.h is included by some file > that does not pull device.h nor platform_device.h beforehand, we have a > compile warning. Fix this by adding a forward declaration. > > Signed-off-by: Jerome Forissier > --- > include/linux/tee_drv.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h > index 8614713..07bd226 100644 > --- a/include/linux/tee_drv.h > +++ b/include/linux/tee_drv.h > @@ -29,6 +29,7 @@ > #define TEE_SHM_DMA_BUF BIT(1) /* Memory with dma-buf handle */ > #define TEE_SHM_EXT_DMA_BUF BIT(2) /* Memory with dma-buf handle */ > > +struct device; > struct tee_device; > struct tee_shm; > struct tee_shm_pool; >
Re: [PATCH] tee: add forward declaration for struct device
On 06/14/2017 01:01 PM, Arnd Bergmann wrote: > On Wed, Jun 14, 2017 at 11:46 AM, Jens Wiklander > wrote: >> On Wed, Jun 14, 2017 at 11:39:50AM +0200, Jerome Forissier wrote: >>> [+Arnd] >>> >>> Ping? >>> >>> Thanks, >>> -- >>> Jerome >>> >>> On 05/31/2017 01:21 PM, Jerome Forissier wrote: >>>> tee_drv.h references struct device, but does not include device.h nor >>>> platform_device.h. Therefore, if tee_drv.h is included by some file >>>> that does not pull device.h nor platform_device.h beforehand, we have a >>>> compile warning. Fix this by adding a forward declaration. >>>> >>>> Signed-off-by: Jerome Forissier > > Acked-by: Arnd Bergmann > > Do we need this to fix a warning in mainline, in linux-next, or only > in combination with some other patches? Only with some other patches. -- Jerome > I have not run into this warning in my build testing. > >>>> include/linux/tee_drv.h | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h >>>> index 8614713..07bd226 100644 >>>> --- a/include/linux/tee_drv.h >>>> +++ b/include/linux/tee_drv.h >>>> @@ -29,6 +29,7 @@ >>>> #define TEE_SHM_DMA_BUFBIT(1) /* Memory with dma-buf handle >>>> */ >>>> #define TEE_SHM_EXT_DMA_BUFBIT(2) /* Memory with dma-buf handle >>>> */ >>>> >>>> +struct device; >>>> struct tee_device; >>>> struct tee_shm; >>>> struct tee_shm_pool; >>>> >> >> Looks good to me. >> >> Reviewed-by: Jens Wiklander > > Can you pick up the patch in your git tree and send a pull request for > the appropriate release (4.12-fixes or 4.13)? > > If you don't expect to send anything else for tee in that release, > you can also forward the patch to a...@kernel.org and ask for > inclusion. If you just reply with the 'Reviewed-by', I would not > expect to have to do anything in the arm-soc tree. > > Arnd >
Re: [PATCH v2] firmware: qcom: scm: Fix incorrect of_node_put call in scm_init
On 12/06/2017 09:06 PM, Stephen Boyd wrote:
> On 12/06, Loys Ollivier wrote:
>> When using other platform architectures, in the init of the qcom_scm
>> driver, of_node_put is called on /firmware if no qcom dt is found.
>> This results in a kernel error: Bad of_node_put() on /firmware.
>>
>> The call to of_node_put from the qcom_scm init is unnecessary as
>> of_find_matching_node is calling it automatically.
>>
>> Remove this of_node_put().
>>
>> Fixes: d0f6fa7ba2d6 ("firmware: qcom: scm: Convert SCM to platform driver")
>> Signed-off-by: Loys Ollivier
>> ---
>
> This still looks wrong. Especially if of_find_matching_node() is
> going to look for siblings of the /firmware node for the
> compatible string for scm device. Why do we check at all? Can't
> we just delete this and let of_platform_populate() take care of
> it? BTW, OP-TEE driver seems to have a similar problem.
https://lkml.org/lkml/2017/11/29/230
>
> ---8<
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index af4c75217ea6..440d8f796faa 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -626,23 +626,11 @@ static int __init qcom_scm_init(void)
> int ret;
>
> fw_np = of_find_node_by_name(NULL, "firmware");
> -
> if (!fw_np)
> - return -ENODEV;
> -
> - np = of_find_matching_node(fw_np, qcom_scm_dt_match);
> -
> - if (!np) {
> - of_node_put(fw_np);
> - return -ENODEV;
> - }
> -
> - of_node_put(np);
> + return 0;
>
> ret = of_platform_populate(fw_np, qcom_scm_dt_match, NULL, NULL);
> -
> of_node_put(fw_np);
> -
> if (ret)
> return ret;
>
>
Re: [Tee-dev] [PATCHv8 1/3] optee: use uuid for sysfs driver entry
On 6/24/20 5:21 PM, James Bottomley wrote: > On Wed, 2020-06-24 at 16:17 +0530, Sumit Garg wrote: >> Apologies for delay in my reply as I was busy with some other stuff. >> >> On Fri, 19 Jun 2020 at 20:30, James Bottomley >> wrote: > [...] >>> it's about consistency with what the kernel types mean. When some >>> checker detects your using little endian operations on a big endian >>> structure (like in the prink for instance) they're going to keep >>> emailing you about it. >> >> As mentioned above, using different terminology is meant to cause >> more confusion than just difference in endianness which is manageable >> inside TEE. >> >> And I think it's safe to say that the kernel implements UUID in big >> endian format and thus uses %pUb whereas OP-TEE implements UUID in >> little endian format and thus uses %pUl. > > So what I think you're saying is that if we still had uuid_be and > uuid_le you'd use uuid_le, because that's exactly the structure > described in the docs. But because we renamed > > uuid_be -> uuid_t > uuid_le -> guid_t > > You can't use guid_t as a kernel type because it has the wrong name? Let me try to clear the confusion that I introduce myself I believe :-/ IMO: - optee_register_device(const uuid_t *device_uuid) *is* the correct prototype. - device_uuid is *guaranteed* to be BE because OP-TEE makes this guarantee (it converts from its internal LE representation to BE when enumerating the devices, but it doesn't matter to the kernel). - Therefore %pUb is the correct format. I'm sorry for doubting the BE order initially. I am so used to OP-TEE using LE internally, that I missed the fact that we have an explicit conversion... Does this sound good? Thanks, -- Jerome
Re: [PATCH v5 2/4] riscv: Introduce CONFIG_RELOCATABLE
On 6/7/20 9:59 AM, Alexandre Ghiti wrote: [...] > +config RELOCATABLE > + bool > + depends on MMU > + help > + This builds a kernel as a Position Independent Executable (PIE), > + which retains all relocation metadata required to relocate the > + kernel binary at runtime to a different virtual address than the > + address it was linked at. > + Since RISCV uses the RELA relocation format, this requires a > + relocation pass at runtime even if the kernel is loaded at the > + same address it was linked at. Is this true? I thought that the GNU linker would write the "proper" values by default, contrary to the LLVM linker (ld.lld) which would need a special flag: --apply-dynamic-relocs (by default the relocated places are set to zero). At least, it is my experience with Aarch64 on a different project. So, sorry if I'm talking nonsense here -- I have not looked at the details. -- Jerome
[PATCH] checkpatch: get CONFIG_ prefix from the environment
Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
environment variable. Out-of-tree projects may therefore use Kconfig
with a different prefix, or they may use a custom configuration tool
which does not use the CONFIG_ prefix at all. Such projects may still
want to adhere to the Linux kernel coding style and run checkpatch.pl.
To make this possible, update checkpatch to use the value of $CONFIG_
if defined or "CONFIG_" otherwise.
Signed-off-by: Jerome Forissier
---
scripts/checkpatch.pl | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2cbeae6d9aee..2cf750175a71 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore
C99_COMMENT_TOLERANC
# git output parsing needs US English output, so first set backtick child
process LANGUAGE
my $git_command ='export LANGUAGE=en_US.UTF-8; git';
my $tabsize = 8;
+my $CONFIG_ = $ENV{"CONFIG_"} || "CONFIG_";
sub help {
my ($exitcode) = @_;
@@ -6528,16 +6529,16 @@ sub process {
}
# check for IS_ENABLED() without CONFIG_ ($rawline for comments too)
- if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
/^CONFIG_/) {
+ if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
/^$CONFIG_/) {
WARN("IS_ENABLED_CONFIG",
-"IS_ENABLED($1) is normally used as
IS_ENABLED(CONFIG_$1)\n" . $herecurr);
+"IS_ENABLED($1) is normally used as
IS_ENABLED($CONFIG_$1)\n" . $herecurr);
}
# check for #if defined CONFIG_ || defined CONFIG__MODULE
- if ($line =~
/^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
{
+ if ($line =~
/^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)($CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
{
my $config = $1;
if (WARN("PREFER_IS_ENABLED",
-"Prefer IS_ENABLED() to CONFIG_ ||
CONFIG__MODULE\n" . $herecurr) &&
+"Prefer IS_ENABLED() to $CONFIG_ ||
$CONFIG__MODULE\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] = "\+#if
IS_ENABLED($config)";
}
--
2.25.1
Re: [PATCH] checkpatch: get CONFIG_ prefix from the environment
On 8/17/20 4:09 PM, Joe Perches wrote:
> On Mon, 2020-08-17 at 11:50 +0200, Jerome Forissier wrote:
>> Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
>> environment variable. Out-of-tree projects may therefore use Kconfig
>> with a different prefix, or they may use a custom configuration tool
>> which does not use the CONFIG_ prefix at all. Such projects may still
>> want to adhere to the Linux kernel coding style and run checkpatch.pl.
>> To make this possible, update checkpatch to use the value of $CONFIG_
>> if defined or "CONFIG_" otherwise.
>>
>> Signed-off-by: Jerome Forissier
>> ---
>> scripts/checkpatch.pl | 9 +
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 2cbeae6d9aee..2cf750175a71 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by
>> --ignore C99_COMMENT_TOLERANC
>> # git output parsing needs US English output, so first set backtick child
>> process LANGUAGE
>> my $git_command ='export LANGUAGE=en_US.UTF-8; git';
>> my $tabsize = 8;
>> +my $CONFIG_ = $ENV{"CONFIG_"} || "CONFIG_";
>
> I'm not a big fan of environment variable being
> used to control program behavior.
OK.
>
> Maybe add something to .checkpatch.conf instead.
That would work equally well for me. I will post a V2.
Thanks,
--
Jerome
>
>> sub help {
>> my ($exitcode) = @_;
>> @@ -6528,16 +6529,16 @@ sub process {
>> }
>>
>> # check for IS_ENABLED() without CONFIG_ ($rawline for comments too)
>> -if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
>> /^CONFIG_/) {
>> +if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
>> /^$CONFIG_/) {
>> WARN("IS_ENABLED_CONFIG",
>> - "IS_ENABLED($1) is normally used as
>> IS_ENABLED(CONFIG_$1)\n" . $herecurr);
>> + "IS_ENABLED($1) is normally used as
>> IS_ENABLED($CONFIG_$1)\n" . $herecurr);
>> }
>>
>> # check for #if defined CONFIG_ || defined CONFIG__MODULE
>> -if ($line =~
>> /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
>> {
>> +if ($line =~
>> /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)($CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
>> {
>> my $config = $1;
>> if (WARN("PREFER_IS_ENABLED",
>> - "Prefer IS_ENABLED() to CONFIG_ ||
>> CONFIG__MODULE\n" . $herecurr) &&
>> + "Prefer IS_ENABLED() to $CONFIG_ ||
>> $CONFIG__MODULE\n" . $herecurr) &&
>> $fix) {
>> $fixed[$fixlinenr] = "\+#if
>> IS_ENABLED($config)";
>> }
>
[PATCH v2] checkpatch: add --kconfig-prefix
Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
environment variable. Out-of-tree projects may therefore use Kconfig
with a different prefix, or they may use a custom configuration tool
which does not use the CONFIG_ prefix at all. Such projects may still
want to adhere to the Linux kernel coding style and run checkpatch.pl.
To make this possible, add the --kconfig-prefix command line option.
Signed-off-by: Jerome Forissier
---
scripts/checkpatch.pl | 12
1 file changed, 8 insertions(+), 4 deletions(-)
v2:
- Use a command-line/.checkpatch.conf option instead of the _CONFIG
environment variable.
- Changed the patch subject (was: "checkpatch: get CONFIG_ prefix from
the environment").
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2cbeae6d9aee..150dfbc04b4b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore
C99_COMMENT_TOLERANC
# git output parsing needs US English output, so first set backtick child
process LANGUAGE
my $git_command ='export LANGUAGE=en_US.UTF-8; git';
my $tabsize = 8;
+my $CONFIG_ = "CONFIG_";
sub help {
my ($exitcode) = @_;
@@ -127,6 +128,8 @@ Options:
--typedefsfile Read additional types from this file
--color[=WHEN] Use colors 'always', 'never', or only when output
is a terminal ('auto'). Default is 'auto'.
+ --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default
+ $CONFIG_)
-h, --help, --version display this help and exit
When FILE is - read standard input.
@@ -235,6 +238,7 @@ GetOptions(
'color=s' => \$color,
'no-color' => \$color, #keep old behaviors of -nocolor
'nocolor' => \$color, #keep old behaviors of -nocolor
+ 'kconfig-prefix=s' => \$CONFIG_,
'h|help'=> \$help,
'version' => \$help
) or help(1);
@@ -6528,16 +6532,16 @@ sub process {
}
# check for IS_ENABLED() without CONFIG_ ($rawline for comments too)
- if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
/^CONFIG_/) {
+ if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
/^$CONFIG_/) {
WARN("IS_ENABLED_CONFIG",
-"IS_ENABLED($1) is normally used as
IS_ENABLED(CONFIG_$1)\n" . $herecurr);
+"IS_ENABLED($1) is normally used as
IS_ENABLED($CONFIG_$1)\n" . $herecurr);
}
# check for #if defined CONFIG_ || defined CONFIG__MODULE
- if ($line =~
/^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
{
+ if ($line =~
/^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)($CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
{
my $config = $1;
if (WARN("PREFER_IS_ENABLED",
-"Prefer IS_ENABLED() to CONFIG_ ||
CONFIG__MODULE\n" . $herecurr) &&
+"Prefer IS_ENABLED() to $CONFIG_ ||
$CONFIG__MODULE\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] = "\+#if
IS_ENABLED($config)";
}
--
2.25.1
[PATCH v3] checkpatch: add --kconfig-prefix
Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
environment variable. Out-of-tree projects may therefore use Kconfig
with a different prefix, or they may use a custom configuration tool
which does not use the CONFIG_ prefix at all. Such projects may still
want to adhere to the Linux kernel coding style and run checkpatch.pl.
One example is OP-TEE [1] which does not use Kconfig but does have
configuration options prefixed with CFG_. It also mostly follows the
kernel coding style and therefore being able to use checkpatch is quite
valuable.
To make this possible, add the --kconfig-prefix command line option.
Signed-off-by: Jerome Forissier
---
scripts/checkpatch.pl | 12
1 file changed, 8 insertions(+), 4 deletions(-)
v3:
- Use ${CONFIG_} instead of $CONFIG_.
- Expand the commit message to mention OP-TEE.
v2:
- Use a command-line/.checkpatch.conf option instead of the _CONFIG
environment variable.
- Changed the patch subject (was: "checkpatch: get CONFIG_ prefix from
the environment").
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2cbeae6d9aee..fd65f8c774ed 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore
C99_COMMENT_TOLERANC
# git output parsing needs US English output, so first set backtick child
process LANGUAGE
my $git_command ='export LANGUAGE=en_US.UTF-8; git';
my $tabsize = 8;
+my ${CONFIG_} = "CONFIG_";
sub help {
my ($exitcode) = @_;
@@ -127,6 +128,8 @@ Options:
--typedefsfile Read additional types from this file
--color[=WHEN] Use colors 'always', 'never', or only when output
is a terminal ('auto'). Default is 'auto'.
+ --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default
+ ${CONFIG_})
-h, --help, --version display this help and exit
When FILE is - read standard input.
@@ -235,6 +238,7 @@ GetOptions(
'color=s' => \$color,
'no-color' => \$color, #keep old behaviors of -nocolor
'nocolor' => \$color, #keep old behaviors of -nocolor
+ 'kconfig-prefix=s' => \${CONFIG_},
'h|help'=> \$help,
'version' => \$help
) or help(1);
@@ -6528,16 +6532,16 @@ sub process {
}
# check for IS_ENABLED() without CONFIG_ ($rawline for comments too)
- if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
/^CONFIG_/) {
+ if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
/^${CONFIG_}/) {
WARN("IS_ENABLED_CONFIG",
-"IS_ENABLED($1) is normally used as
IS_ENABLED(CONFIG_$1)\n" . $herecurr);
+"IS_ENABLED($1) is normally used as
IS_ENABLED(${CONFIG_}$1)\n" . $herecurr);
}
# check for #if defined CONFIG_ || defined CONFIG__MODULE
- if ($line =~
/^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
{
+ if ($line =~
/^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(${CONFIG_}[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
{
my $config = $1;
if (WARN("PREFER_IS_ENABLED",
-"Prefer IS_ENABLED() to CONFIG_ ||
CONFIG__MODULE\n" . $herecurr) &&
+"Prefer IS_ENABLED() to ${CONFIG_}
|| ${CONFIG_}_MODULE\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] = "\+#if
IS_ENABLED($config)";
}
--
2.25.1
Re: [PATCH v3] checkpatch: add --kconfig-prefix
On 8/18/20 9:43 AM, Jerome Forissier wrote:
> Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
> environment variable. Out-of-tree projects may therefore use Kconfig
> with a different prefix, or they may use a custom configuration tool
> which does not use the CONFIG_ prefix at all. Such projects may still
> want to adhere to the Linux kernel coding style and run checkpatch.pl.
>
> One example is OP-TEE [1] which does not use Kconfig but does have
> configuration options prefixed with CFG_. It also mostly follows the
> kernel coding style and therefore being able to use checkpatch is quite
> valuable.
>
> To make this possible, add the --kconfig-prefix command line option.
(Oh I forgot to add the link here :-/ sorry about that. Let me know if
you want me to resend.)
[1] https://github.com/OP-TEE/optee_os
>
> Signed-off-by: Jerome Forissier
> ---
> scripts/checkpatch.pl | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> v3:
> - Use ${CONFIG_} instead of $CONFIG_.
> - Expand the commit message to mention OP-TEE.
>
> v2:
> - Use a command-line/.checkpatch.conf option instead of the _CONFIG
> environment variable.
> - Changed the patch subject (was: "checkpatch: get CONFIG_ prefix from
> the environment").
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 2cbeae6d9aee..fd65f8c774ed 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore
> C99_COMMENT_TOLERANC
> # git output parsing needs US English output, so first set backtick child
> process LANGUAGE
> my $git_command ='export LANGUAGE=en_US.UTF-8; git';
> my $tabsize = 8;
> +my ${CONFIG_} = "CONFIG_";
>
> sub help {
> my ($exitcode) = @_;
> @@ -127,6 +128,8 @@ Options:
>--typedefsfile Read additional types from this file
>--color[=WHEN] Use colors 'always', 'never', or only when
> output
> is a terminal ('auto'). Default is 'auto'.
> + --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols
> (default
> + ${CONFIG_})
>-h, --help, --version display this help and exit
>
> When FILE is - read standard input.
> @@ -235,6 +238,7 @@ GetOptions(
> 'color=s' => \$color,
> 'no-color' => \$color, #keep old behaviors of -nocolor
> 'nocolor' => \$color, #keep old behaviors of -nocolor
> + 'kconfig-prefix=s' => \${CONFIG_},
> 'h|help'=> \$help,
> 'version' => \$help
> ) or help(1);
> @@ -6528,16 +6532,16 @@ sub process {
> }
>
> # check for IS_ENABLED() without CONFIG_ ($rawline for comments too)
> - if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
> /^CONFIG_/) {
> + if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
> /^${CONFIG_}/) {
> WARN("IS_ENABLED_CONFIG",
> - "IS_ENABLED($1) is normally used as
> IS_ENABLED(CONFIG_$1)\n" . $herecurr);
> + "IS_ENABLED($1) is normally used as
> IS_ENABLED(${CONFIG_}$1)\n" . $herecurr);
> }
>
> # check for #if defined CONFIG_ || defined CONFIG__MODULE
> - if ($line =~
> /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
> {
> + if ($line =~
> /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(${CONFIG_}[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
> {
> my $config = $1;
> if (WARN("PREFER_IS_ENABLED",
> - "Prefer IS_ENABLED() to CONFIG_ ||
> CONFIG__MODULE\n" . $herecurr) &&
> + "Prefer IS_ENABLED() to ${CONFIG_}
> || ${CONFIG_}_MODULE\n" . $herecurr) &&
> $fix) {
> $fixed[$fixlinenr] = "\+#if
> IS_ENABLED($config)";
> }
>
[PATCH v4] checkpatch: add --kconfig-prefix
Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
environment variable. Out-of-tree projects may therefore use Kconfig
with a different prefix, or they may use a custom configuration tool
which does not use the CONFIG_ prefix at all. Such projects may still
want to adhere to the Linux kernel coding style and run checkpatch.pl.
One example is OP-TEE [1] which does not use Kconfig but does have
configuration options prefixed with CFG_. It also mostly follows the
kernel coding style and therefore being able to use checkpatch is quite
valuable.
To make this possible, add the --kconfig-prefix command line option.
[1] https://github.com/OP-TEE/optee_os
Signed-off-by: Jerome Forissier
Acked-by: Joe Perches
---
scripts/checkpatch.pl | 12
1 file changed, 8 insertions(+), 4 deletions(-)
v4:
- Add missing link
- Apply Joe's Acked-by: tag
v3:
- Use ${CONFIG_} instead of $CONFIG_.
- Expand the commit message to mention OP-TEE.
v2:
- Use a command-line/.checkpatch.conf option instead of the _CONFIG
environment variable.
- Changed the patch subject (was: "checkpatch: get CONFIG_ prefix from
the environment").
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2cbeae6d9aee..fd65f8c774ed 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore
C99_COMMENT_TOLERANC
# git output parsing needs US English output, so first set backtick child
process LANGUAGE
my $git_command ='export LANGUAGE=en_US.UTF-8; git';
my $tabsize = 8;
+my ${CONFIG_} = "CONFIG_";
sub help {
my ($exitcode) = @_;
@@ -127,6 +128,8 @@ Options:
--typedefsfile Read additional types from this file
--color[=WHEN] Use colors 'always', 'never', or only when output
is a terminal ('auto'). Default is 'auto'.
+ --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default
+ ${CONFIG_})
-h, --help, --version display this help and exit
When FILE is - read standard input.
@@ -235,6 +238,7 @@ GetOptions(
'color=s' => \$color,
'no-color' => \$color, #keep old behaviors of -nocolor
'nocolor' => \$color, #keep old behaviors of -nocolor
+ 'kconfig-prefix=s' => \${CONFIG_},
'h|help'=> \$help,
'version' => \$help
) or help(1);
@@ -6528,16 +6532,16 @@ sub process {
}
# check for IS_ENABLED() without CONFIG_ ($rawline for comments too)
- if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
/^CONFIG_/) {
+ if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~
/^${CONFIG_}/) {
WARN("IS_ENABLED_CONFIG",
-"IS_ENABLED($1) is normally used as
IS_ENABLED(CONFIG_$1)\n" . $herecurr);
+"IS_ENABLED($1) is normally used as
IS_ENABLED(${CONFIG_}$1)\n" . $herecurr);
}
# check for #if defined CONFIG_ || defined CONFIG__MODULE
- if ($line =~
/^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
{
+ if ($line =~
/^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(${CONFIG_}[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/)
{
my $config = $1;
if (WARN("PREFER_IS_ENABLED",
-"Prefer IS_ENABLED() to CONFIG_ ||
CONFIG__MODULE\n" . $herecurr) &&
+"Prefer IS_ENABLED() to ${CONFIG_}
|| ${CONFIG_}_MODULE\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] = "\+#if
IS_ENABLED($config)";
}
--
2.25.1
Re: [Tee-dev] [PATCHv8 1/3] optee: use uuid for sysfs driver entry
On 6/17/20 3:58 PM, Sumit Garg wrote:
> Hi Maxim,
>
> On Thu, 4 Jun 2020 at 23:28, Maxim Uvarov wrote:
>>
>> With the evolving use-cases for TEE bus, now it's required to support
>> multi-stage enumeration process. But using a simple index doesn't
>> suffice this requirement and instead leads to duplicate sysfs entries.
>> So instead switch to use more informative device UUID for sysfs entry
>> like:
>> /sys/bus/tee/devices/optee-ta-
>>
>> Signed-off-by: Maxim Uvarov
>> Reviewed-by: Sumit Garg
>> ---
>> Documentation/ABI/testing/sysfs-bus-optee-devices | 8
>> MAINTAINERS | 1 +
>> drivers/tee/optee/device.c| 9 ++---
>> 3 files changed, 15 insertions(+), 3 deletions(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-optee-devices
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-optee-devices
>> b/Documentation/ABI/testing/sysfs-bus-optee-devices
>> new file mode 100644
>> index ..0ae04ae5374a
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-optee-devices
>> @@ -0,0 +1,8 @@
>> +What: /sys/bus/tee/devices/optee-ta-/
>> +Date: May 2020
>> +KernelVersion 5.7
>> +Contact:tee-...@lists.linaro.org
>> +Description:
>> + OP-TEE bus provides reference to registered drivers under
>> this directory. The
>> + matches Trusted Application (TA) driver and corresponding TA
>> in secure OS. Drivers
>> + are free to create needed API under optee-ta-
>> directory.
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ecc0749810b0..6717afef2de3 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -12516,6 +12516,7 @@ OP-TEE DRIVER
>> M: Jens Wiklander
>> L: tee-...@lists.linaro.org
>> S: Maintained
>> +F: Documentation/ABI/testing/sysfs-bus-optee-devices
>> F: drivers/tee/optee/
>>
>> OP-TEE RANDOM NUMBER GENERATOR (RNG) DRIVER
>> diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c
>> index e3a148521ec1..23d264c8146e 100644
>> --- a/drivers/tee/optee/device.c
>> +++ b/drivers/tee/optee/device.c
>> @@ -65,7 +65,7 @@ static int get_devices(struct tee_context *ctx, u32
>> session,
>> return 0;
>> }
>>
>> -static int optee_register_device(const uuid_t *device_uuid, u32 device_id)
>> +static int optee_register_device(const uuid_t *device_uuid)
>> {
>> struct tee_client_device *optee_device = NULL;
>> int rc;
>> @@ -75,7 +75,10 @@ static int optee_register_device(const uuid_t
>> *device_uuid, u32 device_id)
>> return -ENOMEM;
>>
>> optee_device->dev.bus = &tee_bus_type;
>> - dev_set_name(&optee_device->dev, "optee-clnt%u", device_id);
>> + if (dev_set_name(&optee_device->dev, "optee-ta-%pUl", device_uuid)) {
>
> You should be using format specifier as: "%pUb" instead of "%pUl" as
> UUID representation for TAs is in big endian format. See below:
Where does device_uuid come from? If it comes directly from OP-TEE, then
it should be a pointer to the following struct:
typedef struct
{
uint32_t timeLow;
uint16_t timeMid;
uint16_t timeHiAndVersion;
uint8_t clockSeqAndNode[8];
} TEE_UUID;
(GlobalPlatform TEE Internal Core API spec v1.2.1 section 3.2.4)
- The spec does not mandate any particular endianness and simply warns
about possible issues if secure and non-secure worlds differ in endianness.
- OP-TEE uses %pUl assuming that host order is little endian (that is
true for the Arm platforms that run OP-TEE currently). By the same logic
%pUl should be fine in the kernel.
- On the other hand, the UUID in a Trusted App header is always encoded
big endian by the Python script that signs and optionally encrypts the
TA. This should not have any visible impact on UUIDs exchanged between
the secure and non-secure world though.
So I am wondering why you had to use %pUb. There must be some
inconsistency somewhere :-/
--
Jerome
>
> # ls /sys/bus/tee/devices/
> optee-ta-405b6ad9-e5c3-e321-8794-1002a5d5c61b
> optee-ta-71d950bc-c9d4-c442-82cb-343fb7f37896
> optee-ta-e70f4af0-5d1f-9b4b-abf7-619b85b4ce8c
>
> While UUID for fTPM TA is in big endian format:
> bc50d971-d4c9-42c4-82cb-343fb7f37896
>
> Sorry that I missed it during review and noticed this while testing.
>
> With the above fix included, I tested this series using fTPM early TA
> on Qemu for aarch64 and used basic random number generation test using
> tpm2-tools. So feel free to add:
>
> Tested-by: Sumit Garg
>
> -Sumit
>
>> + kfree(optee_device);
>> + return -ENOMEM;
>> + }
>> uuid_copy(&optee_device->id.uuid, device_uuid);
>>
>> rc = device_register(&optee_device->dev);
>> @@ -144,7 +147,7 @@ int optee_enumerate_devices(void)
>> num_devices = shm_size / sizeof(uuid_t);
>>
>> for (idx = 0; idx < num_devices; idx++) {
>> - rc = optee_register_device(&device_uuid[idx]
Re: [Tee-dev] [PATCHv8 1/3] optee: use uuid for sysfs driver entry
On 6/17/20 9:52 PM, Maxim Uvarov wrote:
> On Wed, 17 Jun 2020 at 18:16, Jerome Forissier wrote:
>>
>> On 6/17/20 3:58 PM, Sumit Garg wrote:
>>> Hi Maxim,
>>>
>>> On Thu, 4 Jun 2020 at 23:28, Maxim Uvarov wrote:
>>>>
>>>> With the evolving use-cases for TEE bus, now it's required to support
>>>> multi-stage enumeration process. But using a simple index doesn't
>>>> suffice this requirement and instead leads to duplicate sysfs entries.
>>>> So instead switch to use more informative device UUID for sysfs entry
>>>> like:
>>>> /sys/bus/tee/devices/optee-ta-
>>>>
>>>> Signed-off-by: Maxim Uvarov
>>>> Reviewed-by: Sumit Garg
>>>> ---
>>>> Documentation/ABI/testing/sysfs-bus-optee-devices | 8
>>>> MAINTAINERS | 1 +
>>>> drivers/tee/optee/device.c| 9 ++---
>>>> 3 files changed, 15 insertions(+), 3 deletions(-)
>>>> create mode 100644 Documentation/ABI/testing/sysfs-bus-optee-devices
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-optee-devices
>>>> b/Documentation/ABI/testing/sysfs-bus-optee-devices
>>>> new file mode 100644
>>>> index ..0ae04ae5374a
>>>> --- /dev/null
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-optee-devices
>>>> @@ -0,0 +1,8 @@
>>>> +What: /sys/bus/tee/devices/optee-ta-/
>>>> +Date: May 2020
>>>> +KernelVersion 5.7
>>>> +Contact:tee-...@lists.linaro.org
>>>> +Description:
>>>> + OP-TEE bus provides reference to registered drivers under
>>>> this directory. The
>>>> + matches Trusted Application (TA) driver and corresponding
>>>> TA in secure OS. Drivers
>>>> + are free to create needed API under optee-ta-
>>>> directory.
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index ecc0749810b0..6717afef2de3 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -12516,6 +12516,7 @@ OP-TEE DRIVER
>>>> M: Jens Wiklander
>>>> L: tee-...@lists.linaro.org
>>>> S: Maintained
>>>> +F: Documentation/ABI/testing/sysfs-bus-optee-devices
>>>> F: drivers/tee/optee/
>>>>
>>>> OP-TEE RANDOM NUMBER GENERATOR (RNG) DRIVER
>>>> diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c
>>>> index e3a148521ec1..23d264c8146e 100644
>>>> --- a/drivers/tee/optee/device.c
>>>> +++ b/drivers/tee/optee/device.c
>>>> @@ -65,7 +65,7 @@ static int get_devices(struct tee_context *ctx, u32
>>>> session,
>>>> return 0;
>>>> }
>>>>
>>>> -static int optee_register_device(const uuid_t *device_uuid, u32 device_id)
>>>> +static int optee_register_device(const uuid_t *device_uuid)
>>>> {
>>>> struct tee_client_device *optee_device = NULL;
>>>> int rc;
>>>> @@ -75,7 +75,10 @@ static int optee_register_device(const uuid_t
>>>> *device_uuid, u32 device_id)
>>>> return -ENOMEM;
>>>>
>>>> optee_device->dev.bus = &tee_bus_type;
>>>> - dev_set_name(&optee_device->dev, "optee-clnt%u", device_id);
>>>> + if (dev_set_name(&optee_device->dev, "optee-ta-%pUl",
>>>> device_uuid)) {
>>>
>>> You should be using format specifier as: "%pUb" instead of "%pUl" as
>>> UUID representation for TAs is in big endian format. See below:
>>
>> Where does device_uuid come from? If it comes directly from OP-TEE, then
>> it should be a pointer to the following struct:
>>
>> typedef struct
>> {
>> uint32_t timeLow;
>> uint16_t timeMid;
>> uint16_t timeHiAndVersion;
>> uint8_t clockSeqAndNode[8];
>> } TEE_UUID;
>>
>> (GlobalPlatform TEE Internal Core API spec v1.2.1 section 3.2.4)
>>
>> - The spec does not mandate any particular endianness and simply warns
>> about possible issues if secure and non-secure worlds differ in endianness.
>> - OP-TEE uses %pUl assuming that host order is little endian (that is
>> true for the Arm platforms that run OP-TEE currently). By the same logic
>> %pUl
Re: [Tee-dev] [PATCHv8 1/3] optee: use uuid for sysfs driver entry
On 6/18/20 6:59 AM, Sumit Garg wrote:
> Hi Jerome,
>
> On Wed, 17 Jun 2020 at 20:46, Jerome Forissier wrote:
>>
>>
>>
>> On 6/17/20 3:58 PM, Sumit Garg wrote:
>>> Hi Maxim,
>>>
>>> On Thu, 4 Jun 2020 at 23:28, Maxim Uvarov wrote:
>>>>
>>>> With the evolving use-cases for TEE bus, now it's required to support
>>>> multi-stage enumeration process. But using a simple index doesn't
>>>> suffice this requirement and instead leads to duplicate sysfs entries.
>>>> So instead switch to use more informative device UUID for sysfs entry
>>>> like:
>>>> /sys/bus/tee/devices/optee-ta-
>>>>
>>>> Signed-off-by: Maxim Uvarov
>>>> Reviewed-by: Sumit Garg
>>>> ---
>>>> Documentation/ABI/testing/sysfs-bus-optee-devices | 8
>>>> MAINTAINERS | 1 +
>>>> drivers/tee/optee/device.c| 9 ++---
>>>> 3 files changed, 15 insertions(+), 3 deletions(-)
>>>> create mode 100644 Documentation/ABI/testing/sysfs-bus-optee-devices
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-optee-devices
>>>> b/Documentation/ABI/testing/sysfs-bus-optee-devices
>>>> new file mode 100644
>>>> index ..0ae04ae5374a
>>>> --- /dev/null
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-optee-devices
>>>> @@ -0,0 +1,8 @@
>>>> +What: /sys/bus/tee/devices/optee-ta-/
>>>> +Date: May 2020
>>>> +KernelVersion 5.7
>>>> +Contact:tee-...@lists.linaro.org
>>>> +Description:
>>>> + OP-TEE bus provides reference to registered drivers under
>>>> this directory. The
>>>> + matches Trusted Application (TA) driver and corresponding
>>>> TA in secure OS. Drivers
>>>> + are free to create needed API under optee-ta-
>>>> directory.
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index ecc0749810b0..6717afef2de3 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -12516,6 +12516,7 @@ OP-TEE DRIVER
>>>> M: Jens Wiklander
>>>> L: tee-...@lists.linaro.org
>>>> S: Maintained
>>>> +F: Documentation/ABI/testing/sysfs-bus-optee-devices
>>>> F: drivers/tee/optee/
>>>>
>>>> OP-TEE RANDOM NUMBER GENERATOR (RNG) DRIVER
>>>> diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c
>>>> index e3a148521ec1..23d264c8146e 100644
>>>> --- a/drivers/tee/optee/device.c
>>>> +++ b/drivers/tee/optee/device.c
>>>> @@ -65,7 +65,7 @@ static int get_devices(struct tee_context *ctx, u32
>>>> session,
>>>> return 0;
>>>> }
>>>>
>>>> -static int optee_register_device(const uuid_t *device_uuid, u32 device_id)
>>>> +static int optee_register_device(const uuid_t *device_uuid)
>>>> {
>>>> struct tee_client_device *optee_device = NULL;
>>>> int rc;
>>>> @@ -75,7 +75,10 @@ static int optee_register_device(const uuid_t
>>>> *device_uuid, u32 device_id)
>>>> return -ENOMEM;
>>>>
>>>> optee_device->dev.bus = &tee_bus_type;
>>>> - dev_set_name(&optee_device->dev, "optee-clnt%u", device_id);
>>>> + if (dev_set_name(&optee_device->dev, "optee-ta-%pUl",
>>>> device_uuid)) {
>>>
>>> You should be using format specifier as: "%pUb" instead of "%pUl" as
>>> UUID representation for TAs is in big endian format. See below:
>>
>> Where does device_uuid come from? If it comes directly from OP-TEE, then
>> it should be a pointer to the following struct:
>>
>> typedef struct
>> {
>> uint32_t timeLow;
>> uint16_t timeMid;
>> uint16_t timeHiAndVersion;
>> uint8_t clockSeqAndNode[8];
>> } TEE_UUID;
>>
>> (GlobalPlatform TEE Internal Core API spec v1.2.1 section 3.2.4)
>>
>> - The spec does not mandate any particular endianness and simply warns
>> about possible issues if secure and non-secure worlds differ in endianness.
>> - OP-TEE uses %pUl assuming that host order is little endian (that is
>> true for the Arm platforms that run OP-TEE currently). By the same logic
>> %pUl should be fine in the kernel.
>> - On the other hand, the UUID in a Trusted App header is always encoded
>> big endian by the Python script that signs and optionally encrypts the
>> TA. This should not have any visible impact on UUIDs exchanged between
>> the secure and non-secure world though.
>>
>> So I am wondering why you had to use %pUb. There must be some
>> inconsistency somewhere :-/
>
> Yes there is. Linux stores UUID in big endian format (16 byte octets)
> and OP-TEE stores UUID in little endian format (in form of struct you
> referenced above).
>
> And format conversion APIs [1] in OP-TEE OS are used while passing
> UUID among Linux and OP-TEE.
>
> So we need to use %pUb in case of Linux and %pUl in case of OP-TEE.
>
> [1] https://github.com/OP-TEE/optee_os/blob/master/core/tee/uuid.c
Got it now. The TA enumeration function in OP-TEE performs the
conversion here:
https://github.com/OP-TEE/optee_os/blob/3.9.0/core/pta/device.c#L34
Thanks for clarifying.
--
Jerome
