[RFC PATCH v5 08/11] ipe: add property for signed dmverity volumes
Allow IPE to leverage the stacked security blob infrastructure, and enlighten IPE to the block_device security blob. This allows IPE to have a property to express rules around a device-mapper verity volume whose root-hash has been signed, and the signature has been verified against the system keyring. This is property is also added in this patch. Signed-off-by: Deven Bowers --- security/ipe/Kconfig | 2 +- security/ipe/Makefile| 1 + security/ipe/ipe-blobs.c | 84 security/ipe/ipe-blobs.h | 18 + security/ipe/ipe-engine.c| 4 + security/ipe/ipe-engine.h| 9 +++ security/ipe/ipe-hooks.c | 1 + security/ipe/ipe-hooks.h | 7 ++ security/ipe/ipe.c | 18 + security/ipe/ipe.h | 2 + security/ipe/properties/Kconfig | 10 +++ security/ipe/properties/Makefile | 1 + security/ipe/properties/dmverity-signature.c | 82 +++ security/ipe/properties/prop-entry.h | 9 +++ security/ipe/utility.h | 10 +++ 15 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 security/ipe/ipe-blobs.c create mode 100644 security/ipe/ipe-blobs.h create mode 100644 security/ipe/properties/dmverity-signature.c diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig index 469ef78c2f4f..11b50ef9abca 100644 --- a/security/ipe/Kconfig +++ b/security/ipe/Kconfig @@ -5,7 +5,7 @@ menuconfig SECURITY_IPE bool "Integrity Policy Enforcement (IPE)" - depends on SECURITY && AUDIT + depends on SECURITY && AUDIT && BLOCK select SYSTEM_DATA_VERIFICATION select SECURITYFS select CRYPTO_SHA1 diff --git a/security/ipe/Makefile b/security/ipe/Makefile index 7e98982c5035..98a2245b6956 100644 --- a/security/ipe/Makefile +++ b/security/ipe/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_SECURITY_IPE) += \ ipe-property.o \ ipe-hooks.o \ ipe-secfs.o \ + ipe-blobs.o \ clean-files := ipe-bp.c diff --git a/security/ipe/ipe-blobs.c b/security/ipe/ipe-blobs.c new file mode 100644 index ..041d7d47b723 --- /dev/null +++ b/security/ipe/ipe-blobs.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) Microsoft Corporation. All rights reserved. + */ + +#include "ipe.h" +#include "ipe-engine.h" +#include "ipe-blobs.h" + +#include +#include +#include + +/** + * ipe_bdev_alloc_security: Performs the initialization of IPE's security blob. + * @bdev: The block device to source the security blob from. + * + * The allocation is performed earlier by the LSM infrastructure, + * (on behalf of all LSMs) in lsm_alloc_bdev. At the moment, IPE uses + * this time to zero out the region of memory reserved for IPE. + * + * Return: + * 0 - OK + */ +int ipe_bdev_alloc_security(struct block_device *bdev) +{ + struct ipe_bdev_blob *bdev_sec = ipe_bdev(bdev); + + memset(bdev_sec, 0x0, sizeof(*bdev_sec)); + + return 0; +} + +/** + * ipe_bdev_free_security: Frees all fields of IPE's block dev security blob. + * @bdev: The block device to source the security blob from. + * + * The deallocation of the blob itself is performed later by the LSM + * infrastructure, (on behalf of all LSMs) in lsm_free_bdev. + * + * Pointers allocated by the bdev_setsecurity hook and alloc_security + * hook need to be deallocated here. + */ +void ipe_bdev_free_security(struct block_device *bdev) +{ + struct ipe_bdev_blob *bdev_sec = ipe_bdev(bdev); + + kfree(bdev_sec->dmverity_rh_sig); + + memset(bdev_sec, 0x0, sizeof(*bdev_sec)); +} + +/** + * ipe_bdev_setsecurity: Sets the a certain field of a block device security + * blob, based on @key. + * @bdev: The block device to source the security blob from. + * @key: The key representing the information to be stored. + * @value: The value to be stored. + * @len: The length of @value. + * + * As block-devices are a generic implementation across specific stacks, + * this allows information to be stored from various stacks. + * + * Return: + * 0 - OK + * !0 - Error + */ +int ipe_bdev_setsecurity(struct block_device *bdev, const char *key, +const void *value, size_t len) +{ + struct ipe_bdev_blob *bdev_sec = ipe_bdev(bdev); + + if (!strcmp(key, DM_VERITY_SIGNATURE_SEC_NAME)) { + bdev_sec->dmverity_rh_sig = kmemdup(value, len, GFP_KERNEL); + if (!bdev_sec->dmverity_rh_sig) + return -ENOMEM; + + bdev_sec->dmv_rh_sig_len = len; + + return 0; + } + + return -ENOSYS; +} diff --git a/security/ipe/ipe-blobs.h b/security/ipe/ipe-blobs.h new file mode 100644 index ..7561f4cef558 --- /dev/null +++ b/security/ipe/ipe-blobs.h @@ -0,0 +1,18 @@ +/*
[RFC PATCH v5 05/11] fs: add security blob and hooks for block_device
Add a security blob and associated allocation, deallocation and set hooks for a block_device structure. Signed-off-by: Deven Bowers --- fs/block_dev.c| 8 include/linux/fs.h| 1 + include/linux/lsm_hook_defs.h | 5 +++ include/linux/lsm_hooks.h | 12 ++ include/linux/security.h | 22 +++ security/security.c | 74 +++ 6 files changed, 122 insertions(+) diff --git a/fs/block_dev.c b/fs/block_dev.c index 0ae656e022fd..8602dd62c3e2 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -34,6 +34,7 @@ #include #include #include +#include #include "internal.h" struct bdev_inode { @@ -768,11 +769,18 @@ static struct inode *bdev_alloc_inode(struct super_block *sb) struct bdev_inode *ei = kmem_cache_alloc(bdev_cachep, GFP_KERNEL); if (!ei) return NULL; + + if (unlikely(security_bdev_alloc(>bdev))) { + kmem_cache_free(bdev_cachep, ei); + return NULL; + } + return >vfs_inode; } static void bdev_free_inode(struct inode *inode) { + security_bdev_free(_I(inode)->bdev); kmem_cache_free(bdev_cachep, BDEV_I(inode)); } diff --git a/include/linux/fs.h b/include/linux/fs.h index f5abba86107d..42d7e3ce7712 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -509,6 +509,7 @@ struct block_device { int bd_fsfreeze_count; /* Mutex for freeze */ struct mutexbd_fsfreeze_mutex; + void*security; } __randomize_layout; /* XArray tags, for tagging dirty and writeback pages in the pagecache. */ diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index af998f93d256..f3c0da0db4e8 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -391,3 +391,8 @@ LSM_HOOK(void, LSM_RET_VOID, perf_event_free, struct perf_event *event) LSM_HOOK(int, 0, perf_event_read, struct perf_event *event) LSM_HOOK(int, 0, perf_event_write, struct perf_event *event) #endif /* CONFIG_PERF_EVENTS */ + +LSM_HOOK(int, 0, bdev_alloc_security, struct block_device *bdev) +LSM_HOOK(void, LSM_RET_VOID, bdev_free_security, struct block_device *bdev) +LSM_HOOK(int, 0, bdev_setsecurity, struct block_device *bdev, const char *name, +const void *value, size_t size) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 95b7c1d32062..8670c19a8cef 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1507,6 +1507,17 @@ * * @what: kernel feature being accessed * + * @bdev_alloc_security: + * Initialize the security field inside a block_device structure. + * + * @bdev_free_security: + * Cleanup the security information stored inside a block_device structure. + * + * @bdev_setsecurity: + * Set a security property associated with @name for @bdev with + * value @value. @size indicates the size of @value in bytes. + * If a @name is not implemented, return -ENOSYS. + * * Security hooks for perf events * * @perf_event_open: @@ -1553,6 +1564,7 @@ struct lsm_blob_sizes { int lbs_ipc; int lbs_msg_msg; int lbs_task; + int lbs_bdev; }; /* diff --git a/include/linux/security.h b/include/linux/security.h index 0a0a03b36a3b..8f83fdc6c65d 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -451,6 +451,11 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen); int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen); int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen); int security_locked_down(enum lockdown_reason what); +int security_bdev_alloc(struct block_device *bdev); +void security_bdev_free(struct block_device *bdev); +int security_bdev_setsecurity(struct block_device *bdev, + const char *name, const void *value, + size_t size); #else /* CONFIG_SECURITY */ static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data) @@ -1291,6 +1296,23 @@ static inline int security_locked_down(enum lockdown_reason what) { return 0; } + +static inline int security_bdev_alloc(struct block_device *bdev) +{ + return 0; +} + +static inline void security_bdev_free(struct block_device *bdev) +{ +} + +static inline int security_bdev_setsecurity(struct block_device *bdev, + const char *name, + const void *value, size_t size) +{ + return 0; +} + #endif /* CONFIG_SECURITY */ #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE) diff --git a/security/security.c b/security/security.c index 70a7ad357bc6..fff445eba400 100644 --- a/security/security.c +++ b/security/security.c @@ -28,6 +28,7 @@ #include #include #include +#include #define
Re: [PATCH v36 23/24] docs: x86/sgx: Document SGX micro architecture and kernel internals
Hi! > +CPUs starting from Icelake use Total Memory Encryption (TME) in the place of > +MEE. TME throws away the Merkle tree, which means losing integrity and > +anti-replay protection but also enables variable size memory pools for EPC. > +Using this attack for benefit would require an interposer on the system bus. It is not exactly clear what "this attack" means. (And it would be cool to explain against what SGX is protecting. I thought it was malicious RAM, but apparently not on Icelake+). > +Backing storage > +=== > + > +Backing storage is shared and not accounted. It is implemented as a private > +shmem file. Providing a backing storage in some form from user space is not > +possible - accounting would go to invalid state as reclaimed pages would get > +accounted to the processes of which behalf the kernel happened to be acting > on. "of which behalf" -- I can't parse that? > +Access control > +== > + > +`mmap()` permissions are capped by the enclave permissions. A direct > +consequence of this is that all the pages for an address range must be added > +before `mmap()` can be applied. Effectively an enclave page with minimum > +permission in the address range sets the permission cap for the mapping ~~ permissions? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: PGP signature
Re: [PATCH 10/15] iio: sx9310: Simplify error return handling
Quoting Daniel Campello (2020-07-28 14:23:29) > On Tue, Jul 28, 2020 at 1:40 PM Stephen Boyd wrote: > > > > Quoting Daniel Campello (2020-07-28 08:12:53) > > > @@ -368,13 +368,13 @@ static int sx9310_wait_for_sample(struct > > > sx9310_data *data) > > > static int sx9310_read_proximity(struct sx9310_data *data, > > > const struct iio_chan_spec *chan, int > > > *val) > > > { > > > - int ret = 0; > > > + int ret; > > > __be16 rawval; > > > > > > mutex_lock(>mutex); > > > > > > ret = sx9310_get_read_channel(data, chan->channel); > > > - if (ret < 0) > > > + if (ret) > > > goto out; > > > > > > if (data->client->irq) { > > > @@ -394,11 +394,11 @@ static int sx9310_read_proximity(struct sx9310_data > > > *data, > > > > > > mutex_lock(>mutex); > > > > > > - if (ret < 0) > > > + if (ret) > > > goto out_disable_irq; > > > > Why is this condition checked after grabbing the mutex? Shouldn't it be > > checked before grabbing the mutex? Or is that supposed to be a > > mutex_unlock()? > We acquire the lock before jumping to out_disable_irq which is before > a mutex_unlock() Does this function need to hold the mutex lock around get/put_read_channel? It drops the lock while waiting and then regrabs it which seems to imply that another reader could come in and try to get the channel again during the wait. So put another way, it may be simpler to shorten the lock area and then bail out of this function to a place where the lock isn't held already on the return path.
[PATCH] Staging : rtl8712 : Fixed a coding sytle issue
Removed braces for a 'if' condition as it contain only single line & there is no need for braces for such case according to coding style rules. Signed-off-by: Ankit Baluni --- drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c index c6f6ccd060bb..df6ae855f3c1 100644 --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c @@ -238,9 +238,8 @@ static char *translate_scan(struct _adapter *padapter, /* parsing HT_CAP_IE */ p = r8712_get_ie(>network.IEs[12], _HT_CAPABILITY_IE_, _ielen, pnetwork->network.IELength - 12); - if (p && ht_ielen > 0) { + if (p && ht_ielen > 0) ht_cap = true; - } /* Add the protocol name */ iwe.cmd = SIOCGIWNAME; if (r8712_is_cckratesonly_included(pnetwork->network.rates)) { -- 2.25.1
Re: [PATCH 04/15] iio: sx9310: Remove acpi and of table macros
Quoting Daniel Campello (2020-07-28 13:47:15) > On Tue, Jul 28, 2020 at 12:09 PM Andy Shevchenko > wrote: > > > > On Tue, Jul 28, 2020 at 6:16 PM Daniel Campello > > wrote: > > > > > > Avoids unused warnings due to acpi/of table macros. > > > > > > > At the same time I would check if mod_devicetable.h is included. > I did the following and no error showed up: > #ifndef LINUX_MOD_DEVICETABLE_H > #error Missing include > #endif That's fine, but it's usually better to avoid implicit include dependencies so that order of includes doesn't matter and so that if the headers change outside of this driver this driver doesn't have to be fixed to include something it needs like mod_devicetable.h
Re: [PATCH 06/15] iio: sx9310: Align memory
On Tue, Jul 28, 2020 at 12:11 PM Andy Shevchenko wrote: > > On Tue, Jul 28, 2020 at 6:15 PM Daniel Campello wrote: > > > > Use __aligned(8) to ensure that the timestamp is correctly aligned > > when we call push_to_buffers > > > > Signed-off-by: Daniel Campello > > --- > > > > drivers/iio/proximity/sx9310.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c > > index de52afd7c1..fb5c16f2aa6b1a 100644 > > --- a/drivers/iio/proximity/sx9310.c > > +++ b/drivers/iio/proximity/sx9310.c > > @@ -131,8 +131,8 @@ struct sx9310_data { > > */ > > bool prox_stat[SX9310_NUM_CHANNELS]; > > bool trigger_enabled; > > - __be16 buffer[SX9310_NUM_CHANNELS + > > - 4]; /* 64-bit data + 64-bit timestamp */ > > + /* 64-bit data + 64-bit timestamp buffer */ > > + __be16 buffer[SX9310_NUM_CHANNELS + 4] __aligned(8); > > If the data amount (channels) is always the same, please, use struct approach. > Otherwise put a comment explaining dynamic data. I'm not sure what you mean here. I have a comment above for the size of the array. > > -- > With Best Regards, > Andy Shevchenko
[PATCH] platform/chrome: cros_ec_typec: Send enum values to usb_role_switch_set_role()
usb_role_switch_set_role() has the second argument as enum for usb_role. Currently depending upon the data role i.e. UFP(0) or DFP(1) is sent. This eventually translates to USB_ROLE_NONE in case of UFP and USB_ROLE_DEVICE in case of DFP. Correct this by sending correct enum values as USB_ROLE_DEVICE in case of UFP and USB_ROLE_HOST in case of UFP. Fixes: 7e7def15fa4b ("platform/chrome: cros_ec_typec: Add USB mux control") Signed-off-by: Azhar Shaikh Cc: Prashant Malani --- drivers/platform/chrome/cros_ec_typec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c index 3eae01f4c9f7..eb4713b7ae14 100644 --- a/drivers/platform/chrome/cros_ec_typec.c +++ b/drivers/platform/chrome/cros_ec_typec.c @@ -590,7 +590,8 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num) dev_warn(typec->dev, "Configure muxes failed, err = %d\n", ret); return usb_role_switch_set_role(typec->ports[port_num]->role_sw, - !!(resp.role & PD_CTRL_RESP_ROLE_DATA)); + resp.role & PD_CTRL_RESP_ROLE_DATA + ? USB_ROLE_HOST : USB_ROLE_DEVICE); } static int cros_typec_get_cmd_version(struct cros_typec_data *typec) -- 2.17.1
Re: [PATCH 10/15] iio: sx9310: Simplify error return handling
On Tue, Jul 28, 2020 at 1:40 PM Stephen Boyd wrote: > > Quoting Daniel Campello (2020-07-28 08:12:53) > > @@ -368,13 +368,13 @@ static int sx9310_wait_for_sample(struct sx9310_data > > *data) > > static int sx9310_read_proximity(struct sx9310_data *data, > > const struct iio_chan_spec *chan, int *val) > > { > > - int ret = 0; > > + int ret; > > __be16 rawval; > > > > mutex_lock(>mutex); > > > > ret = sx9310_get_read_channel(data, chan->channel); > > - if (ret < 0) > > + if (ret) > > goto out; > > > > if (data->client->irq) { > > @@ -394,11 +394,11 @@ static int sx9310_read_proximity(struct sx9310_data > > *data, > > > > mutex_lock(>mutex); > > > > - if (ret < 0) > > + if (ret) > > goto out_disable_irq; > > Why is this condition checked after grabbing the mutex? Shouldn't it be > checked before grabbing the mutex? Or is that supposed to be a > mutex_unlock()? We acquire the lock before jumping to out_disable_irq which is before a mutex_unlock() > > > > > ret = sx9310_read_prox_data(data, chan, ); > > - if (ret < 0) > > + if (ret) > > goto out_disable_irq; > > > > *val = sign_extend32(be16_to_cpu(rawval),
Re: [GIT PULL][PATCH v9 0/7] Add support for ZSTD-compressed kernel and initramfs
On Tue, Jul 28, 2020 at 1:10 AM Nick Terrell wrote: > > From: Nick Terrell > > Please pull from > > g...@github.com:terrelln/linux.git tags/v9-zstd > > to get these changes. Alternatively the patchset is included. > > Hi all, > > This patch set adds support for a ZSTD-compressed kernel, ramdisk, and > initramfs in the kernel boot process. ZSTD-compressed ramdisk and initramfs > are supported on all architectures. The ZSTD-compressed kernel is only > hooked up to x86 in this patch set. > > Zstandard requires slightly more memory during the kernel decompression > on x86 (192 KB vs 64 KB), and the memory usage is independent of the > window size. > > Zstandard requires memory proprortional to the window size used during > compression for decompressing the ramdisk image, since streaming mode is > used. Newer versions of zstd (1.3.2+) list the window size of a file > with `zstd -lv '. The absolute maximum amount of memory required > is just over 8 MB, but it can be controlled at compression time. > > This patch set has been boot tested with buildroot and QEMU based off > of linux-5.6-rc6. > > On i386 and x86_64 I have tested the following configurations: > * zstd compressed kernel and a separate zstd compressed initramfs > * zstd compressed kernel and a built-in zstd compressed initramfs > * gzip compressed kernel and a separate gzip compressed initramfs > * gzip compressed kernel and a built-in gzip compressed initramfs > > On arm and aarch64 I tested the same configurations, except that the kernel is > always gzip compressed. > > Facebook has been using v1 of these patches on x86_64 devices for more than 6 > months. When we switched from a xz compressed initramfs to a zstd compressed > initramfs decompression time shrunk from 12 seconds to 3 seconds. When we > switched from a xz compressed kernel to a zstd compressed kernel we saved 2 > seconds of boot time. > > Facebook has been using v2 of these patches on aarch64 devices for a few > weeks. > When we switched from an lzma compressed initramfs to a zstd compressed > initramfs > decompression time shrunk from 27 seconds to 8 seconds. > > The zstd compressed kernel is smaller than the gzip compressed kernel but > larger > than the xz or lzma compressed kernels, and it decompresses faster than > everything except lz4. See the table below for the measurement of an x86_64 > kernel ordered by compressed size: > > algosize > xz 6,509,792 > lzma 6,856,576 > zstd 7,399,157 > gzip 8,522,527 > bzip 8,629,603 > lzo 9,808,035 > lz4 10,705,570 > none32,565,672 > > Alex Xu ran benchmarks in https://lkml.org/lkml/2020/7/1/722. > > v1 -> v2: > - Rebase > - usr/Makefile and init/Kconfig were changed so the patches were updated > - No functional changes except to rebase > - Split the patches up into smaller chunks > > v2 -> v3: > - Add *.zst to the .gitignore in patch 8 > - Style nits in patch 3 > - Rename the PREBOOT macro to ZSTD_PREBOOT and XXH_PREBOOT in patches > 1 through 3 > > v3 -> v4: > - Increase the ZSTD_IOBUF_SIZE from 4KB to 128KB to improve performance. > With this change I switch from malloc() to large_malloc() for the > buffers. > - Increase the maximum allowed window size from 8 MB to 128 MB, which is > the max that zstd in the kernel supports. > > v4 -> v5: > - Update commit message for patch 6 in response to comments > - Rebase onto next-20200408 > > v5 -> v6: > - Rebase onto v5.8-rc4 > > v6 -> v7: > - (1/7) Don't define or use 'ZSTD_PREBOOT' to hide exports > - (2/8) Drop 'lib: prepare xxhash for preboot environment' > - (2/7) Use '__DISABLE_EXPORTS' in unzstd to hide exports > - (3/7) Update zstd compression cmd to follow other compressors > - (3/7) Add zstd22 cmd > - (6/7) Use zstd -22 --ultra (zstd22) for x86 kernel compression > > v7 -> v8: > - (2/7) Don't define '__DISABLE_EXPORTS' > - (6/7) Define '__DISABLE_EXPORTS' in misc.c > > v8 -> v9: > - Rebase onto v5.8-rc7 > - (2/7) Fix nits about comment style & typos > - (3/7) Fix typo in init/Kconfig description > - (6/7) Explain BOOT_HEAP_SIZE increase and define __DISABLE_EXPORTS in > Makefile KBUILD_CFLAGS and remove definitions from kaslr.c and misc.c > Tested v9 on top of Linux v5.8-rc7 with these combinations: #1: GCC v10.2 and GNU/ld v2.35 #2: LLVM v11.0.0-rc1 (LLVM=1 and LLVM_IAS=1) - Sedat - > Best, > Nick Terrell > > Adam Borowski (1): > .gitignore: add ZSTD-compressed files > > Nick Terrell (6): > lib: prepare zstd for preboot environment > lib: add zstd support to decompress > init: add support for zstd compressed kernel > usr: add support for zstd compressed initramfs > x86: bump ZO_z_extra_bytes margin for zstd > x86: Add support for ZSTD compressed kernel > > .gitignore| 1 + > Documentation/x86/boot.rst| 6 +- > Makefile | 3 +- > arch/x86/Kconfig | 1 + > arch/x86/boot/compressed/Makefile | 6 +- >
Re: [PATCH RFC v6 0/6] Add Unisoc's drm kms module
Hi Kevin. Thanks for submitting this set of drivers. To better review the pataches can you please give some kind of high level overview. An ascii block diagram that identifies all the relevant blocks and how they relate would be great. This makes it easier to verify if the right modelling is used. Sam On Tue, Jul 28, 2020 at 06:07:53PM +0800, Kevin Tang wrote: > From: Kevin Tang > > ChangeList: > v1: > 1. only upstream modeset and atomic at first commit. > 2. remove some unused code; > 3. use alpha and blend_mode properties; > 3. add yaml support; > 4. remove auto-adaptive panel driver; > 5. bugfix > > v2: > 1. add sprd crtc and plane module for KMS, preparing for multi crtc > 2. remove gem drivers, use generic CMA handlers > 3. remove redundant "module_init", all the sub modules loading by KMS > > v3: > 1. multi crtc design have problem, so rollback to v1 > > v4: > 1. update to gcc-linaro-7.5.0 > 2. update to Linux 5.6-rc3 > 3. remove pm_runtime support > 4. add COMPILE_TEST, remove unused kconfig > 5. "drm_dev_put" on drm_unbind > 6. fix some naming convention issue > 7. remove semaphore lock for crtc flip > 8. remove static variables > > v5: > 1. optimize encoder and connector code implementation > 2. use "platform_get_irq" and "platform_get_resource" > 3. drop useless function return type, drop unless debug log > 4. custom properties should be separate, so drop it > 5. use DRM_XXX replase pr_xxx > 6. drop dsi hal callback ops > 7. drop unless callback ops checking > 8. add comments for sprd dpu structure > > v6: > 1. Access registers via readl/writel > 2. Checking for unsupported KMS properties (format, rotation, blend_mode, > etc) on plane_check ops > 3. Remove always true checks for dpu core ops > > Kevin Tang (6): > dt-bindings: display: add Unisoc's drm master bindings > drm/sprd: add Unisoc's drm kms master > dt-bindings: display: add Unisoc's dpu bindings > drm/sprd: add Unisoc's drm display controller driver > dt-bindings: display: add Unisoc's mipi dsi bindings > drm/sprd: add Unisoc's drm mipi dsi driver > > .../devicetree/bindings/display/sprd/dphy.yaml | 75 + > .../devicetree/bindings/display/sprd/dpu.yaml | 82 ++ > .../devicetree/bindings/display/sprd/drm.yaml | 36 + > .../devicetree/bindings/display/sprd/dsi.yaml | 98 ++ > drivers/gpu/drm/Kconfig|2 + > drivers/gpu/drm/Makefile |1 + > drivers/gpu/drm/sprd/Kconfig | 12 + > drivers/gpu/drm/sprd/Makefile | 13 + > drivers/gpu/drm/sprd/disp_lib.c| 57 + > drivers/gpu/drm/sprd/disp_lib.h| 16 + > drivers/gpu/drm/sprd/dphy/Makefile |7 + > drivers/gpu/drm/sprd/dphy/pll/Makefile |3 + > drivers/gpu/drm/sprd/dphy/pll/megacores_sharkle.c | 473 +++ > drivers/gpu/drm/sprd/dphy/sprd_dphy_api.c | 201 +++ > drivers/gpu/drm/sprd/dphy/sprd_dphy_api.h | 22 + > drivers/gpu/drm/sprd/dpu/Makefile |3 + > drivers/gpu/drm/sprd/dpu/dpu_r2p0.c| 503 +++ > drivers/gpu/drm/sprd/dsi/Makefile |8 + > drivers/gpu/drm/sprd/dsi/core/Makefile |4 + > drivers/gpu/drm/sprd/dsi/core/dsi_ctrl_r1p0.c | 964 + > drivers/gpu/drm/sprd/dsi/core/dsi_ctrl_r1p0.h | 1477 > > drivers/gpu/drm/sprd/dsi/core/dsi_ctrl_r1p0_ppi.c | 328 + > drivers/gpu/drm/sprd/dsi/core/dsi_ctrl_r1p0_ppi.h | 32 + > drivers/gpu/drm/sprd/dsi/sprd_dsi_api.c| 590 > drivers/gpu/drm/sprd/dsi/sprd_dsi_api.h| 26 + > drivers/gpu/drm/sprd/sprd_dphy.c | 209 +++ > drivers/gpu/drm/sprd/sprd_dphy.h | 50 + > drivers/gpu/drm/sprd/sprd_dpu.c| 668 + > drivers/gpu/drm/sprd/sprd_dpu.h| 190 +++ > drivers/gpu/drm/sprd/sprd_drm.c| 227 +++ > drivers/gpu/drm/sprd/sprd_drm.h| 18 + > drivers/gpu/drm/sprd/sprd_dsi.c| 571 > drivers/gpu/drm/sprd/sprd_dsi.h| 108 ++ > 33 files changed, 7074 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/sprd/dphy.yaml > create mode 100644 Documentation/devicetree/bindings/display/sprd/dpu.yaml > create mode 100644 Documentation/devicetree/bindings/display/sprd/drm.yaml > create mode 100644 Documentation/devicetree/bindings/display/sprd/dsi.yaml > create mode 100644 drivers/gpu/drm/sprd/Kconfig > create mode 100644 drivers/gpu/drm/sprd/Makefile > create mode 100644 drivers/gpu/drm/sprd/disp_lib.c > create mode 100644 drivers/gpu/drm/sprd/disp_lib.h > create mode 100644 drivers/gpu/drm/sprd/dphy/Makefile > create mode 100644 drivers/gpu/drm/sprd/dphy/pll/Makefile > create mode 100644
Re: [PATCH 4.19 00/86] 4.19.135-rc1 review
On Mon 2020-07-27 16:03:34, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.19.135 release. > There are 86 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Wed, 29 Jul 2020 13:48:51 +. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.135-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-4.19.y > and the diffstat can be found below. It passes tests on CIP test farm: https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: PGP signature
Re: [PATCH v3 3/4] regulator: core: Add basic enable/disable support for sync_state() callbacks
On Tue, Jul 21, 2020 at 1:18 PM Mark Brown wrote: > Hi Mark, It *might* be easier if you jump to the bottom and read the reasoning for the current design. The stuff in between is just me trying to clarify some misunderstandings. > On Mon, Jul 20, 2020 at 08:22:15PM -0700, Saravana Kannan wrote: > > On Mon, Jul 20, 2020 at 7:28 AM Mark Brown wrote: > > > On Wed, Jul 15, 2020 at 09:20:52PM -0700, Saravana Kannan wrote: > > > > > There are Android devices that exhibit the issue in the example where > > > > regulator-X is an LDO, device-A is a camera device and device-B and > > > > device-C are UFS and USB. To avoid this, they have their own downstream > > > > changes to the regulator framework. > > > > Can you provide any references to these bodges? > > > This is the best I could dig up. The site is slow as molasses. I don't > > want to focus or critique any specific vendor's downstream code > > though. Providing these links just to prove that this is a real issue. > > The issue is not that I can't see that there might be a problem. The > issue I have is that you seem to be starting from the point of a > specific solution that happens to work for your systems and then working > back from that rather than reasoning forwards from the problem to come > to a solution with the result that I can't explain the design you're > proposing. I'm sorry it comes across that way. That's certainly not my intention. Hopefully, we can walk through this together and I'll make more sense. > Even with this version I have difficulty telling from the > changelog that the change doesn't restrict things based on the consumers > of a given regulator but rather based on the PMIC and I don't see why > this is being done. Happy to copy-paste any commit text suggestions that make this clearer. > > Search for "proxy" here. You'll notice how that will also need changes > > to regulator header files, etc. The 4.9 kernel is the latest publicly > > available version AFAIK. > > https://source.codeaurora.org/quic/la/kernel/msm-4.9/plain/drivers/regulator/core.c?h=msm-4.9 > > Oh, the Qualcomm stuff - which looks a lot like what you've got here. Eh... I would disagree. The similarity stops at "both solutions use a regulator consumer to set the limits". The implementation you see there is based on QC specific DT entries, is based on late_initcall to "ensure" all consumers have probed, doesn't work for modules, etc. > > > As I indicated on my previous review this doesn't seem OK, given that a > > > huge proportion of the regulators on most systems are part of a single > > > PMIC this means that devices won't be able to fully control regulators > > > for the majority of the boot process, possibly quite a long time after > > > userspace has started in systems where not all devices have drivers. > > ... > > > I think the default behavior should be for functional correctness > > (example in the commit text) and then go for optimization (being able > > to power off regulators before 30s into boot). Even with the timeout > > set, this series makes it much easier for driver developers to ensure > > functional correctness instead of playing initcall chicken between the > > supplier and the N consumers. > > I don't see how any of this stuff about initcall chicken or whatever is > relevant here. You're replying to my concern that your change isn't > just waiting for all the consumers of a given regulator, it's waiting > for every other consumer of any other regulator or random other feature > that happens to be supplied by the same PMIC but only that PMIC. That > is not init ordering, it just seems like an arbitrary delay even once > all the consumers are ready and I can't see any particular logic behind > it. It's not just waiting for all the users of the individual resource > to be active but it's also not waiting for the system as a whole, > instead it's waiting for some effectively random grouping of devices > spread over the whole system to appear. I can't articulate a reason why > we'd do that, it seems to be combining the worst aspects of the two > approaches. > > Please engage with this issue, it is crucial but you keep on sending the > same thing without explaining why so I'm left guessing and I really > can't come up with anything. Ok, at the end of the email, I'll start from a high level and then dig into more details at each step. Hopefully that'll clarify the reasoning. [...] > > > I don't understand the motivation for doing things this way. Like I > > > said last time it really feels like this turns the whole mechanism into > > > a very complicated way of implementing a new initcall. > > > Treating this as a "LATER_initcall()" has several issues. > > 1. initcall levels set a max limit on the depth of device dependency. > > Since DT devices are added at arch initcall level, that gives us about > > 5 levels if you ignore the _sync ones. Adding more isn't going to > > scale or solve the problem because in reality, the
Re: [PATCH RFC v6 4/6] drm/sprd: add Unisoc's drm display controller driver
Hi Kevin. Some feedback in the following. I lost track of thing for the atomic modesettting stuff and I hope other will review that. Sam On Tue, Jul 28, 2020 at 06:07:57PM +0800, Kevin Tang wrote: > From: Kevin Tang > > Adds DPU(Display Processor Unit) support for the Unisoc's display subsystem. > It's support multi planes, scaler, rotation, PQ(Picture Quality) and more. > > RFC v6: > - Access registers via readl/writel > - Checking for unsupported KMS properties (format, rotation, blend_mode, > etc) on plane_check ops > - Remove always true checks for dpu core ops > > Cc: Orson Zhai > Cc: Chunyan Zhang > Signed-off-by: Kevin Tang > --- > drivers/gpu/drm/sprd/Makefile | 5 +- > drivers/gpu/drm/sprd/dpu/Makefile | 3 + > drivers/gpu/drm/sprd/dpu/dpu_r2p0.c | 503 > drivers/gpu/drm/sprd/sprd_dpu.c | 646 > > drivers/gpu/drm/sprd/sprd_dpu.h | 187 +++ > drivers/gpu/drm/sprd/sprd_drm.c | 1 + > drivers/gpu/drm/sprd/sprd_drm.h | 2 + > 7 files changed, 1346 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/sprd/dpu/Makefile > create mode 100644 drivers/gpu/drm/sprd/dpu/dpu_r2p0.c > create mode 100644 drivers/gpu/drm/sprd/sprd_dpu.c > create mode 100644 drivers/gpu/drm/sprd/sprd_dpu.h > > diff --git a/drivers/gpu/drm/sprd/Makefile b/drivers/gpu/drm/sprd/Makefile > index 86d95d9..88ab32a 100644 > --- a/drivers/gpu/drm/sprd/Makefile > +++ b/drivers/gpu/drm/sprd/Makefile > @@ -2,4 +2,7 @@ > > subdir-ccflags-y += -I$(srctree)/$(src) Not needed - drop. > > -obj-y := sprd_drm.o > +obj-y := sprd_drm.o \ > + sprd_dpu.o > + > +obj-y += dpu/ Until there are several DPU's there is no need for a separate directory. Make it simpler by keeping it all in drm/sprd/* > diff --git a/drivers/gpu/drm/sprd/dpu/Makefile > b/drivers/gpu/drm/sprd/dpu/Makefile > new file mode 100644 > index 000..40278b6 > --- /dev/null > +++ b/drivers/gpu/drm/sprd/dpu/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +obj-y += dpu_r2p0.o > diff --git a/drivers/gpu/drm/sprd/dpu/dpu_r2p0.c > b/drivers/gpu/drm/sprd/dpu/dpu_r2p0.c > new file mode 100644 > index 000..4b9521d > --- /dev/null > +++ b/drivers/gpu/drm/sprd/dpu/dpu_r2p0.c > @@ -0,0 +1,503 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2020 Unisoc Inc. > + */ > + > +#include > +#include > +#include > +#include > + > +#include "sprd_dpu.h" > + > +/* DPU registers size, 4 Bytes(32 Bits) */ > +#define DPU_REG_SIZE 0x04 > + > +/* Layer registers offset */ > +#define DPU_LAY_REG_OFFSET 0x0C > + > +#define DPU_LAY_REG(reg, index) \ > + (reg + index * DPU_LAY_REG_OFFSET * DPU_REG_SIZE) Use a static inline to get better typecheck. > +#define DPU_REG_RD(reg) readl_relaxed(reg) > + > +#define DPU_REG_WR(reg, mask) writel_relaxed(mask, reg) > + > +#define DPU_REG_SET(reg, mask) \ > + writel_relaxed(readl_relaxed(reg) | mask, reg) > + > +#define DPU_REG_CLR(reg, mask) \ > + writel_relaxed(readl_relaxed(reg) & ~mask, reg) I am no fan of macros used like this. Maybe use static inlines and add struct dpu_context * as first argument. Then adding base can be hidden away. I had a hard time convincing myself that _relaxed was the right variants. If I get it correct we may see writes re-ordered with the _relaxed variants wich would be no good when dealign with interrupts. But I may miss somethign here. > + > +/* Global control registers */ > +#define REG_DPU_CTRL 0x04 > +#define REG_DPU_CFG0 0x08 > +#define REG_DPU_CFG1 0x0C > +#define REG_DPU_CFG2 0x10 > +#define REG_PANEL_SIZE 0x20 > +#define REG_BLEND_SIZE 0x24 > +#define REG_BG_COLOR 0x2C > + > +/* Layer0 control registers */ > +#define REG_LAY_BASE_ADDR0 0x30 > +#define REG_LAY_BASE_ADDR1 0x34 > +#define REG_LAY_BASE_ADDR2 0x38 > +#define REG_LAY_CTRL 0x40 > +#define REG_LAY_SIZE 0x44 > +#define REG_LAY_PITCH0x48 > +#define REG_LAY_POS 0x4C > +#define REG_LAY_ALPHA0x50 > +#define REG_LAY_PALLETE 0x58 > +#define REG_LAY_CROP_START 0x5C > + > +/* Interrupt control registers */ > +#define REG_DPU_INT_EN 0x1E0 > +#define REG_DPU_INT_CLR 0x1E4 > +#define REG_DPU_INT_STS 0x1E8 > + > +/* DPI control registers */ > +#define REG_DPI_CTRL 0x1F0 > +#define REG_DPI_H_TIMING 0x1F4 > +#define REG_DPI_V_TIMING 0x1F8 > + > +/* MMU control registers */ > +#define REG_MMU_EN 0x800 > +#define REG_MMU_VPN_RANGE0x80C > +#define REG_MMU_VAOR_ADDR_RD 0x818 > +#define REG_MMU_VAOR_ADDR_WR 0x81C > +#define REG_MMU_INV_ADDR_RD 0x820 > +#define REG_MMU_INV_ADDR_WR 0x824 > +#define REG_MMU_PPN1 0x83C > +#define REG_MMU_RANGE1 0x840 > +#define REG_MMU_PPN2 0x844 > +#define REG_MMU_RANGE2
Re: [PATCH] staging/speakup: Update TODO list
Greg KH, le mar. 28 juil. 2020 10:18:42 +0200, a ecrit: > On Sun, Jul 26, 2020 at 06:54:52PM +0200, Samuel Thibault wrote: > > Thanks to Okash's latest work, the TODO list is essentially empty, so > > the way out from staging now seems open. > > > > The status of the remaining issue mentioned in TODO is not clear, we > > asked the speakup user mailing list for reproducer cases, but didn't get > > a really specific scenario. One serious bug was fixed by 9d32c0cde4e2 > > ("staging/speakup: fix get_word non-space look-ahead"), which does not > > seem to really be related to the bug mentioned in TODO. Possibly the bug > > mentioned in TODO has been fixed long ago and people have been thinking > > that it was not because they can not distinguish the symptoms mentioned > > in TODO from the symptoms of the bug fixed by 9d32c0cde4e2. > > I think it's time we move speakup out of staging. Care to submit a > patch series that does this? Yes! Done so. Samuel
[PATCH] staging/speakup: Move out of staging
The nasty TODO items are done. Signed-off-by: Samuel Thibault --- .../ABI/stable}/sysfs-driver-speakup | 0 .../admin-guide}/spkguide.txt | 0 MAINTAINERS | 19 +-- drivers/accessibility/Kconfig | 2 ++ drivers/accessibility/Makefile| 1 + .../speakup/DefaultKeyAssignments | 0 .../speakup/Kconfig | 0 .../speakup/Makefile | 0 .../{staging => accessibility}/speakup/TODO | 0 .../speakup/buffers.c | 0 .../speakup/devsynth.c| 0 .../speakup/fakekey.c | 0 .../{staging => accessibility}/speakup/i18n.c | 0 .../{staging => accessibility}/speakup/i18n.h | 0 .../speakup/keyhelp.c | 0 .../speakup/kobjects.c| 0 .../{staging => accessibility}/speakup/main.c | 0 .../speakup/selection.c | 0 .../speakup/serialio.c| 0 .../speakup/serialio.h| 0 .../speakup/speakup.h | 0 .../speakup/speakup_acnt.h| 0 .../speakup/speakup_acntpc.c | 0 .../speakup/speakup_acntsa.c | 0 .../speakup/speakup_apollo.c | 0 .../speakup/speakup_audptr.c | 0 .../speakup/speakup_bns.c | 0 .../speakup/speakup_decext.c | 0 .../speakup/speakup_decpc.c | 0 .../speakup/speakup_dectlk.c | 0 .../speakup/speakup_dtlk.c| 0 .../speakup/speakup_dtlk.h| 0 .../speakup/speakup_dummy.c | 0 .../speakup/speakup_keypc.c | 0 .../speakup/speakup_ltlk.c| 0 .../speakup/speakup_soft.c| 0 .../speakup/speakup_spkout.c | 0 .../speakup/speakup_txprt.c | 0 .../speakup/speakupmap.h | 0 .../speakup/speakupmap.map| 0 .../speakup/spk_priv.h| 0 .../speakup/spk_priv_keyinfo.h| 0 .../speakup/spk_ttyio.c | 0 .../speakup/spk_types.h | 0 .../speakup/synth.c | 0 .../speakup/thread.c | 0 .../speakup/varhandlers.c | 0 drivers/staging/Kconfig | 2 -- drivers/staging/Makefile | 1 - 49 files changed, 12 insertions(+), 13 deletions(-) rename {drivers/staging/speakup => Documentation/ABI/stable}/sysfs-driver-speakup (100%) rename {drivers/staging/speakup => Documentation/admin-guide}/spkguide.txt (100%) rename drivers/{staging => accessibility}/speakup/DefaultKeyAssignments (100%) rename drivers/{staging => accessibility}/speakup/Kconfig (100%) rename drivers/{staging => accessibility}/speakup/Makefile (100%) rename drivers/{staging => accessibility}/speakup/TODO (100%) rename drivers/{staging => accessibility}/speakup/buffers.c (100%) rename drivers/{staging => accessibility}/speakup/devsynth.c (100%) rename drivers/{staging => accessibility}/speakup/fakekey.c (100%) rename drivers/{staging => accessibility}/speakup/i18n.c (100%) rename drivers/{staging => accessibility}/speakup/i18n.h (100%) rename drivers/{staging => accessibility}/speakup/keyhelp.c (100%) rename drivers/{staging => accessibility}/speakup/kobjects.c (100%) rename drivers/{staging => accessibility}/speakup/main.c (100%) rename drivers/{staging => accessibility}/speakup/selection.c (100%) rename drivers/{staging => accessibility}/speakup/serialio.c (100%) rename drivers/{staging => accessibility}/speakup/serialio.h (100%) rename drivers/{staging => accessibility}/speakup/speakup.h (100%) rename drivers/{staging => accessibility}/speakup/speakup_acnt.h (100%) rename drivers/{staging => accessibility}/speakup/speakup_acntpc.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_acntsa.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_apollo.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_audptr.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_bns.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_decext.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_decpc.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_dectlk.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_dtlk.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_dtlk.h (100%) rename drivers/{staging => accessibility}/speakup/speakup_dummy.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_keypc.c (100%) rename drivers/{staging => accessibility}/speakup/speakup_ltlk.c (100%)
Re: [PATCH 08/10] media: uapi: h264: Clean slice invariants syntax elements
Le mardi 28 juillet 2020 à 14:44 +0200, Maxime Ripard a écrit : > Hi, > > On Mon, Jul 27, 2020 at 11:39:12AM -0300, Ezequiel Garcia wrote: > > On Sat, 2020-07-25 at 23:34 +0900, Alexandre Courbot wrote: > > > On Thu, Jul 16, 2020 at 5:23 AM Ezequiel Garcia > > > wrote: > > > > The H.264 specification requires in its "Slice header semantics" > > > > section that the following values shall be the same in all slice > > > > headers: > > > > > > > > pic_parameter_set_id > > > > frame_num > > > > field_pic_flag > > > > bottom_field_flag > > > > idr_pic_id > > > > pic_order_cnt_lsb > > > > delta_pic_order_cnt_bottom > > > > delta_pic_order_cnt[ 0 ] > > > > delta_pic_order_cnt[ 1 ] > > > > sp_for_switch_flag > > > > slice_group_change_cycle > > > > > > > > and can therefore be moved to the per-frame decode parameters control. > > > > > > I am really not a H.264 expert, so this question may not be relevant, > > > > All questions are welcome. I'm more than happy to discuss this patchset. > > > > > but are these values specified for every slice header in the > > > bitstream, or are they specified only once per frame? > > > > > > I am asking this because it would certainly make user-space code > > > simpler if we could remain as close to the bitstream as possible. If > > > these values are specified once per slice, then factorizing them would > > > leave user-space with the burden of deciding what to do if they change > > > across slices. > > > > > > Note that this is a double-edged sword, because it is not necessarily > > > better to leave the firmware in charge of deciding what to do in such > > > a case. :) So hopefully these are only specified once per frame in the > > > bitstream, in which case your proposal makes complete sense. > > > > Frame-based hardwares accelerators such as Hantro and Rockchip VDEC > > are doing the slice header parsing themselves. Therefore, the > > driver is not really parsing these fields on each slice header. > > > > Currently, we are already using only the first slice in a frame, > > as you can see from: > > > > if (slices[0].flags & V4L2_H264_SLICE_FLAG_FIELD_PIC) > > reg |= G1_REG_DEC_CTRL0_PIC_FIELDMODE_E; > > > > Even if these fields are transported in the slice header, > > I think it makes sense for us to split them into the decode params > > (per-frame) control. > > I don't really get it though. The initial plan that was asked was to > mimic as much as possible the bitstream and that's what we did. > > But that requirement seems to have changed now? Indeed, that has changed and has been discussed multiple times already. But in general what you need to realize is that the bitstream is made of redundancy in order to be robust over data lost. Most importantly, it is by design that you can loose a slice, but still the decode the other slices. Carrying this redundancy over the kernel API though didn't make much sense. It only made the amount of data we have to pass (and copy) for each frames bigger. The goal of this change is to reduce the amount of data you actually need to pass, both for frame and slice decoding. For frame decoding, all the invariants from the slice header (notice the spec name here) have been moved from slice_params (not a spec term) into the decode_params (also not a spec term). This way, the slice_params are no longer needed to be passed at all to driver. As for the slice decoding, assuming you care making use of the VB2 control caching, you can pass the decode_params on the first slice only, and only pass the slice_params, which is now slimmer, for the following slices. Remember that Paul initially believed that V4L2 stateless decoder was a stateless API, while in fact the HW is stateless, but V4L2 API are statefull by nature. p.s. it was also request to use raster scan scaling matrix, which isn't the order found in the bitstream. That was also discussed, and the reason is that other existing interfaces use raster order, so not using that is error prone for both kernel and userspace coders. Cedrus has been in raster scan order since the start, ignoring that rule already. > > Even if it did change though, if this is defined as a slice parameter in > the spec, why would we want to move it to some other control entirely if > we are to keep the slice parameters control? You are confusing a term we invented, slice_params, with a specified term slice header. The decode_params are as document information per frame/picture, while slice_params is information needed per slice, nothing more. > > Especially if the reason is to make the life easier on a couple of > drivers, that's really not the point of a userspace API, and we can > always add an helper if it really shows up as a pattern. We have made userspace implementation of this, GStreamer for now, I can only disagree with you. This does not make userspace much more complex and on top of that, it allow reducing some overhead, as prior to that
[tip:locking/header] BUILD SUCCESS e885d5d94793ef342e49d55672baabbc16e32bb1
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking/header branch HEAD: e885d5d94793ef342e49d55672baabbc16e32bb1 lockdep: Move list.h inclusion into lockdep.h elapsed time: 725m configs tested: 69 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig arm prima2_defconfig arm footbridge_defconfig mipsnlm_xlr_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68kdefconfig m68k allmodconfig m68k allyesconfig nios2 defconfig nds32 allnoconfig c6x allyesconfig arc allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig powerpc defconfig i386 randconfig-a003-20200728 i386 randconfig-a004-20200728 i386 randconfig-a005-20200728 i386 randconfig-a002-20200728 i386 randconfig-a006-20200728 i386 randconfig-a001-20200728 x86_64 randconfig-a014-20200728 x86_64 randconfig-a012-20200728 x86_64 randconfig-a015-20200728 x86_64 randconfig-a016-20200728 x86_64 randconfig-a013-20200728 x86_64 randconfig-a011-20200728 i386 randconfig-a016-20200728 i386 randconfig-a012-20200728 i386 randconfig-a013-20200728 i386 randconfig-a014-20200728 i386 randconfig-a011-20200728 i386 randconfig-a015-20200728 riscv allnoconfig riscv defconfig riscvallyesconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH v6 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver
On Wed, Jul 15, 2020 at 08:33:17AM -0700, Ben Levinsky wrote: > R5 is included in Xilinx Zynq UltraScale MPSoC so by adding this > remotproc driver, we can boot the R5 sub-system in different > configurations. > > Acked-by: Stefano Stabellini > Acked-by: Ben Levinsky > Reviewed-by: Radhey Shyam Pandey > Signed-off-by: Ben Levinsky > Signed-off-by: Wendy Liang > Signed-off-by: Michal Simek > Signed-off-by: Ed Mooring > Signed-off-by: Jason Wu > Tested-by: Ben Levinsky > --- > v2: > - remove domain struct as per review from Mathieu > v3: > - add xilinx-related platform mgmt fn's instead of wrapping around > function pointer in xilinx eemi ops struct > v4: > - add default values for enums > - fix formatting as per checkpatch.pl --strict. Note that 1 warning and 1 > check > are still raised as each is due to fixing the warning results in that > particular line going over 80 characters. > v5: > - parse_fw change from use of rproc_of_resm_mem_entry_init to > rproc_mem_entry_init and use of alloc/release > - var's of type zynqmp_r5_pdata all have same local variable name > - use dev_dbg instead of dev_info > v6: > - adding memory carveouts is handled much more similarly. All mem carveouts > are > now described in reserved memory as needed. That is, TCM nodes are not > coupled to remoteproc anymore. This is reflected in the remoteproc R5 driver > and the device tree binding. > - remove mailbox from device tree binding as it is not necessary for elf > loading > - use lockstep-mode property for configuring RPU > --- > drivers/remoteproc/Kconfig| 10 + > drivers/remoteproc/Makefile | 1 + > drivers/remoteproc/zynqmp_r5_remoteproc.c | 911 ++ > 3 files changed, 922 insertions(+) > create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > index c4d1731295eb..342a7e668636 100644 > --- a/drivers/remoteproc/Kconfig > +++ b/drivers/remoteproc/Kconfig > @@ -249,6 +249,16 @@ config STM32_RPROC > > This can be either built-in or a loadable module. > > +config ZYNQMP_R5_REMOTEPROC > + tristate "ZynqMP_R5 remoteproc support" > + depends on ARM64 && PM && ARCH_ZYNQMP > + select RPMSG_VIRTIO > + select MAILBOX > + select ZYNQMP_IPI_MBOX > + help > + Say y here to support ZynqMP R5 remote processors via the remote > + processor framework. > + > endif # REMOTEPROC > > endmenu > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile > index e8b886e511f0..04d1c95d06d7 100644 > --- a/drivers/remoteproc/Makefile > +++ b/drivers/remoteproc/Makefile > @@ -28,5 +28,6 @@ obj-$(CONFIG_QCOM_WCNSS_PIL)+= > qcom_wcnss_pil.o > qcom_wcnss_pil-y += qcom_wcnss.o > qcom_wcnss_pil-y += qcom_wcnss_iris.o > obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o > +obj-$(CONFIG_ZYNQMP_R5_REMOTEPROC) += zynqmp_r5_remoteproc.o > obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o > obj-$(CONFIG_STM32_RPROC)+= stm32_rproc.o > diff --git a/drivers/remoteproc/zynqmp_r5_remoteproc.c > b/drivers/remoteproc/zynqmp_r5_remoteproc.c > new file mode 100644 > index ..b600759e257e > --- /dev/null > +++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c > @@ -0,0 +1,911 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Zynq R5 Remote Processor driver > + * > + * Copyright (C) 2019, 2020 Xilinx Inc. Ben Levinsky > > + * Copyright (C) 2015 - 2018 Xilinx Inc. > + * Copyright (C) 2015 Jason Wu > + * > + * Based on origin OMAP and Zynq Remote Processor driver > + * > + * Copyright (C) 2012 Michal Simek > + * Copyright (C) 2012 PetaLogix > + * Copyright (C) 2011 Texas Instruments, Inc. > + * Copyright (C) 2011 Google, Inc. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "remoteproc_internal.h" > + > +#define MAX_RPROCS 2 /* Support up to 2 RPU */ > +#define MAX_MEM_PNODES 4 /* Max power nodes for one RPU memory > instance */ > + > +#define DEFAULT_FIRMWARE_NAME"rproc-rpu-fw" > + > +/* PM proc states */ > +#define PM_PROC_STATE_ACTIVE 1U > + > +/* IPI buffer MAX length */ > +#define IPI_BUF_LEN_MAX 32U > +/* RX mailbox client buffer max length */ > +#define RX_MBOX_CLIENT_BUF_MAX (IPI_BUF_LEN_MAX + \ > + sizeof(struct zynqmp_ipi_message)) > + > +#define ZYNQMP_R5_NUM_TCM_BANKS 4 > + > +/* lookup table mapping power-node-ID of TCM bank to absolute base address */ > +static unsigned long tcm_addr_to_pnode[ZYNQMP_R5_NUM_TCM_BANKS][2] = { > + {0xFFE0,0xF }, > + {0xFFE2,0x10}, > + {0xFFE9,
Re: rtsx_pci not restoring ASPM state after suspend/resume
On Mon, 2020-07-27 at 16:47 -0500, Bjorn Helgaas wrote: > > I don't see anything in rtsx that enables L0s. Can you collect the > dmesg log when booting with "pci=earlydump"? That will show whether > the BIOS left it this way. The PCI core isn't supposed to do this, > so > if it did, we need to fix that. > dmesg log attached to the bugzilla as: https://bugzilla.kernel.org/attachment.cgi?id=290655 (to keep everything in one place). Thanks, -James
Re: [PATCH v4 3/6] mm/notifier: add migration invalidation type
On 7/28/20 12:15 PM, Jason Gunthorpe wrote: On Thu, Jul 23, 2020 at 03:30:01PM -0700, Ralph Campbell wrote: static inline int mm_has_notifiers(struct mm_struct *mm) @@ -513,6 +519,7 @@ static inline void mmu_notifier_range_init(struct mmu_notifier_range *range, range->start = start; range->end = end; range->flags = flags; + range->migrate_pgmap_owner = NULL; } Since this function is commonly called and nobody should read migrate_pgmap_owner unless MMU_NOTIFY_MIGRATE is set as the event, this assignment can be dropped. Jason I agree. Acked-by: Ralph Campbell
BTF_KIND_FWD enums
Hi. Re: https://github.com/torvalds/linux/commit/9d5f9f701b1891466fb3dbb1806ad97716f95cc3 Both GCC and LLVM support forward-declared (a.k.a. incomplete) enums as a language extension - https://gcc.gnu.org/onlinedocs/gcc/Incomplete-Enums.html. (C++11 has a different notion of incomplete enum type - opaque enum declaration - storage size is known but enumerators are not) Forward-declared enums feature in various places in kernel code and allow the usual things to be done (passing around pointers to such). I'm curious as to if and how they are they are handled by BTF and whether a further change to btf_type is needed: 1. Use BTF_KIND_FWD, with another spare bit to allow up to 4 kinds of forward-declaration; or 2. use BTF_KIND_ENUM, kind_flag 0 and vlen 0 (as empty enums are currently illegal C); or 3. use BTF_KIND_ENUM, kind_flag 1 and vlen 0. If I had a working pahole -J, I'd test this myself. :-) $ cat /tmp/en.c enum H; enum H * fun(enum H * x) { return x; } $ clang -Wall -Wextra -ggdb -c /tmp/en.c $ build/pahole -J /tmp/en.o Segmentation fault $ build/pahole -J /dev/null btf_elf__new: cannot get elf header. ctf__new: cannot get elf header. Segmentation fault My interest here is that I helped add support for incomplete enums to libabigail which we're using to monitor kernel ABIs. Regards, Giuliano. (resend due to email address typo)
Re: [PATCH v4 0/5] MIPS: Loongson64: Process ISA Node in DeviceTree
On Tue, Jul 28, 2020 at 11:36:54PM +0800, Jiaxun Yang wrote: > Hi, > > This series convert reservation of Loongson64 Logic PIO into DeviceTree based > method. > > It can be used to replace Huacai's > "MIPS: Loongson64: Reserve legacy MMIO space according to bridge type". > > Thanks. > > v2: > - Address Rob and Huacai's review comments. > v3: > - Address Rob, Thomas's review comments. > v4: > - Fix typo & grammar issue according to Xuerui's suggestion. > > Jiaxun Yang (5): > of_address: Add bus type match for pci ranges parser > MIPS: Loongson64: Process ISA Node in DeviceTree > MIPS: Loongson64: Enlarge IO_SPACE_LIMIT > MIPS: Loongson64: DTS: Fix ISA and PCI I/O ranges for RS780E PCH > MIPS: Loongson64: Add ISA node for LS7A PCH > > arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 ++ > arch/mips/boot/dts/loongson/rs780e-pch.dtsi | 4 +- > arch/mips/include/asm/io.h| 2 - > arch/mips/include/asm/mach-generic/spaces.h | 4 + > .../mips/include/asm/mach-loongson64/spaces.h | 3 +- > arch/mips/loongson64/init.c | 87 +-- > drivers/of/address.c | 29 --- > include/linux/of_address.h| 4 + > 8 files changed, 97 insertions(+), 43 deletions(-) series applied to mips-next. Thomas. -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ]
Re: [PATCH v4 2/2] MIPS: ingenic: Enable JZ4780_NEMC manually
On Tue, Jul 28, 2020 at 02:53:08PM +0200, Krzysztof Kozlowski wrote: > On Tue, Jul 28, 2020 at 02:51:23PM +0200, Paul Cercueil wrote: > > > > > > Le mar. 28 juil. 2020 à 14:00, Krzysztof Kozlowski a écrit > > : > > > On Tue, Jul 28, 2020 at 01:37:02PM +0200, Thomas Bogendoerfer wrote: > > > > On Tue, Jul 28, 2020 at 01:19:35PM +0200, Krzysztof Kozlowski wrote: > > > > > On Tue, Jul 28, 2020 at 01:12:11PM +0200, Paul Cercueil wrote: > > > > > > Hi Krzysztof, > > > > > > > > > > > > Le mar. 28 juil. 2020 à 12:45, Krzysztof Kozlowski > > > > a écrit > > > > > > : > > > > > > > The CONFIG_JZ4780_NEMC was previously a default on MIPS but > > > > now it has > > > > > > > to be enabled manually. > > > > > > > > > > > > > > Signed-off-by: Krzysztof Kozlowski > > > > > > > > > > > > I think you should swap the two so that there are no problems > > > > when > > > > > > bisecting. > > > > > > > > > > Good point. I was thinking that it will go via some of MIPS trees > > > > and > > > > > the patch #1 will just wait a cycle. However with acks, I can > > > > take it > > > > > through drivers/memory tree. > > > > > > > > I've acked the patch. > > > > > > > > Thomas. > > > > > > Thanks but now I noticed that one of changed configs > > > (arch/mips/configs/rs90_defconfig) is only in MIPS tree. > > > > > > I think it is easier then to take the patch #2 (configs) via MIPS and > > > wait with #1 for the next cycle or also take it via MIPS if it applies > > > cleanly. > > > > Why not take them both in the MIPS tree then? Would that conflict with > > changes in your tree? > > Exactly (last part of my sentence). There should be no conflicts. I've applied both patches to mips-next. Thomas. -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ]
Re: [PATCH RFC v6 3/6] dt-bindings: display: add Unisoc's dpu bindings
Hi Kevin. On Tue, Jul 28, 2020 at 06:07:56PM +0800, Kevin Tang wrote: > From: Kevin Tang > > DPU (Display Processor Unit) is the Display Controller for the Unisoc SoCs > which transfers the image data from a video memory buffer to an internal > LCD interface. > > Cc: Orson Zhai > Cc: Chunyan Zhang > Signed-off-by: Kevin Tang > --- > .../devicetree/bindings/display/sprd/dpu.yaml | 82 > ++ Could we name it after the compatible "sharkl3-dpu.yaml"? > 1 file changed, 82 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/sprd/dpu.yaml > > diff --git a/Documentation/devicetree/bindings/display/sprd/dpu.yaml > b/Documentation/devicetree/bindings/display/sprd/dpu.yaml > new file mode 100644 > index 000..54ba9b6f > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/sprd/dpu.yaml > @@ -0,0 +1,82 @@ > +# SPDX-License-Identifier: GPL-2.0 Can this be: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/sprd/dpu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Unisoc SoC Display Processor Unit (DPU) > + > +maintainers: > + - Mark Rutland > + > +description: | > + DPU (Display Processor Unit) is the Display Controller for the Unisoc SoCs > + which transfers the image data from a video memory buffer to an internal > + LCD interface. > + > +properties: > + compatible: > +const: sprd,sharkl3-dpu > + > + reg: > +maxItems: 1 > +description: > + Physical base address and length of the DPU registers set > + > + interrupts: > +maxItems: 1 > +description: > + The interrupt signal from DPU > + > + clocks: > +minItems: 2 > + > + clock-names: > +items: > + - const: clk_src_128m > + - const: clk_src_384m > + > + power-domains: > +description: A phandle to DPU power domain node. maxItems: 1 > + > + iommus: > +description: A phandle to DPU iommu node. maxItems: 1 > + > + port: > +type: object > +description: > + A port node with endpoint definitions as defined in > + Documentation/devicetree/bindings/media/video-interfaces.txt. > + That port should be the output endpoint, usually output to > + the associated DSI. > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - port > + > +additionalProperties: false > + > +examples: > + - | > +#include > +#include > +dpu: dpu@0x6300 { > + compatible = "sprd,sharkl3-dpu"; > + reg = <0x0 0x6300 0x0 0x1000>; > + interrupts = ; > + clock-names = "clk_src_128m", > + "clk_src_384m"; > + > + clocks = < CLK_TWPLL_128M>, > +< CLK_TWPLL_384M>; > + > + dpu_port: port { > + dpu_out: endpoint { > + remote-endpoint = <_in>; > + }; > + }; > +}; Be consistent in indent. Now it is a mix of 6 and 8 spaces. (My preference is 4 spaces, but 2,4,6,8 are all OK) End the file with and end statement like this: ... > -- > 2.7.4 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 3/4] igb/igb_ethtool.c : Remove unnecessary usages of memset.
> From: Suraj Upadhyay > Sent: Tuesday, July 14, 2020 12:42 PM > To: Kirsher, Jeffrey T ; da...@davemloft.net; > k...@kernel.org > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; linux- > ker...@vger.kernel.org; kernel-janit...@vger.kernel.org > Subject: [PATCH 3/4] igb/igb_ethtool.c : Remove unnecessary usages of > memset. > > Replace memsets of 1 byte with simple assignment. > Issue found with checkpatch.pl > > Signed-off-by: Suraj Upadhyay > --- > drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Tested-by: Aaron Brown
RE: [PATCH 2/4] e1000e/ethtool.c : Remove unnecessary usages of memset.
> From: Suraj Upadhyay > Sent: Tuesday, July 14, 2020 12:41 PM > To: Kirsher, Jeffrey T ; da...@davemloft.net; > k...@kernel.org > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; linux- > ker...@vger.kernel.org; kernel-janit...@vger.kernel.org > Subject: [PATCH 2/4] e1000e/ethtool.c : Remove unnecessary usages of > memset. > > Replace memsets of 1 byte with simple assignments. > Issue found with checkpatch.pl > > Signed-off-by: Suraj Upadhyay > --- > drivers/net/ethernet/intel/e1000e/ethtool.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Tested-by: Aaron Brown
Re: [PATCH 04/15] iio: sx9310: Remove acpi and of table macros
On Tue, Jul 28, 2020 at 12:09 PM Andy Shevchenko wrote: > > On Tue, Jul 28, 2020 at 6:16 PM Daniel Campello wrote: > > > > Avoids unused warnings due to acpi/of table macros. > > > > At the same time I would check if mod_devicetable.h is included. I did the following and no error showed up: #ifndef LINUX_MOD_DEVICETABLE_H #error Missing include #endif > > > Signed-off-by: Daniel Campello > > Reported-by: kbuild test robot > > > -- > With Best Regards, > Andy Shevchenko Regards, Daniel Campello
RE: [PATCH 1/4] e1000/e1000_ethtool.c: Remove unnecessary usages of memset
> From: Suraj Upadhyay > Sent: Tuesday, July 14, 2020 12:41 PM > To: Kirsher, Jeffrey T ; da...@davemloft.net; > k...@kernel.org > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; linux- > ker...@vger.kernel.org; kernel-janit...@vger.kernel.org > Subject: [PATCH 1/4] e1000/e1000_ethtool.c: Remove unnecessary usages of > memset > > Replace memsets of 1 byte with simple assignments. > Issue reported by checkpatch.pl. > > Signed-off-by: Suraj Upadhyay > --- > drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Tested-by: Aaron Brown
Re: [PATCH RFC v6 2/6] drm/sprd: add Unisoc's drm kms master
Hi Kevin. Nice split of the driver. Some feedback in the following. Most to bring the driver up-to-date with what have happened since we saw it last time. Keeping up with the changes in drm is not always easy. Sam On Tue, Jul 28, 2020 at 06:07:55PM +0800, Kevin Tang wrote: > From: Kevin Tang > > Adds drm support for the Unisoc's display subsystem. > > This is drm kms driver, this driver provides support for the > application framework in Android, Yocto and more. > > Application framework can access Unisoc's display internel > peripherals through libdrm or libkms, it's test ok by modetest > (DRM/KMS test tool) and Android HWComposer. > > Cc: Orson Zhai > Cc: Chunyan Zhang > Signed-off-by: Kevin Tang > --- > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile| 1 + > drivers/gpu/drm/sprd/Kconfig| 12 +++ > drivers/gpu/drm/sprd/Makefile | 5 + > drivers/gpu/drm/sprd/sprd_drm.c | 226 > > drivers/gpu/drm/sprd/sprd_drm.h | 16 +++ > 6 files changed, 262 insertions(+) > create mode 100644 drivers/gpu/drm/sprd/Kconfig > create mode 100644 drivers/gpu/drm/sprd/Makefile > create mode 100644 drivers/gpu/drm/sprd/sprd_drm.c > create mode 100644 drivers/gpu/drm/sprd/sprd_drm.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index c4fd57d..7fe7ab91 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -386,6 +386,8 @@ source "drivers/gpu/drm/mcde/Kconfig" > > source "drivers/gpu/drm/tidss/Kconfig" > > +source "drivers/gpu/drm/sprd/Kconfig" > + > # Keep legacy drivers last > > menuconfig DRM_LEGACY > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 2c0e5a7..ee2ccd9 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -123,3 +123,4 @@ obj-$(CONFIG_DRM_PANFROST) += panfrost/ > obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/ > obj-$(CONFIG_DRM_MCDE) += mcde/ > obj-$(CONFIG_DRM_TIDSS) += tidss/ > +obj-$(CONFIG_DRM_SPRD) += sprd/ > diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig > new file mode 100644 > index 000..b189a54 > --- /dev/null > +++ b/drivers/gpu/drm/sprd/Kconfig > @@ -0,0 +1,12 @@ > +config DRM_SPRD > + tristate "DRM Support for Unisoc SoCs Platform" > + depends on ARCH_SPRD || COMPILE_TEST > + depends on DRM && OF > + select DRM_KMS_HELPER > + select DRM_GEM_CMA_HELPER > + select DRM_KMS_CMA_HELPER > + select DRM_MIPI_DSI > + help > + Choose this option if you have a Unisoc chipsets. > + If M is selected the module will be called sprd-drm. > + > diff --git a/drivers/gpu/drm/sprd/Makefile b/drivers/gpu/drm/sprd/Makefile > new file mode 100644 > index 000..86d95d9 > --- /dev/null > +++ b/drivers/gpu/drm/sprd/Makefile > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +subdir-ccflags-y += -I$(srctree)/$(src) subdir-ccflags-y is not needed - drop it. > + > +obj-y := sprd_drm.o > diff --git a/drivers/gpu/drm/sprd/sprd_drm.c b/drivers/gpu/drm/sprd/sprd_drm.c > new file mode 100644 > index 000..4706185 > --- /dev/null > +++ b/drivers/gpu/drm/sprd/sprd_drm.c > @@ -0,0 +1,226 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2020 Unisoc Inc. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "sprd_drm.h" > + > +#define DRIVER_NAME "sprd" > +#define DRIVER_DESC "Spreadtrum SoCs' DRM Driver" > +#define DRIVER_DATE "20200201" > +#define DRIVER_MAJOR 1 > +#define DRIVER_MINOR 0 > + > +static const struct drm_mode_config_helper_funcs sprd_drm_mode_config_helper > = { > + .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm, > +}; > + > +static const struct drm_mode_config_funcs sprd_drm_mode_config_funcs = { > + .fb_create = drm_gem_fb_create, > + .atomic_check = drm_atomic_helper_check, > + .atomic_commit = drm_atomic_helper_commit, > +}; > + > +static void sprd_drm_mode_config_init(struct drm_device *drm) > +{ > + drm_mode_config_init(drm); The documentation of this functions says: FIXME: This function is deprecated and drivers should be converted over to drmm_mode_config_init(). > + > + drm->mode_config.min_width = 0; > + drm->mode_config.min_height = 0; > + drm->mode_config.max_width = 8192; > + drm->mode_config.max_height = 8192; > + drm->mode_config.allow_fb_modifiers = true; > + > + drm->mode_config.funcs = _drm_mode_config_funcs; > + drm->mode_config.helper_private = _drm_mode_config_helper; > +} > + > +DEFINE_DRM_GEM_CMA_FOPS(sprd_drm_fops); > + > +static struct drm_driver sprd_drm_drv = { > + .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > + .fops = _drm_fops, > + > + /* GEM Operations */ > + DRM_GEM_CMA_VMAP_DRIVER_OPS, > + > +
[PATCH] hv_utils: Add validation for untrusted Hyper-V values
For additional robustness in the face of Hyper-V errors or malicious behavior, validate all values that originate from packets that Hyper-V has sent to the guest in the host-to-guest ring buffer. Ensure that invalid values cannot cause indexing off the end of the icversion_data array in vmbus_prep_negotiate_resp(). Signed-off-by: Andres Beltran --- drivers/hv/channel_mgmt.c | 17 ++- drivers/hv/hv_fcopy.c | 35 +++-- drivers/hv/hv_kvp.c | 121 + drivers/hv/hv_snapshot.c | 88 +++-- drivers/hv/hv_util.c | 265 +++--- include/linux/hyperv.h| 9 +- 6 files changed, 327 insertions(+), 208 deletions(-) diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 591106cf58fc..fcabf488fa58 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -202,8 +202,8 @@ static u16 hv_get_dev_type(const struct vmbus_channel *channel) * Set up and fill in default negotiate response message. * Mainly used by Hyper-V drivers. */ -bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, - u8 *buf, const int *fw_version, int fw_vercnt, +bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, u8 *buf, + u32 buflen, const int *fw_version, int fw_vercnt, const int *srv_version, int srv_vercnt, int *nego_fw_version, int *nego_srv_version) { @@ -216,9 +216,7 @@ bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, struct icmsg_negotiate *negop; icmsghdrp->icmsgsize = 0x10; - negop = (struct icmsg_negotiate *)[ - sizeof(struct vmbuspipe_hdr) + - sizeof(struct icmsg_hdr)]; + negop = (struct icmsg_negotiate *)[ICMSG_HDR]; icframe_major = negop->icframe_vercnt; icframe_minor = 0; @@ -226,6 +224,15 @@ bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, icmsg_major = negop->icmsg_vercnt; icmsg_minor = 0; + /* Validate negop packet */ + if (icframe_major > IC_VERSION_NEGOTIATION_MAX_VER_COUNT || + icmsg_major > IC_VERSION_NEGOTIATION_MAX_VER_COUNT || + ICMSG_NEGOTIATE_PKT_SIZE(icframe_major, icmsg_major) > buflen) { + pr_err("Invalid icmsg negotiate - icframe_major: %u, icmsg_major: %u", + icframe_major, icmsg_major); + goto fw_error; + } + /* * Select the framework version number we will * support. diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c index 5040d7e0cd9e..a9330abe83b2 100644 --- a/drivers/hv/hv_fcopy.c +++ b/drivers/hv/hv_fcopy.c @@ -235,15 +235,26 @@ void hv_fcopy_onchannelcallback(void *context) if (fcopy_transaction.state > HVUTIL_READY) return; - vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 2, , -); - if (recvlen <= 0) + if (vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 2, , )) { + pr_info("Fcopy request received. Could not read into recv buf\n"); return; + } + + if (!recvlen) + return; + + /* Ensure recvlen is big enough to read header data */ + if (recvlen < ICMSG_HDR) { + pr_info("Fcopy request received. Packet length too small: %d\n", recvlen); + return; + } icmsghdr = (struct icmsg_hdr *)_buffer[ sizeof(struct vmbuspipe_hdr)]; + if (icmsghdr->icmsgtype == ICMSGTYPE_NEGOTIATE) { - if (vmbus_prep_negotiate_resp(icmsghdr, recv_buffer, + if (vmbus_prep_negotiate_resp(icmsghdr, + recv_buffer, recvlen, fw_versions, FW_VER_COUNT, fcopy_versions, FCOPY_VER_COUNT, NULL, _srv_version)) { @@ -252,10 +263,14 @@ void hv_fcopy_onchannelcallback(void *context) fcopy_srv_version >> 16, fcopy_srv_version & 0x); } - } else { - fcopy_msg = (struct hv_fcopy_hdr *)_buffer[ - sizeof(struct vmbuspipe_hdr) + - sizeof(struct icmsg_hdr)]; + } else if (icmsghdr->icmsgtype == ICMSGTYPE_FCOPY) { + /* Ensure recvlen is big enough to contain hv_fcopy_hdr */ + if (recvlen < ICMSG_HDR + sizeof(struct hv_fcopy_hdr)) { + pr_info("Invalid Fcopy hdr. Packet length too small: %u\n", + recvlen); + return; + } + fcopy_msg = (struct hv_fcopy_hdr *)_buffer[ICMSG_HDR]; /* * Stash away this global state for completing the @@ -280,6 +295,10 @@ void
Re: [PATCH] tty: Add MOXA NPort Real TTY Driver
Hi! > + This driver can also be built as a module. The module will be called > + npreal2 by setting M. Odd wording... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v2] powercap: Add Power Limit4 support
Hi! > Modern Intel Mobile platforms support power limit4 (PL4), which is > the SoC package level maximum power limit (in Watts). It can be used > to preemptively limits potential SoC power to prevent power spikes > from tripping the power adapter and battery over-current protection. > This patch enables this feature by exposing package level peak power > capping control to userspace via RAPL sysfs interface. With this, > application like DTPF can modify PL4 power limit, the similar way > of other package power limit (PL1). > As this feature is not tested on previous generations, here it is > enabled only for the platform that has been verified to work, > for safety concerns. So what happens when the user sets this too high? Battery on fire? Or just sudden poweroff? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: Linux kernel in-tree Rust support
Hi! > > No, please make it a "is rust available" automatic config option. The > > exact same way we already do the compiler versions and check for > > various availability of compiler flags at config time. > > That sounds even better, and will definitely allow for more testing. > > We just need to make sure that any kernel CI infrastructure tests that > right away, then, so that failures don't get introduced by a patch from > someone without a Rust toolchain and not noticed until someone with a > Rust toolchain tests it. So... I'm fan of Rust, but while trying to use it one thing was obvious: it takes _significantly_ longer than C to compile and needs gigabyte a lot of RAM. Kernel is quite big project, can CI infrastructure handle additional load? Will developers see significantly longer compile times when Rust is widespread? Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v31 00/12] /dev/random - a new approach with full SP800-90B
Hi! > The following patch set provides a different approach to /dev/random which is > called > Linux Random Number Generator (LRNG) to collect entropy within the Linux > kernel. The > main improvements compared to the existing /dev/random is to provide > sufficient entropy > during boot time as well as in virtual environments and when using SSDs. A > secondary > design goal is to limit the impact of the entropy collection on massive > parallel systems > and also allow the use accelerated cryptographic primitives. Also, all steps > of the > entropic data processing are testable. That sounds good.. maybe too good. Where does LRNG get the entropy? That is the part that should be carefully documented.. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v3 19/19] arm64: lto: Strengthen READ_ONCE() to acquire when CONFIG_LTO=y
On Fri 2020-07-10 17:52:03, Will Deacon wrote: > When building with LTO, there is an increased risk of the compiler > converting an address dependency headed by a READ_ONCE() invocation > into a control dependency and consequently allowing for harmful > reordering by the CPU. > > Ensure that such transformations are harmless by overriding the generic > READ_ONCE() definition with one that provides acquire semantics when > building with LTO. Traditionally, READ_ONCE had only effects on compiler optimalizations, not on special semantics of the load instruction. Do you have example how LTO optimalizations break the code? Should some documentation be added? Because I believe users will need to understand what is going on there. It is not LTO-only problem and it is not arm64-only problem, right? Best regards, Pavel > +#ifdef CONFIG_AS_HAS_LDAPR > +#define __LOAD_RCPC(sfx, regs...)\ > + ALTERNATIVE(\ > + "ldar" #sfx "\t" #regs,\ > + ".arch_extension rcpc\n"\ > + "ldapr" #sfx "\t" #regs,\ > + ARM64_HAS_LDAPR) > +#else > +#define __LOAD_RCPC(sfx, regs...)"ldar" #sfx "\t" #regs > +#endif /* CONFIG_AS_HAS_LDAPR */ > + > +#define __READ_ONCE(x) > \ > +({ \ > + typeof(&(x)) __x = &(x);\ > + int atomic = 1; \ > + union { __unqual_scalar_typeof(*__x) __val; char __c[1]; } __u; \ > + switch (sizeof(x)) {\ > + case 1: \ > + asm volatile(__LOAD_RCPC(b, %w0, %1)\ > + : "=r" (*(__u8 *)__u.__c) \ > + : "Q" (*__x) : "memory"); \ > + break; \ > + case 2: \ > + asm volatile(__LOAD_RCPC(h, %w0, %1)\ > + : "=r" (*(__u16 *)__u.__c) \ > + : "Q" (*__x) : "memory"); \ > + break; \ > + case 4: \ > + asm volatile(__LOAD_RCPC(, %w0, %1) \ > + : "=r" (*(__u32 *)__u.__c) \ > + : "Q" (*__x) : "memory"); \ > + break; \ > + case 8: \ > + asm volatile(__LOAD_RCPC(, %0, %1) \ > + : "=r" (*(__u64 *)__u.__c) \ > + : "Q" (*__x) : "memory"); \ > + break; \ > + default:\ > + atomic = 0; \ > + } \ > + atomic ? (typeof(*__x))__u.__val : (*(volatile typeof(__x))__x);\ > +}) > + > +#endif /* !BUILD_VDSO */ > +#endif /* CONFIG_LTO */ > + > +#include > + > +#endif /* __ASM_RWONCE_H */ > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile > index 45d5cfe46429..60df97f2e7de 100644 > --- a/arch/arm64/kernel/vdso/Makefile > +++ b/arch/arm64/kernel/vdso/Makefile > @@ -28,7 +28,7 @@ ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 > --hash-style=sysv\ >$(btildflags-y) -T > > ccflags-y := -fno-common -fno-builtin -fno-stack-protector -ffixed-x18 > -ccflags-y += -DDISABLE_BRANCH_PROFILING > +ccflags-y += -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO > > CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os $(CC_FLAGS_SCS) > $(GCC_PLUGINS_CFLAGS) > KBUILD_CFLAGS+= $(DISABLE_LTO) > diff --git a/arch/arm64/kernel/vdso32/Makefile > b/arch/arm64/kernel/vdso32/Makefile > index d88148bef6b0..4fdf3754a058 100644 > --- a/arch/arm64/kernel/vdso32/Makefile > +++ b/arch/arm64/kernel/vdso32/Makefile > @@ -43,7 +43,7 @@ cc32-as-instr = $(call try-run,\ > # As a result we set our own flags here. > > # KBUILD_CPPFLAGS and NOSTDINC_FLAGS from top-level Makefile > -VDSO_CPPFLAGS := -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT) > -print-file-name=include) > +VDSO_CPPFLAGS := -DBUILD_VDSO
RE: [Intel-wired-lan] [PATCH v2] igb: reinit_locked() should be called with rtnl_lock
> From: Intel-wired-lan On Behalf Of > Francesco Ruggeri > Sent: Thursday, July 2, 2020 3:39 PM > To: linux-kernel@vger.kernel.org; net...@vger.kernel.org; intel-wired- > l...@lists.osuosl.org; k...@kernel.org; da...@davemloft.net; Kirsher, Jeffrey > T ; frugg...@arista.com > Subject: [Intel-wired-lan] [PATCH v2] igb: reinit_locked() should be called > with > rtnl_lock > > We observed two panics involving races with igb_reset_task. > The first panic is caused by this race condition: > > kworker reboot -f > > igb_reset_task > igb_reinit_locked > igb_down > napi_synchronize > __igb_shutdown > igb_clear_interrupt_scheme > igb_free_q_vectors > igb_free_q_vector > adapter->q_vector[v_idx] = NULL; > napi_disable > Panics trying to access > adapter->q_vector[v_idx].napi_state > > The second panic (a divide error) is caused by this race: > > kworker reboot -f tx packet > > igb_reset_task > __igb_shutdown > rtnl_lock() > ... > igb_clear_interrupt_scheme > igb_free_q_vectors > adapter->num_tx_queues = 0 > ... > rtnl_unlock() > rtnl_lock() > igb_reinit_locked > igb_down > igb_up > netif_tx_start_all_queues > dev_hard_start_xmit > igb_xmit_frame > igb_tx_queue_mapping > Panics on > r_idx % adapter->num_tx_queues > > This commit applies to igb_reset_task the same changes that > were applied to ixgbe in commit 2f90b8657ec9 ("ixgbe: this patch > adds support for DCB to the kernel and ixgbe driver"), > commit 8f4c5c9fb87a ("ixgbe: reinit_locked() should be called with > rtnl_lock") and commit 88adce4ea8f9 ("ixgbe: fix possible race in > reset subtask"). > > v2: add fix for second race condition above. > > Signed-off-by: Francesco Ruggeri > > --- > drivers/net/ethernet/intel/igb/igb_main.c | 9 + > 1 file changed, 9 insertions(+) Tested-by: Aaron Brown
Re: [PATCH net 0/3] net: lan78xx: fix NULL deref and memory leak
From: Johan Hovold Date: Tue, 28 Jul 2020 14:10:28 +0200 > The first two patches fix a NULL-pointer dereference at probe that can > be triggered by a malicious device and a small transfer-buffer memory > leak, respectively. > > For another subsystem I would have marked them: > > Cc: sta...@vger.kernel.org # 4.3 > > The third one replaces the driver's current broken endpoint lookup > helper, which could end up accepting incomplete interfaces and whose > results weren't even useeren Series applied and queued up for -stable.
RE: [Intel-wired-lan] [v4][PATCH] e1000e: continue to init phy even when failed to disable ULP
> From: Intel-wired-lan On Behalf Of > Aaron Ma > Sent: Wednesday, June 17, 2020 11:55 PM > To: k...@kernel.org; Kirsher, Jeffrey T ; > da...@davemloft.net; intel-wired-...@lists.osuosl.org; > net...@vger.kernel.org; linux-kernel@vger.kernel.org; Lifshits, Vitaly > ; kai.heng.f...@canonical.com; Neftin, Sasha > > Subject: [Intel-wired-lan] [v4][PATCH] e1000e: continue to init phy even when > failed to disable ULP > > After 'commit e086ba2fccda4 ("e1000e: disable s0ix entry and exit flows > for ME systems")', > ThinkPad P14s always failed to disable ULP by ME. > 'commit 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")' > break out of init phy: > > error log: > [ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP > [ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast > Packet > [ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error > > When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1. > If continue to init phy like before, it can work as before. > iperf test result good too. > > Fixes: 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed") > Signed-off-by: Aaron Ma > --- > drivers/net/ethernet/intel/e1000e/ich8lan.c | 1 - > 1 file changed, 1 deletion(-) I never did find a system that triggered the initial problem, but from a compatibility with the set of systems I do have working... Tested-by: Aaron Brown
Re: [PATCH V4 2/3] mfd: Intel Platform Monitoring Technology support
Hi Lee, Thanks for this thorough review. Ack on all the comments with particular thanks for spoting the missing continue. David On Tue, 2020-07-28 at 08:58 +0100, Lee Jones wrote: > On Fri, 17 Jul 2020, David E. Box wrote: > > > Intel Platform Monitoring Technology (PMT) is an architecture for > > enumerating and accessing hardware monitoring facilities. PMT > > supports > > multiple types of monitoring capabilities. This driver creates > > platform > > devices for each type so that they may be managed by capability > > specific > > drivers (to be introduced). Capabilities are discovered using PCIe > > DVSEC > > ids. Support is included for the 3 current capability types, > > Telemetry, > > Watcher, and Crashlog. The features are available on new Intel > > platforms > > starting from Tiger Lake for which support is added. > > > > Also add a quirk mechanism for several early hardware differences > > and bugs. > > For Tiger Lake, do not support Watcher and Crashlog capabilities > > since they > > will not be compatible with future product. Also, fix use a quirk > > to fix > > the discovery table offset. > > > > Reviewed-by: Andy Shevchenko > > Co-developed-by: Alexander Duyck > > > > Signed-off-by: Alexander Duyck > > Signed-off-by: David E. Box > > This should be in chronological order. > > > --- > > MAINTAINERS | 5 + > > drivers/mfd/Kconfig | 10 ++ > > drivers/mfd/Makefile| 1 + > > drivers/mfd/intel_pmt.c | 215 > > > > 4 files changed, 231 insertions(+) > > create mode 100644 drivers/mfd/intel_pmt.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index b4a43a9e7fbc..2e42bf0c41ab 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -8845,6 +8845,11 @@ F: drivers/mfd/intel_soc_pmic* > > F: include/linux/mfd/intel_msic.h > > F: include/linux/mfd/intel_soc_pmic* > > > > +INTEL PMT DRIVER > > +M: "David E. Box" > > +S: Maintained > > +F: drivers/mfd/intel_pmt.c > > + > > INTEL PRO/WIRELESS 2100, 2200BG, 2915ABG NETWORK CONNECTION > > SUPPORT > > M: Stanislav Yakovlev > > L: linux-wirel...@vger.kernel.org > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > index a37d7d171382..1a62ce2c68d9 100644 > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -670,6 +670,16 @@ config MFD_INTEL_PMC_BXT > > Register and P-unit access. In addition this creates devices > > for iTCO watchdog and telemetry that are part of the PMC. > > > > +config MFD_INTEL_PMT > > + tristate "Intel Platform Monitoring Technology support" > > Nit: "Intel Platform Monitoring Technology (PMT) support" > > > + depends on PCI > > + select MFD_CORE > > + help > > + The Intel Platform Monitoring Technology (PMT) is an > > interface that > > + provides access to hardware monitor registers. This driver > > supports > > + Telemetry, Watcher, and Crashlog PMT capabilities/devices for > > + platforms starting from Tiger Lake. > > + > > config MFD_IPAQ_MICRO > > bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support" > > depends on SA1100_H3100 || SA1100_H3600 > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > index 9367a92f795a..1961b4737985 100644 > > --- a/drivers/mfd/Makefile > > +++ b/drivers/mfd/Makefile > > @@ -216,6 +216,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS_PCI)+= > > intel-lpss-pci.o > > obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o > > obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o > > obj-$(CONFIG_MFD_INTEL_PMC_BXT)+= intel_pmc_bxt.o > > +obj-$(CONFIG_MFD_INTEL_PMT)+= intel_pmt.o > > obj-$(CONFIG_MFD_PALMAS) += palmas.o > > obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o > > obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o > > diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c > > new file mode 100644 > > index ..6857eaf4ff86 > > --- /dev/null > > +++ b/drivers/mfd/intel_pmt.c > > @@ -0,0 +1,215 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Intel Platform Monitoring Technology MFD driver > > s/MFD/(PMT)/ > > > + * Copyright (c) 2020, Intel Corporation. > > + * All Rights Reserved. > > + * > > + * Authors: David E. Box > > Looks odd to use a plural for a single author. > > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > Alphabetical please. > > > +/* Intel DVSEC capability vendor space offsets */ > > +#define INTEL_DVSEC_ENTRIES0xA > > +#define INTEL_DVSEC_SIZE 0xB > > +#define INTEL_DVSEC_TABLE 0xC > > +#define INTEL_DVSEC_TABLE_BAR(x) ((x) & GENMASK(2, 0)) > > +#define INTEL_DVSEC_TABLE_OFFSET(x)((x) & GENMASK(31, 3)) > > +#define INTEL_DVSEC_ENTRY_SIZE 4 > > + > > +/* PMT capabilities */ > > +#define DVSEC_INTEL_ID_TELEMETRY 2 > > +#define DVSEC_INTEL_ID_WATCHER 3 > > +#define
Re: [PATCH v7] Makefile: Add clang-tidy and static analyzer support to makefile
On Mon, Jul 27, 2020 at 5:47 PM 'Nathan Huckleberry' via Clang Built Linux wrote: > > This patch adds clang-tidy and the clang static-analyzer as make > targets. The goal of this patch is to make static analysis tools > usable and extendable by any developer or researcher who is familiar > with basic c++. > > The current static analysis tools require intimate knowledge of the > internal workings of the static analysis. Clang-tidy and the clang > static analyzers expose an easy to use api and allow users unfamiliar > with clang to write new checks with relative ease. > > ===Clang-tidy=== > > Clang-tidy is an easily extendable 'linter' that runs on the AST. > Clang-tidy checks are easy to write and understand. A check consists of > two parts, a matcher and a checker. The matcher is created using a > domain specific language that acts on the AST > (https://clang.llvm.org/docs/LibASTMatchersReference.html). When AST > nodes are found by the matcher a callback is made to the checker. The > checker can then execute additional checks and issue warnings. > > Here is an example clang-tidy check to report functions that have calls > to local_irq_disable without calls to local_irq_enable and vice-versa. > Functions flagged with __attribute((annotation("ignore_irq_balancing"))) > are ignored for analysis. (https://reviews.llvm.org/D65828) > > ===Clang static analyzer=== > > The clang static analyzer is a more powerful static analysis tool that > uses symbolic execution to find bugs. Currently there is a check that > looks for potential security bugs from invalid uses of kmalloc and > kfree. There are several more general purpose checks that are useful for > the kernel. > > The clang static analyzer is well documented and designed to be > extensible. > (https://clang-analyzer.llvm.org/checker_dev_manual.html) > (https://github.com/haoNoQ/clang-analyzer-guide/releases/download/v0.1/clang-analyzer-guide-v0.1.pdf) > > The main draw of the clang tools is how accessible they are. The clang > documentation is very nice and these tools are built specifically to be > easily extendable by any developer. They provide an accessible method of > bug-finding and research to people who are not overly familiar with the > kernel codebase. > > Signed-off-by: Nathan Huckleberry > --- > Changes v6->v7 > * Fix issues with relative paths > * Additional style fixes > MAINTAINERS | 1 + > Makefile | 3 + > scripts/clang-tools/Makefile.clang-tools | 23 ++ > .../{ => clang-tools}/gen_compile_commands.py | 0 > scripts/clang-tools/run-clang-tools.py| 74 +++ > 5 files changed, 101 insertions(+) > create mode 100644 scripts/clang-tools/Makefile.clang-tools > rename scripts/{ => clang-tools}/gen_compile_commands.py (100%) > create mode 100755 scripts/clang-tools/run-clang-tools.py > > diff --git a/MAINTAINERS b/MAINTAINERS > index 1d4aa7f942de..a444564e5572 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4198,6 +4198,7 @@ W:https://clangbuiltlinux.github.io/ > B: https://github.com/ClangBuiltLinux/linux/issues > C: irc://chat.freenode.net/clangbuiltlinux > F: Documentation/kbuild/llvm.rst > +F: scripts/clang-tools/ > K: \b(?i:clang|llvm)\b > > CLEANCACHE API > diff --git a/Makefile b/Makefile > index fe0164a654c7..3e2df010b342 100644 > --- a/Makefile > +++ b/Makefile > @@ -747,6 +747,7 @@ KBUILD_CFLAGS += $(call > cc-option,-fno-allow-store-data-races) > > include scripts/Makefile.kcov > include scripts/Makefile.gcc-plugins > +include scripts/clang-tools/Makefile.clang-tools > > ifdef CONFIG_READABLE_ASM > # Disable optimizations that make assembler listings hard to read. > @@ -1543,6 +1544,8 @@ help: > @echo ' export_report - List the usages of all exported symbols' > @echo ' headerdep - Detect inclusion cycles in headers' > @echo ' coccicheck - Check with Coccinelle' > + @echo ' clang-analyzer - Check with clang static analyzer' > + @echo ' clang-tidy - Check with clang-tidy' > @echo '' > @echo 'Tools:' > @echo ' nsdeps - Generate missing symbol namespace > dependencies' > diff --git a/scripts/clang-tools/Makefile.clang-tools > b/scripts/clang-tools/Makefile.clang-tools > new file mode 100644 > index ..5c9d76f77595 > --- /dev/null > +++ b/scripts/clang-tools/Makefile.clang-tools > @@ -0,0 +1,23 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Copyright (C) Google LLC, 2020 > +# > +# Author: Nathan Huckleberry > +# > +PHONY += clang-tidy > +clang-tidy: > +ifdef CONFIG_CC_IS_CLANG > + $(PYTHON3) scripts/clang-tools/gen_compile_commands.py > + $(PYTHON3) scripts/clang-tools/run-clang-tools.py clang-tidy > compile_commands.json > +else > + $(error clang-tidy requires CC=clang) > +endif > + > +PHONY += clang-analyzer > +clang-analyzer: > +ifdef
[PATCH net] ptp: ptp_clockmatrix: update to support 4.8.7 firmware
From: Min Li With 4.8.7 firmware, adjtime can change delta instead of absolute time, which greately increases snap accuracy. PPS alignment doesn't have to be set for every single TOD change. Other minor changes includes: adding more debug logs, increasing snap accuracy for pre 4.8.7 firmware and supporting new tcs2bin format. Signed-off-by: Min Li --- drivers/ptp/idt8a340_reg.h| 48 ++ drivers/ptp/ptp_clockmatrix.c | 1151 ++--- drivers/ptp/ptp_clockmatrix.h | 61 ++- 3 files changed, 1048 insertions(+), 212 deletions(-) diff --git a/drivers/ptp/idt8a340_reg.h b/drivers/ptp/idt8a340_reg.h index 69eedda..b297c4a 100644 --- a/drivers/ptp/idt8a340_reg.h +++ b/drivers/ptp/idt8a340_reg.h @@ -20,6 +20,10 @@ #define HW_DPLL_1 (0x8b00) #define HW_DPLL_2 (0x8c00) #define HW_DPLL_3 (0x8d00) +#define HW_DPLL_4 (0x8e00) +#define HW_DPLL_5 (0x8f00) +#define HW_DPLL_6 (0x9000) +#define HW_DPLL_7 (0x9100) #define HW_DPLL_TOD_SW_TRIG_ADDR__0 (0x080) #define HW_DPLL_TOD_CTRL_1(0x089) @@ -57,6 +61,43 @@ #define SYNCTRL1_Q1_DIV_SYNC_TRIG BIT(1) #define SYNCTRL1_Q0_DIV_SYNC_TRIG BIT(0) +#define HW_Q8_CTRL_SPARE (0xa7d4) +#define HW_Q11_CTRL_SPARE (0xa7ec) + +/** + * Select FOD5 as sync_trigger for Q8 divider. + * Transition from logic zero to one + * sets trigger to sync Q8 divider. + * + * Unused when FOD4 is driving Q8 divider (normal operation). + */ +#define Q9_TO_Q8_SYNC_TRIG BIT(1) + +/** + * Enable FOD5 as driver for clock and sync for Q8 divider. + * Enable fanout buffer for FOD5. + * + * Unused when FOD4 is driving Q8 divider (normal operation). + */ +#define Q9_TO_Q8_FANOUT_AND_CLOCK_SYNC_ENABLE_MASK (BIT(0) | BIT(2)) + +/** + * Select FOD6 as sync_trigger for Q11 divider. + * Transition from logic zero to one + * sets trigger to sync Q11 divider. + * + * Unused when FOD7 is driving Q11 divider (normal operation). + */ +#define Q10_TO_Q11_SYNC_TRIG BIT(1) + +/** + * Enable FOD6 as driver for clock and sync for Q11 divider. + * Enable fanout buffer for FOD6. + * + * Unused when FOD7 is driving Q11 divider (normal operation). + */ +#define Q10_TO_Q11_FANOUT_AND_CLOCK_SYNC_ENABLE_MASK (BIT(0) | BIT(2)) + #define RESET_CTRL0xc000 #define SM_RESET 0x0012 #define SM_RESET_CMD 0x5A @@ -191,6 +232,7 @@ #define DPLL_CTRL_0 0xc600 #define DPLL_CTRL_DPLL_MANU_REF_CFG 0x0001 +#define DPLL_CTRL_COMBO_MASTER_CFG0x003a #define DPLL_CTRL_1 0xc63c @@ -646,6 +688,9 @@ /* Bit definitions for the TOD_WRITE_CMD register */ #define TOD_WRITE_SELECTION_SHIFT (0) #define TOD_WRITE_SELECTION_MASK (0xf) +/* 4.8.7 */ +#define TOD_WRITE_TYPE_SHIFT (4) +#define TOD_WRITE_TYPE_MASK (0x3) /* Bit definitions for the TOD_READ_PRIMARY_SEL_CFG_0 register */ #define RD_PWM_DECODER_INDEX_SHIFT(4) @@ -658,4 +703,7 @@ #define TOD_READ_TRIGGER_SHIFT(0) #define TOD_READ_TRIGGER_MASK (0xf) +/* Bit definitions for the DPLL_CTRL_COMBO_MASTER_CFG register */ +#define COMBO_MASTER_HOLD BIT(0) + #endif diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c index ceb6bc5..73aaae5 100644 --- a/drivers/ptp/ptp_clockmatrix.c +++ b/drivers/ptp/ptp_clockmatrix.c @@ -13,6 +13,7 @@ #include #include #include +#include #include "ptp_private.h" #include "ptp_clockmatrix.h" @@ -23,6 +24,13 @@ MODULE_AUTHOR("IDT support-1588 "); MODULE_VERSION("1.0"); MODULE_LICENSE("GPL"); +/* + * The name of the firmware file to be loaded + * over-rides any automatic selection + */ +static char *firmware; +module_param(firmware, charp, 0); + #define SETTIME_CORRECTION (0) static long set_write_phase_ready(struct ptp_clock_info *ptp) @@ -95,6 +103,45 @@ static int timespec_to_char_array(struct timespec64 const *ts, return 0; } +static int idtcm_strverscmp(const char *ver1, const char *ver2) +{ + u8 num1; + u8 num2; + int result = 0; + + /* loop through each level of the version string */ + while (result == 0) { + /* extract leading version numbers */ + if (kstrtou8(ver1, 10, ) < 0) + return -1; + + if (kstrtou8(ver2, 10, ) < 0) + return -1; + + /* if numbers differ, then set the result */ + if (num1 < num2) + result = -1; + else if (num1 > num2) + result = 1; + else { + /* if numbers are the same, go to next level */ + ver1 = strchr(ver1, '.'); + ver2 =
Re: [PATCH RFC v6 1/6] dt-bindings: display: add Unisoc's drm master bindings
Hi Kevin On Tue, Jul 28, 2020 at 06:07:54PM +0800, Kevin Tang wrote: > From: Kevin Tang > > The Unisoc DRM master device is a virtual device needed to list all > DPU devices or other display interface nodes that comprise the > graphics subsystem > > Cc: Orson Zhai > Cc: Chunyan Zhang > Signed-off-by: Kevin Tang > --- > .../devicetree/bindings/display/sprd/drm.yaml | 36 > ++ > 1 file changed, 36 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/sprd/drm.yaml > > diff --git a/Documentation/devicetree/bindings/display/sprd/drm.yaml > b/Documentation/devicetree/bindings/display/sprd/drm.yaml > new file mode 100644 > index 000..b5792c0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/sprd/drm.yaml drm seems like a sub-optimal name. How about usign the compatible name "display-subsystem" as it is a bit more specific (but not good). > @@ -0,0 +1,36 @@ > +# SPDX-License-Identifier: GPL-2.0 Any chance this can be (GPL-2.0-only OR BSD-2-Clause). I noticed that for example clock/sprd,sc9863a-clk.yaml uses this license so I hope this is OK. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/sprd/drm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Unisoc DRM master device > + > +maintainers: > + - Mark Rutland > + > +description: | > + The Unisoc DRM master device is a virtual device needed to list all > + DPU devices or other display interface nodes that comprise the > + graphics subsystem. > + > +properties: > + compatible: > +const: sprd,display-subsystem > + > + ports: > +description: > + Should contain a list of phandles pointing to display interface port > + of DPU devices. Add type - like this: $ref: /schemas/types.yaml#/definitions/phandle-array See for example display/rockchip/rockchip-drm.yaml Any specific reason why this is not a ports node like used by many other display bindings? In other words - I think this is too simple. > + > +required: > + - compatible > + - ports > + Add: additionalProperties: false so we catch if other properties sneak in. > +examples: > + - | > +display-subsystem { > +compatible = "sprd,display-subsystem"; > +ports = <_out>; > +}; > + > -- > 2.7.4 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/6] Reclaim x86 psABI TIF flags
Hi, This patch set reduces the number of arch-specific TIF_ flags in x86, as a clean up to reduce the pressure over the few remaining x86_32 TIF bits and as a preparation to have the arch-agnostic TIF_ flags shared by different architectures by the common syscall entry code recently published by Thomas Gleixner. This is based on top of: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/entry It was tested by booting and running an i386 rootfs over the 64bit kernel, and also by running x32 binaries on top of it. There is one remaining use of TIF_IA32 in User Mode Linux which is turned off by a bogus define, such that I think it is currently dead code and apparently a bug. I think, it was wired to an old TIF_IA32 flag definition that doesn't exist anymore in arch/um. I think it deserves a fix other than dropping that code, but it is a different piece of work that I will tackle next. Gabriel Krisman Bertazi (6): arch: x86: Don't use TIF flags for mm context arch: x86: Wrap TIF_IA32 checks arch: x86: Wrap TIF_X32 checks arch: x86: Expose psABI on thread_info arch: x86: Reclaim TIF_IA32 flag arch: x86: Reclaim TIF_X32 flag arch/x86/entry/vdso/vma.c | 2 +- arch/x86/events/core.c | 4 ++-- arch/x86/events/intel/ds.c | 2 +- arch/x86/events/intel/lbr.c| 2 +- arch/x86/include/asm/compat.h | 2 +- arch/x86/include/asm/elf.h | 2 +- arch/x86/include/asm/mmu_context.h | 2 +- arch/x86/include/asm/thread_info.h | 15 +++ arch/x86/kernel/perf_regs.c| 2 +- arch/x86/kernel/process_64.c | 16 ++-- arch/x86/oprofile/backtrace.c | 2 +- 11 files changed, 27 insertions(+), 24 deletions(-) -- 2.27.0
[PATCH 4/6] arch: x86: Expose psABI on thread_info
Expose psABI in thread_info, in preparation for the TIF_IA32 and TIF_X32 flags removal. Signed-off-by: Gabriel Krisman Bertazi --- arch/x86/include/asm/thread_info.h | 2 ++ arch/x86/kernel/process_64.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index 6d55a9c0dda2..698feefd5f5f 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -62,11 +62,13 @@ enum { struct thread_info { unsigned long flags; /* low level flags */ u32 status; /* thread synchronous flags */ + short int psabi; }; #define INIT_THREAD_INFO(tsk) \ { \ .flags = 0,\ + .psabi = 0,\ } #else /* !__ASSEMBLY__ */ diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index f20a365017b8..aea2c03e8a5d 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -532,6 +532,7 @@ void set_personality_64bit(void) /* inherit personality from parent */ /* Make sure to be in 64bit mode */ + current_thread_info()->psabi = PSABI_IA64; clear_thread_flag(TIF_IA32); clear_thread_flag(TIF_ADDR32); clear_thread_flag(TIF_X32); @@ -553,6 +554,7 @@ void set_personality_64bit(void) static void __set_personality_x32(void) { #ifdef CONFIG_X86_X32 + current_thread_info()->psabi = PSABI_X32; clear_thread_flag(TIF_IA32); set_thread_flag(TIF_X32); if (current->mm) @@ -574,6 +576,7 @@ static void __set_personality_x32(void) static void __set_personality_ia32(void) { #ifdef CONFIG_IA32_EMULATION + current_thread_info()->psabi = PSABI_IA32; set_thread_flag(TIF_IA32); clear_thread_flag(TIF_X32); if (current->mm) -- 2.27.0
[PATCH 5/6] arch: x86: Reclaim TIF_IA32 flag
Dropping this as a TIF flag is interesting given the pressure over x86 remaining x86 flags, plus considering the current common entry code, reducing arch-specific flags is a good thing. Notice that no path really relies on TIF_IA32 as part of a critical path, therefore the cost of checking another field in thread_info shouldn't be a problem. Signed-off-by: Gabriel Krisman Bertazi --- arch/x86/include/asm/thread_info.h | 4 +--- arch/x86/kernel/process_64.c | 3 --- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index 698feefd5f5f..aa7d27054a8a 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -99,7 +99,6 @@ struct thread_info { #define TIF_NEED_FPU_LOAD 14 /* load FPU on return to userspace */ #define TIF_NOCPUID15 /* CPUID is not accessible in userland */ #define TIF_NOTSC 16 /* TSC is not accessible in userland */ -#define TIF_IA32 17 /* IA32 compatibility process */ #define TIF_SLD18 /* Restore split lock detection on context switch */ #define TIF_MEMDIE 20 /* is terminating due to OOM killer */ #define TIF_POLLING_NRFLAG 21 /* idle is polling for TIF_NEED_RESCHED */ @@ -129,7 +128,6 @@ struct thread_info { #define _TIF_NEED_FPU_LOAD (1 << TIF_NEED_FPU_LOAD) #define _TIF_NOCPUID (1 << TIF_NOCPUID) #define _TIF_NOTSC (1 << TIF_NOTSC) -#define _TIF_IA32 (1 << TIF_IA32) #define _TIF_SLD (1 << TIF_SLD) #define _TIF_POLLING_NRFLAG(1 << TIF_POLLING_NRFLAG) #define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP) @@ -245,7 +243,7 @@ extern void arch_setup_new_exec(void); #define arch_setup_new_exec arch_setup_new_exec #endif /* !__ASSEMBLY__ */ -#define TASK_IA32(tsk) test_tsk_thread_flag(tsk, TIF_IA32) +#define TASK_IA32(tsk) (task_thread_info(tsk)->psabi == PSABI_IA32) #define TASK_X32(tsk) test_tsk_thread_flag(tsk, TIF_X32) #endif /* _ASM_X86_THREAD_INFO_H */ diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index aea2c03e8a5d..75059c9de829 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -533,7 +533,6 @@ void set_personality_64bit(void) /* Make sure to be in 64bit mode */ current_thread_info()->psabi = PSABI_IA64; - clear_thread_flag(TIF_IA32); clear_thread_flag(TIF_ADDR32); clear_thread_flag(TIF_X32); /* Pretend that this comes from a 64bit execve */ @@ -555,7 +554,6 @@ static void __set_personality_x32(void) { #ifdef CONFIG_X86_X32 current_thread_info()->psabi = PSABI_X32; - clear_thread_flag(TIF_IA32); set_thread_flag(TIF_X32); if (current->mm) current->mm->context.ia32_compat = PSABI_X32; @@ -577,7 +575,6 @@ static void __set_personality_ia32(void) { #ifdef CONFIG_IA32_EMULATION current_thread_info()->psabi = PSABI_IA32; - set_thread_flag(TIF_IA32); clear_thread_flag(TIF_X32); if (current->mm) current->mm->context.ia32_compat = PSABI_IA32; -- 2.27.0
[PATCH 6/6] arch: x86: Reclaim TIF_X32 flag
Dropping this as a TIF flag is interesting given the pressure over x86 remaining x86 flags, plus considering the current common entry code, reducing arch-specific flags is a good thing. Notice that no path really relies on TIF_X32 as part of a critical path, therefore the cost of checking another field in thread_info shouldn't be a problem. Signed-off-by: Gabriel Krisman Bertazi --- arch/x86/events/core.c | 2 +- arch/x86/include/asm/thread_info.h | 4 +--- arch/x86/kernel/process_64.c | 3 --- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index 42dff74c6197..389a840e2211 100644 --- a/arch/x86/events/core.c +++ b/arch/x86/events/core.c @@ -2574,7 +2574,7 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs * IA32 - Where we need to look at GDT/LDT segment descriptor tables * to figure out what the 32bit base address is. * - *X32 - has TIF_X32 set, but is running in x86_64 + *X32 - has PSABI_X32 set, but is running in x86_64 * * X86_64 - CS,DS,SS,ES are all zero based. */ diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index aa7d27054a8a..3059af355cdb 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -108,7 +108,6 @@ struct thread_info { #define TIF_LAZY_MMU_UPDATES 27 /* task is updating the mmu lazily */ #define TIF_SYSCALL_TRACEPOINT 28 /* syscall tracepoint instrumentation */ #define TIF_ADDR32 29 /* 32-bit address space on 64 bits */ -#define TIF_X3230 /* 32-bit native x86-64 binary */ #define TIF_FSCHECK31 /* Check FS is USER_DS on return */ #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) @@ -136,7 +135,6 @@ struct thread_info { #define _TIF_LAZY_MMU_UPDATES (1 << TIF_LAZY_MMU_UPDATES) #define _TIF_SYSCALL_TRACEPOINT(1 << TIF_SYSCALL_TRACEPOINT) #define _TIF_ADDR32(1 << TIF_ADDR32) -#define _TIF_X32 (1 << TIF_X32) #define _TIF_FSCHECK (1 << TIF_FSCHECK) /* flags to check in __switch_to() */ @@ -244,6 +242,6 @@ extern void arch_setup_new_exec(void); #endif /* !__ASSEMBLY__ */ #define TASK_IA32(tsk) (task_thread_info(tsk)->psabi == PSABI_IA32) -#define TASK_X32(tsk) test_tsk_thread_flag(tsk, TIF_X32) +#define TASK_X32(tsk) (task_thread_info(tsk)->psabi == PSABI_X32) #endif /* _ASM_X86_THREAD_INFO_H */ diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index 75059c9de829..d9a72e186db6 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -534,7 +534,6 @@ void set_personality_64bit(void) /* Make sure to be in 64bit mode */ current_thread_info()->psabi = PSABI_IA64; clear_thread_flag(TIF_ADDR32); - clear_thread_flag(TIF_X32); /* Pretend that this comes from a 64bit execve */ task_pt_regs(current)->orig_ax = __NR_execve; current_thread_info()->status &= ~TS_COMPAT; @@ -554,7 +553,6 @@ static void __set_personality_x32(void) { #ifdef CONFIG_X86_X32 current_thread_info()->psabi = PSABI_X32; - set_thread_flag(TIF_X32); if (current->mm) current->mm->context.ia32_compat = PSABI_X32; current->personality &= ~READ_IMPLIES_EXEC; @@ -575,7 +573,6 @@ static void __set_personality_ia32(void) { #ifdef CONFIG_IA32_EMULATION current_thread_info()->psabi = PSABI_IA32; - clear_thread_flag(TIF_X32); if (current->mm) current->mm->context.ia32_compat = PSABI_IA32; current->personality |= force_personality32; -- 2.27.0
[PATCH 1/6] arch: x86: Don't use TIF flags for mm context
TIF_IA32 and TIF_X32 are going away. Create a dedicated enum for the MM context. Signed-off-by: Gabriel Krisman Bertazi --- arch/x86/include/asm/mmu_context.h | 2 +- arch/x86/include/asm/thread_info.h | 6 ++ arch/x86/kernel/process_64.c | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h index 47562147e70b..055ee5d66b41 100644 --- a/arch/x86/include/asm/mmu_context.h +++ b/arch/x86/include/asm/mmu_context.h @@ -178,7 +178,7 @@ static inline void arch_exit_mmap(struct mm_struct *mm) static inline bool is_64bit_mm(struct mm_struct *mm) { return !IS_ENABLED(CONFIG_IA32_EMULATION) || - !(mm->context.ia32_compat == TIF_IA32); + !(mm->context.ia32_compat == PSABI_IA32); } #else static inline bool is_64bit_mm(struct mm_struct *mm) diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index 267701ae3d86..934aa15b20f2 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -53,6 +53,12 @@ struct task_struct; #include #include +enum { + PSABI_IA64 = 0, + PSABI_IA32 = 1, + PSABI_X32 = 2 +}; + struct thread_info { unsigned long flags; /* low level flags */ u32 status; /* thread synchronous flags */ diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index 9a97415b2139..4452a35402f9 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -557,7 +557,7 @@ static void __set_personality_x32(void) clear_thread_flag(TIF_IA32); set_thread_flag(TIF_X32); if (current->mm) - current->mm->context.ia32_compat = TIF_X32; + current->mm->context.ia32_compat = PSABI_X32; current->personality &= ~READ_IMPLIES_EXEC; /* * in_32bit_syscall() uses the presence of the x32 syscall bit @@ -578,7 +578,7 @@ static void __set_personality_ia32(void) set_thread_flag(TIF_IA32); clear_thread_flag(TIF_X32); if (current->mm) - current->mm->context.ia32_compat = TIF_IA32; + current->mm->context.ia32_compat = PSABI_IA32; current->personality |= force_personality32; /* Prepare the first "return" to user space */ task_pt_regs(current)->orig_ax = __NR_ia32_execve; -- 2.27.0
[PATCH 2/6] arch: x86: Wrap TIF_IA32 checks
In preparation to remove TIF_IA32, add wrapper that check the process has IA32 ABI without using the flag directly. Signed-off-by: Gabriel Krisman Bertazi --- arch/x86/events/core.c | 2 +- arch/x86/events/intel/ds.c | 2 +- arch/x86/events/intel/lbr.c| 2 +- arch/x86/include/asm/compat.h | 2 +- arch/x86/include/asm/thread_info.h | 2 ++ arch/x86/kernel/perf_regs.c| 2 +- arch/x86/oprofile/backtrace.c | 2 +- 7 files changed, 8 insertions(+), 6 deletions(-) diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index 4103665c6e03..42dff74c6197 100644 --- a/arch/x86/events/core.c +++ b/arch/x86/events/core.c @@ -2491,7 +2491,7 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent struct stack_frame_ia32 frame; const struct stack_frame_ia32 __user *fp; - if (!test_thread_flag(TIF_IA32)) + if (!TASK_IA32(current)) return 0; cs_base = get_segment_base(regs->cs); diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c index dc43cc124e09..27d1cc1f3d05 100644 --- a/arch/x86/events/intel/ds.c +++ b/arch/x86/events/intel/ds.c @@ -1261,7 +1261,7 @@ static int intel_pmu_pebs_fixup_ip(struct pt_regs *regs) old_to = to; #ifdef CONFIG_X86_64 - is_64bit = kernel_ip(to) || !test_thread_flag(TIF_IA32); + is_64bit = kernel_ip(to) || !TASK_IA32(current); #endif insn_init(, kaddr, size, is_64bit); insn_get_length(); diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c index 65113b16804a..6c097a2eac97 100644 --- a/arch/x86/events/intel/lbr.c +++ b/arch/x86/events/intel/lbr.c @@ -920,7 +920,7 @@ static int branch_type(unsigned long from, unsigned long to, int abort) * on 64-bit systems running 32-bit apps */ #ifdef CONFIG_X86_64 - is64 = kernel_ip((unsigned long)addr) || !test_thread_flag(TIF_IA32); + is64 = kernel_ip((unsigned long)addr) || !TASK_IA32(current); #endif insn_init(, addr, bytes_read, is64); insn_get_opcode(); diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h index d4edf281fff4..d39f9b3ae683 100644 --- a/arch/x86/include/asm/compat.h +++ b/arch/x86/include/asm/compat.h @@ -181,7 +181,7 @@ static inline void __user *arch_compat_alloc_user_space(long len) { compat_uptr_t sp; - if (test_thread_flag(TIF_IA32)) { + if (TASK_IA32(current)) { sp = task_pt_regs(current)->sp; } else { /* -128 for the x32 ABI redzone */ diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index 934aa15b20f2..a3859595847c 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -243,4 +243,6 @@ extern void arch_setup_new_exec(void); #define arch_setup_new_exec arch_setup_new_exec #endif /* !__ASSEMBLY__ */ +#define TASK_IA32(tsk) test_tsk_thread_flag(tsk, TIF_IA32) + #endif /* _ASM_X86_THREAD_INFO_H */ diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c index bb7e1132290b..9b446d3b67d2 100644 --- a/arch/x86/kernel/perf_regs.c +++ b/arch/x86/kernel/perf_regs.c @@ -123,7 +123,7 @@ int perf_reg_validate(u64 mask) u64 perf_reg_abi(struct task_struct *task) { - if (test_tsk_thread_flag(task, TIF_IA32)) + if (TASK_IA32(task)) return PERF_SAMPLE_REGS_ABI_32; else return PERF_SAMPLE_REGS_ABI_64; diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c index a2488b6e27d6..3f1086afa297 100644 --- a/arch/x86/oprofile/backtrace.c +++ b/arch/x86/oprofile/backtrace.c @@ -49,7 +49,7 @@ x86_backtrace_32(struct pt_regs * const regs, unsigned int depth) struct stack_frame_ia32 *head; /* User process is IA32 */ - if (!current || !test_thread_flag(TIF_IA32)) + if (!current || !TASK_IA32(current)) return 0; head = (struct stack_frame_ia32 *) regs->bp; -- 2.27.0
[PATCH 3/6] arch: x86: Wrap TIF_X32 checks
In preparation to remove TIF_X32, add a wrapper that checks the process is using the X32 ABI without using the flag directly. Signed-off-by: Gabriel Krisman Bertazi --- arch/x86/entry/vdso/vma.c | 2 +- arch/x86/include/asm/elf.h | 2 +- arch/x86/include/asm/thread_info.h | 1 + arch/x86/kernel/process_64.c | 3 +-- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c index ea7c1f0b79df..0f54a5feeced 100644 --- a/arch/x86/entry/vdso/vma.c +++ b/arch/x86/entry/vdso/vma.c @@ -417,7 +417,7 @@ int compat_arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp) { #ifdef CONFIG_X86_X32_ABI - if (test_thread_flag(TIF_X32)) { + if (TASK_X32(current)) { if (!vdso64_enabled) return 0; return map_vdso_randomized(_image_x32); diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h index 452beed7892b..a5c8f10d5180 100644 --- a/arch/x86/include/asm/elf.h +++ b/arch/x86/include/asm/elf.h @@ -363,7 +363,7 @@ do { \ #define AT_SYSINFO 32 #define COMPAT_ARCH_DLINFO \ -if (test_thread_flag(TIF_X32)) \ +if (TASK_X32(current)) \ ARCH_DLINFO_X32;\ else \ ARCH_DLINFO_IA32 diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index a3859595847c..6d55a9c0dda2 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -244,5 +244,6 @@ extern void arch_setup_new_exec(void); #endif /* !__ASSEMBLY__ */ #define TASK_IA32(tsk) test_tsk_thread_flag(tsk, TIF_IA32) +#define TASK_X32(tsk) test_tsk_thread_flag(tsk, TIF_X32) #endif /* _ASM_X86_THREAD_INFO_H */ diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index 4452a35402f9..f20a365017b8 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -406,8 +406,7 @@ EXPORT_SYMBOL_GPL(start_thread); void compat_start_thread(struct pt_regs *regs, u32 new_ip, u32 new_sp) { start_thread_common(regs, new_ip, new_sp, - test_thread_flag(TIF_X32) - ? __USER_CS : __USER32_CS, + TASK_X32(current) ? __USER_CS : __USER32_CS, __USER_DS, __USER_DS); } #endif -- 2.27.0
Re: [PATCH 0/2] Small cleanups to ingenic-drm driver
Hi Paul. On Tue, Jul 28, 2020 at 05:16:39PM +0200, Paul Cercueil wrote: > Here are a few cleanups to the ingenic-drm driver. > - some error paths were missing and have been added; > - the mode validation has been moved to the .mode_valid helper callback. > > Cheers, > -Paul > > Paul Cercueil (2): > drm/ingenic: Handle errors of drm_atomic_get_plane_state > drm/ingenic: Validate mode in a .mode_valid callback Both looks fine, you can add my: Reviewed-by: Sam Ravnborg I assume you will apply the patches. Maybe wait for Daniel to take a look, he had some feedback on where to add checks. I assume this is covered by the second patch. Sam > > drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 41 +++ > 1 file changed, 27 insertions(+), 14 deletions(-) > > -- > 2.27.0 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 16/19] perf metric: Make compute_single function more precise
On 7/28/20 10:56 PM, Ian Rogers wrote: > On Tue, Jul 28, 2020 at 5:36 AM Arnaldo Carvalho de Melo > wrote: >> >> Em Sun, Jul 19, 2020 at 08:13:17PM +0200, Jiri Olsa escreveu: >>> So far compute_single function relies on the fact, that >>> there's only single metric defined within evlist in all >>> tests. In following patch we will add test for metric >>> group, so we need to be able to compute metric by given >>> name. >>> >>> Adding the name argument to compute_single and iterating >>> evlist and evsel's expression to find the given metric. >> >> Applied, thanks. >> >> Ian, Kajol, I didn't notice your Acked-by or Reviewed-by, like for the >> other patches, can you check? > > > Acked-by: Ian Rogers Reviewed-By: Kajol Jain Thanks, Kajol Jain > > Thanks, > Ian > >> - Arnaldo >> >>> Signed-off-by: Jiri Olsa >>> --- >>> tools/perf/tests/parse-metric.c | 22 +- >>> 1 file changed, 13 insertions(+), 9 deletions(-) >>> >>> diff --git a/tools/perf/tests/parse-metric.c >>> b/tools/perf/tests/parse-metric.c >>> index 01370ccb9ed9..5ac32f80f8ea 100644 >>> --- a/tools/perf/tests/parse-metric.c >>> +++ b/tools/perf/tests/parse-metric.c >>> @@ -108,17 +108,21 @@ static void load_runtime_stat(struct runtime_stat >>> *st, struct evlist *evlist, >>> } >>> >>> static double compute_single(struct rblist *metric_events, struct evlist >>> *evlist, >>> - struct runtime_stat *st) >>> + struct runtime_stat *st, const char *name) >>> { >>> - struct evsel *evsel = evlist__first(evlist); >>> + struct metric_expr *mexp; >>> struct metric_event *me; >>> + struct evsel *evsel; >>> >>> - me = metricgroup__lookup(metric_events, evsel, false); >>> - if (me != NULL) { >>> - struct metric_expr *mexp; >>> - >>> - mexp = list_first_entry(>head, struct metric_expr, nd); >>> - return test_generic_metric(mexp, 0, st); >>> + evlist__for_each_entry(evlist, evsel) { >>> + me = metricgroup__lookup(metric_events, evsel, false); >>> + if (me != NULL) { >>> + list_for_each_entry (mexp, >head, nd) { >>> + if (strcmp(mexp->metric_name, name)) >>> + continue; >>> + return test_generic_metric(mexp, 0, st); >>> + } >>> + } >>> } >>> return 0.; >>> } >>> @@ -162,7 +166,7 @@ static int compute_metric(const char *name, struct >>> value *vals, double *ratio) >>> load_runtime_stat(, evlist, vals); >>> >>> /* And execute the metric */ >>> - *ratio = compute_single(_events, evlist, ); >>> + *ratio = compute_single(_events, evlist, , name); >>> >>> /* ... clenup. */ >>> metricgroup__rblist_exit(_events); >>> -- >>> 2.25.4 >>> >> >> -- >> >> - Arnaldo
Re: [PATCHv2] coresight: etm4x: Fix etm4_count race by moving cpuhp callbacks to init
Hi Sai, On Tue, 28 Jul 2020 at 08:51, Sai Prakash Ranjan wrote: > > etm4_count keeps track of number of ETMv4 registered and on some systems, > a race is observed on etm4_count variable which can lead to multiple calls > to cpuhp_setup_state_nocalls_cpuslocked(). This function internally calls > cpuhp_store_callbacks() which prevents multiple registrations of callbacks > for a given state and due to this race, it returns -EBUSY leading to ETM > probe failures like below. > > coresight-etm4x: probe of 704.etm failed with error -16 > > This race can easily be triggered with async probe by setting probe type > as PROBE_PREFER_ASYNCHRONOUS and with ETM power management property > "arm,coresight-loses-context-with-cpu". > > Prevent this race by moving cpuhp callbacks to etm driver init since the > cpuhp callbacks doesn't have to depend on the etm4_count and can be once > setup during driver init. Similarly we move cpu_pm notifier registration > to driver init and completely remove etm4_count usage. > > Fixes: 9b6a3f3633a5 ("coresight: etmv4: Fix CPU power management setup in > probe() function") > Fixes: 58eb457be028 ("hwtracing/coresight-etm4x: Convert to hotplug state > machine") > Suggested-by: Suzuki K Poulose > Signed-off-by: Sai Prakash Ranjan > --- > > Changes in v2: > * Rearrange cpuhp callbacks and move them to driver init (Suzuki K Poulose) > > --- > drivers/hwtracing/coresight/coresight-etm4x.c | 51 ++- > 1 file changed, 27 insertions(+), 24 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c > b/drivers/hwtracing/coresight/coresight-etm4x.c > index 6d7d2169bfb2..adb71987a1e3 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c > @@ -48,8 +48,6 @@ module_param(pm_save_enable, int, 0444); > MODULE_PARM_DESC(pm_save_enable, > "Save/restore state on power down: 1 = never, 2 = self-hosted"); > > -/* The number of ETMv4 currently registered */ > -static int etm4_count; > static struct etmv4_drvdata *etmdrvdata[NR_CPUS]; > static void etm4_set_default_config(struct etmv4_config *config); > static int etm4_set_event_filters(struct etmv4_drvdata *drvdata, > @@ -1403,12 +1401,9 @@ static int etm4_pm_setup_cpuslocked(void) > { consider renaming this to etm4_pm_setup() and handing any cpu locking inside the function. In the circumstances - as part of the driver init rather than probe it may be sufficient to call the cpuhp_setup functions without the _cpuslocked suffix and allow the calls to lock the cpus as they are made. i.e. cpuhp_setup_state_nocalls_cpuslocked() => cpuhp_setup_state_nocalls() > int ret; > > - if (etm4_count++) > - return 0; > - > ret = cpu_pm_register_notifier(_cpu_pm_nb); > if (ret) > - goto reduce_count; > + return ret; > > ret = > cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING, >"arm/coresight4:starting", > @@ -1432,17 +1427,11 @@ static int etm4_pm_setup_cpuslocked(void) > > unregister_notifier: > cpu_pm_unregister_notifier(_cpu_pm_nb); > - > -reduce_count: > - --etm4_count; > return ret; > } > > static void etm4_pm_clear(void) > { > - if (--etm4_count != 0) > - return; > - > cpu_pm_unregister_notifier(_cpu_pm_nb); > cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); > if (hp_online) { > @@ -1498,22 +1487,12 @@ static int etm4_probe(struct amba_device *adev, const > struct amba_id *id) > if (!desc.name) > return -ENOMEM; > > - cpus_read_lock(); > etmdrvdata[drvdata->cpu] = drvdata; > > if (smp_call_function_single(drvdata->cpu, > etm4_init_arch_data, drvdata, 1)) > dev_err(dev, "ETM arch init failed\n"); > > - ret = etm4_pm_setup_cpuslocked(); > - cpus_read_unlock(); > - > - /* etm4_pm_setup_cpuslocked() does its own cleanup - exit on error */ > - if (ret) { > - etmdrvdata[drvdata->cpu] = NULL; > - return ret; > - } > - > if (etm4_arch_supported(drvdata->arch) == false) { > ret = -EINVAL; > goto err_arch_supported; > @@ -1560,7 +1539,6 @@ static int etm4_probe(struct amba_device *adev, const > struct amba_id *id) > > err_arch_supported: > etmdrvdata[drvdata->cpu] = NULL; > - etm4_pm_clear(); > return ret; > } > > @@ -1598,4 +1576,29 @@ static struct amba_driver etm4x_driver = { > .probe = etm4_probe, > .id_table = etm4_ids, > }; > -builtin_amba_driver(etm4x_driver); > + > +static int __init etm4x_init(void) > +{ > + int ret; > + > + cpus_read_lock(); > + ret = etm4_pm_setup_cpuslocked(); > + cpus_read_unlock(); See my comment above about rename and use of
Re: [PATCH 03/15] iio: sx9310: Fix irq handling
Quoting Daniel Campello (2020-07-28 13:07:00) > On Tue, Jul 28, 2020 at 12:08 PM Andy Shevchenko > wrote: > > > > On Tue, Jul 28, 2020 at 6:14 PM Daniel Campello > > wrote: > > > > > > Fixes enable/disable irq handling at various points. The driver needs to > > > only enable/disable irqs if there is an actual irq handler installed. > > > > > - enable_irq(data->client->irq); > > > + if (!ret) > > > + enable_irq(data->client->irq); > > > > > > return ret; > > > } > > > > Can it be a usual pattern? > > > > if (ret) > > return ret; > > ... > > return 0; > > I think this way is more readable. The alternative would have to be > something like this: > > > if (ret) > goto out; > mutex_unlock(>mutex); > enable_irq(data->client->irq); > return 0; > > out: > mutex_unlock(>mutex); > return ret; > I think the suggestion is mutex_unlock(>mutex); if (ret) return ret; enable_irq(data->client->irq); return 0;
Re: [PATCH v2 6/8] x86/kaslr: Simplify process_gb_huge_pages
On Tue, Jul 28, 2020 at 03:45:11PM -0400, Arvind Sankar wrote: > But the IS_ENABLED check allows the compiler to eliminate the entire > function at compile time. Ah, I thought it'd be a const false, which would do the same... -- Kees Cook
Re: [PATCH v3 15/19] IMA: Add support for file reads without contents
On Tue, Jul 28, 2020 at 09:56:40PM +0200, Greg Kroah-Hartman wrote: > On Tue, Jul 28, 2020 at 12:44:50PM -0700, Kees Cook wrote: > > On Mon, Jul 27, 2020 at 09:23:34AM -0400, Mimi Zohar wrote: > > > On Fri, 2020-07-24 at 14:36 -0700, Kees Cook wrote: > > > > From: Scott Branden > > > > > > > > When the kernel_read_file LSM hook is called with contents=false, IMA > > > > can appraise the file directly, without requiring a filled buffer. When > > > > such a buffer is available, though, IMA can continue to use it instead > > > > of forcing a double read here. > > > > > > > > Signed-off-by: Scott Branden > > > > Link: > > > > https://lore.kernel.org/lkml/20200706232309.12010-10-scott.bran...@broadcom.com/ > > > > Signed-off-by: Kees Cook > > > > > > After adjusting the comment below. > > > > > > Reviewed-by: Mimi Zohar > > > > Sure! > > > > Greg, shall I send a v4 with added Reviews and the comment change or is > > that minor enough that you're able to do it? > > v4 is needed, as this series is a mess of reviewes and you will have to > redo at least one patch and drop some others, right? Well, I wasn't sure what your desire was, given the weirdness of taking some and reverting others. I will do a v4 based on driver-core-next. Thanks! -- Kees Cook
Re: [PATCH v4 4/5] arm64: dts: sdm845: Add OPP tables and power-domains for venus
On Tue, Jul 28 2020 at 13:51 -0600, Stephen Boyd wrote: Quoting Lina Iyer (2020-07-28 09:52:12) On Mon, Jul 27 2020 at 18:45 -0600, Stephen Boyd wrote: >Quoting Lina Iyer (2020-07-24 09:28:25) >> On Fri, Jul 24 2020 at 03:03 -0600, Rajendra Nayak wrote: >> >Hi Maulik/Lina, >> > >> >On 7/23/2020 11:36 PM, Stanimir Varbanov wrote: >> >>Hi Rajendra, >> >> >> >>After applying 2,3 and 4/5 patches on linaro-integration v5.8-rc2 I see >> >>below messages on db845: >> >> >> >>qcom-venus aa0.video-codec: dev_pm_opp_set_rate: failed to find >> >>current OPP for freq 53397 (-34) >> >> >> >>^^^ This one is new. >> >> >> >>qcom_rpmh TCS Busy, retrying RPMH message send: addr=0x3 >> >> >> >>^^^ and this message is annoying, can we make it pr_debug in rpmh? >> > >> How annoyingly often do you see this message? >> Usually, this is an indication of bad system state either on remote >> processors in the SoC or in Linux itself. On a smooth sailing build you >> should not see this 'warning'. >> >> >Would you be fine with moving this message to a pr_debug? Its currently >> >a pr_info_ratelimited() >> I would rather not, moving this out of sight will mask a lot serious >> issues that otherwise bring attention to the developers. >> > >I removed this warning message in my patch posted to the list[1]. If >it's a serious problem then I suppose a timeout is more appropriate, on >the order of several seconds or so and then a pr_warn() and bail out of >the async call with an error. > The warning used to capture issues that happen within a second and it helps capture system related issues. Timing out after many seconds overlooks the system issues that generally tend to resolve itself, but nevertheless need to be investigated. Is it correct to read "system related issues" as performance problems where the thread is spinning forever trying to send a message and it can't? So the problem is mostly that it's an unbounded amount of time before the message is sent to rpmh and this printk helps identify those situations where that is happening? Yes, but mostly a short period of time like when other processors are in the middle of a restart or resource states changes have taken unusual amounts of time. The system will generally recover from this without crashing in this case. User action is investigation of the situation leading to these messages. Otherwise as you say above it's a bad system state where the rpmh processor has gotten into a bad state like a crash? Can we recover from that? Or is the only recovery a reboot of the system? Does the rpmh processor reboot the system if it crashes? We cannot recover from such a state. The remote processor will reboot if it detects a failure at it's end. If the system entered a bad state, it is possible that RPMH requests start timing out in Linux and remote processor may not detect it. Hence, the timeout in rpmh_write() API. The advised course of action is a restart as there is no way to recover from this state. --Lina
[PATCH v18 0/4] Add JEITA properties and introduce the bq2515x charger
Hello, This patchset adds additional health properties to the power_supply header. These additional properties are taken from the JEITA specification. This patchset also introduces the bq2515x family of charging ICs. Dan Murphy (2): power_supply: Add additional health properties to the header dt-bindings: power: Convert battery.txt to battery.yaml Ricardo Rivera-Matos (2): dt-bindings: power: Add the bindings for the bq2515x family of chargers. power: supply: bq25150 introduce the bq25150 Documentation/ABI/testing/sysfs-class-power |3 +- .../bindings/power/supply/battery.txt | 86 +- .../bindings/power/supply/battery.yaml| 139 ++ .../bindings/power/supply/bq2515x.yaml| 93 ++ drivers/power/supply/Kconfig | 13 + drivers/power/supply/Makefile |1 + drivers/power/supply/bq2515x_charger.c| 1169 + drivers/power/supply/power_supply_sysfs.c |3 + include/linux/power_supply.h |3 + 9 files changed, 1424 insertions(+), 86 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.yaml create mode 100644 Documentation/devicetree/bindings/power/supply/bq2515x.yaml create mode 100644 drivers/power/supply/bq2515x_charger.c -- 2.27.0
[PATCH v18 4/4] power: supply: bq25150 introduce the bq25150
Introduce the bq2515x family of chargers. The BQ2515X family of devices are highly integrated battery management ICs that integrate the most common functions for wearable devices namely a charger, an output voltage rail, ADC for battery and system monitoring, and a push-button controller. Datasheets: bq25150 - http://www.ti.com/lit/ds/symlink/bq25150.pdf bq25155 - http://www.ti.com/lit/ds/symlink/bq25155.pdf Signed-off-by: Ricardo Rivera-Matos --- drivers/power/supply/Kconfig | 13 + drivers/power/supply/Makefile |1 + drivers/power/supply/bq2515x_charger.c | 1169 3 files changed, 1183 insertions(+) create mode 100644 drivers/power/supply/bq2515x_charger.c diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index 44d3c8512fb8..faf2830aa152 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -610,6 +610,19 @@ config CHARGER_BQ24735 help Say Y to enable support for the TI BQ24735 battery charger. +config CHARGER_BQ2515X + tristate "TI BQ2515X battery charger family" + depends on I2C + depends on GPIOLIB || COMPILE_TEST + select REGMAP_I2C + help + Say Y to enable support for the TI BQ2515X family of battery + charging integrated circuits. The BQ2515X are highly integrated + battery charge management ICs that integrate the most common + functions for wearable devices, namely a charger, an output voltage + rail, ADC for battery and system monitoring, and push-button + controller. + config CHARGER_BQ25890 tristate "TI BQ25890 battery charger driver" depends on I2C diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile index b9644663e435..b3c694a65114 100644 --- a/drivers/power/supply/Makefile +++ b/drivers/power/supply/Makefile @@ -82,6 +82,7 @@ obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o obj-$(CONFIG_CHARGER_BQ24257) += bq24257_charger.o obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o +obj-$(CONFIG_CHARGER_BQ2515X) += bq2515x_charger.o obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o diff --git a/drivers/power/supply/bq2515x_charger.c b/drivers/power/supply/bq2515x_charger.c new file mode 100644 index ..63e8e317af56 --- /dev/null +++ b/drivers/power/supply/bq2515x_charger.c @@ -0,0 +1,1169 @@ +// SPDX-License-Identifier: GPL-2.0 +// BQ2515X Battery Charger Driver +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define BQ2515X_MANUFACTURER "Texas Instruments" + +#define BQ2515X_STAT0 0x00 +#define BQ2515X_STAT1 0x01 +#define BQ2515X_STAT2 0x02 +#define BQ2515X_FLAG0 0x03 +#define BQ2515X_FLAG1 0x04 +#define BQ2515X_FLAG2 0x05 +#define BQ2515X_FLAG3 0x06 +#define BQ2515X_MASK0 0x07 +#define BQ2515X_MASK1 0x08 +#define BQ2515X_MASK2 0x09 +#define BQ2515X_MASK3 0x0a +#define BQ2515X_VBAT_CTRL 0x12 +#define BQ2515X_ICHG_CTRL 0x13 +#define BQ2515X_PCHRGCTRL 0x14 +#define BQ2515X_TERMCTRL 0x15 +#define BQ2515X_BUVLO 0x16 +#define BQ2515X_CHARGERCTRL0 0x17 +#define BQ2515X_CHARGERCTRL1 0x18 +#define BQ2515X_ILIMCTRL 0x19 +#define BQ2515X_LDOCTRL0x1d +#define BQ2515X_MRCTRL 0x30 +#define BQ2515X_ICCTRL00x35 +#define BQ2515X_ICCTRL10x36 +#define BQ2515X_ICCTRL20x37 +#define BQ2515X_ADCCTRL0 0x40 +#define BQ2515X_ADCCTRL1 0x41 +#define BQ2515X_ADC_VBAT_M 0x42 +#define BQ2515X_ADC_VBAT_L 0x43 +#define BQ2515X_ADC_TS_M 0x44 +#define BQ2515X_ADC_TS_L 0x45 +#define BQ2515X_ADC_ICHG_M 0x46 +#define BQ2515X_ADC_ICHG_L 0x47 +#define BQ2515X_ADC_ADCIN_M0x48 +#define BQ2515X_ADC_ADCIN_L0x49 +#define BQ2515X_ADC_VIN_M 0x4a +#define BQ2515X_ADC_VIN_L 0x4b +#define BQ2515X_ADC_PMID_M 0x4c +#define BQ2515X_ADC_PMID_L 0x4d +#define BQ2515X_ADC_IIN_M 0x4e +#define BQ2515X_ADC_IIN_L 0x4f +#define BQ2515X_ADC_COMP1_M0x52 +#define BQ2515X_ADC_COMP1_L0X53 +#define BQ2515X_ADC_COMP2_M0X54 +#define BQ2515X_ADC_COMP2_L0x55 +#define BQ2515X_ADC_COMP3_M0x56 +#define BQ2515X_ADC_COMP3_L0x57 +#define BQ2515X_ADC_READ_EN0x58 +#define BQ2515X_TS_FASTCHGCTRL 0x61 +#define BQ2515X_TS_COLD0x62 +#define BQ2515X_TS_COOL0x63 +#define BQ2515X_TS_WARM0x64 +#define BQ2515X_TS_HOT 0x65 +#define BQ2515X_DEVICE_ID 0x6f + +#define BQ2515X_DEFAULT_ICHG_UA1 +#define BQ25150_DEFAULT_ILIM_UA
[PATCH v18 2/4] dt-bindings: power: Convert battery.txt to battery.yaml
From: Dan Murphy Convert the battery.txt file to yaml and fix up the examples. Signed-off-by: Dan Murphy Reviewed-by: Rob Herring --- .../bindings/power/supply/battery.txt | 86 +-- .../bindings/power/supply/battery.yaml| 139 ++ 2 files changed, 140 insertions(+), 85 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.yaml diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt index 5e29595edd74..a9f80cc49068 100644 --- a/Documentation/devicetree/bindings/power/supply/battery.txt +++ b/Documentation/devicetree/bindings/power/supply/battery.txt @@ -1,87 +1,3 @@ -Battery Characteristics - -The devicetree battery node provides static battery characteristics. -In smart batteries, these are typically stored in non-volatile memory -on a fuel gauge chip. The battery node should be used where there is -no appropriate non-volatile memory, or it is unprogrammed/incorrect. - -Upstream dts files should not include battery nodes, unless the battery -represented cannot easily be replaced in the system by one of a -different type. This prevents unpredictable, potentially harmful, -behavior should a replacement that changes the battery type occur -without a corresponding update to the dtb. +The contents of this file has been moved to battery.yaml Please note that not all charger drivers respect all of the properties. - -Required Properties: - - compatible: Must be "simple-battery" - -Optional Properties: - - over-voltage-threshold-microvolt: battery over-voltage limit - - re-charge-voltage-microvolt: limit to automatically start charging again - - voltage-min-design-microvolt: drained battery voltage - - voltage-max-design-microvolt: fully charged battery voltage - - energy-full-design-microwatt-hours: battery design energy - - charge-full-design-microamp-hours: battery design capacity - - trickle-charge-current-microamp: current for trickle-charge phase - - precharge-current-microamp: current for pre-charge phase - - precharge-upper-limit-microvolt: limit when to change to constant charging - - charge-term-current-microamp: current for charge termination phase - - constant-charge-current-max-microamp: maximum constant input current - - constant-charge-voltage-max-microvolt: maximum constant input voltage - - factory-internal-resistance-micro-ohms: battery factory internal resistance - - ocv-capacity-table-0: An array providing the open circuit voltage (OCV) - of the battery and corresponding battery capacity percent, which is used - to look up battery capacity according to current OCV value. And the open - circuit voltage unit is microvolt. - - ocv-capacity-table-1: Same as ocv-capacity-table-0 - .. - - ocv-capacity-table-n: Same as ocv-capacity-table-0 - - ocv-capacity-celsius: An array containing the temperature in degree Celsius, - for each of the battery capacity lookup table. The first temperature value - specifies the OCV table 0, and the second temperature value specifies the - OCV table 1, and so on. - - resistance-temp-table: An array providing the temperature in degree Celsius - and corresponding battery internal resistance percent, which is used to look - up the resistance percent according to current temperature to get a accurate - batterty internal resistance in different temperatures. - -Battery properties are named, where possible, for the corresponding -elements in enum power_supply_property, defined in -https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h - -Batteries must be referenced by chargers and/or fuel-gauges -using a phandle. The phandle's property should be named -"monitored-battery". - -Example: - - bat: battery { - compatible = "simple-battery"; - voltage-min-design-microvolt = <320>; - voltage-max-design-microvolt = <420>; - energy-full-design-microwatt-hours = <529>; - charge-full-design-microamp-hours = <143>; - precharge-current-microamp = <256000>; - charge-term-current-microamp = <128000>; - constant-charge-current-max-microamp = <90>; - constant-charge-voltage-max-microvolt = <420>; - factory-internal-resistance-micro-ohms = <25>; - ocv-capacity-celsius = <(-10) 0 10>; - ocv-capacity-table-0 = <4185000 100>, <4113000 95>, <4066000 90>, ...; - ocv-capacity-table-1 = <420 100>, <4185000 95>, <4113000 90>, ...; - ocv-capacity-table-2 = <425 100>, <420 95>, <4185000 90>, ...; - resistance-temp-table = <20 100>, <10 90>, <0 80>, <(-10) 60>; - }; - - charger: charger@11 { - - monitored-battery = <>; - ... - }; - - fuel_gauge:
[PATCH v18 3/4] dt-bindings: power: Add the bindings for the bq2515x family of chargers.
The BQ2515X family of devices are highly integrated battery management ICs that integrate the most common functions for wearable devices namely a charger, an output voltage rail, ADC for battery and system monitoring, and a push-button controller. Datasheets: http://www.ti.com/lit/ds/symlink/bq25150.pdf http://www.ti.com/lit/ds/symlink/bq25155.pdf Reviewed-by: Rob Herring Signed-off-by: Ricardo Rivera-Matos --- .../bindings/power/supply/bq2515x.yaml| 93 +++ 1 file changed, 93 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/bq2515x.yaml diff --git a/Documentation/devicetree/bindings/power/supply/bq2515x.yaml b/Documentation/devicetree/bindings/power/supply/bq2515x.yaml new file mode 100644 index ..75a56773be4a --- /dev/null +++ b/Documentation/devicetree/bindings/power/supply/bq2515x.yaml @@ -0,0 +1,93 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright (C) 2020 Texas Instruments Incorporated +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/power/supply/bq2515x.yaml#; +$schema: "http://devicetree.org/meta-schemas/core.yaml#; + +title: TI bq2515x 500-mA Linear charger family + +maintainers: + - Dan Murphy + - Ricardo Rivera-Matos + +description: | + The BQ2515x family is a highly integrated battery charge management IC that + integrates the most common functions for wearable devices, namely a charger, + an output voltage rail, ADC for battery and system monitoring, and + push-button controller. + + Specifications about the charger can be found at: +http://www.ti.com/lit/ds/symlink/bq25150.pdf +http://www.ti.com/lit/ds/symlink/bq25155.pdf + +properties: + compatible: +enum: + - ti,bq25150 + - ti,bq25155 + + reg: +maxItems: 1 +description: I2C address of the charger. + + ac-detect-gpios: +description: | + GPIO used for connecting the bq2515x device PG (AC Detect) + pin. +maxItems: 1 + + reset-gpios: +description: GPIO used for hardware reset. +maxItems: 1 + + powerdown-gpios: +description: GPIO used for low power mode of IC. +maxItems: 1 + + charge-enable-gpios: +description: GPIO used to turn on and off charging. +maxItems: 1 + + input-current-limit-microamp: +$ref: /schemas/types.yaml#/definitions/uint32 +description: Maximum input current in micro Amps. +minimum: 5 +maximum: 50 + + monitored-battery: +$ref: /schemas/types.yaml#/definitions/phandle +description: phandle to the battery node being monitored + +required: + - compatible + - reg + - monitored-battery + +additionalProperties: false + +examples: + - | +bat: battery { + compatible = "simple-battery"; + constant-charge-current-max-microamp = <5>; + precharge-current-microamp = <2500>; + constant-charge-voltage-max-microvolt = <400>; +}; +#include +i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + bq25150: charger@6b { +compatible = "ti,bq25150"; +reg = <0x6b>; +monitored-battery = <>; +input-current-limit-microamp = <10>; + +ac-detect-gpios = < 28 GPIO_ACTIVE_HIGH>; +reset-gpios = < 14 GPIO_ACTIVE_HIGH>; +powerdown-gpios = < 15 GPIO_ACTIVE_HIGH>; +charge-enable-gpios = < 13 GPIO_ACTIVE_LOW>; + }; +}; -- 2.27.0
[PATCH v18 1/4] power_supply: Add additional health properties to the header
From: Dan Murphy Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum. HEALTH_WARM, HEALTH_COOL, and HEALTH_HOT properties are taken from JEITA specification JISC8712:2015 Acked-by: Andrew F. Davis Tested-by: Guru Das Srinagesh Signed-off-by: Dan Murphy --- Documentation/ABI/testing/sysfs-class-power | 3 ++- drivers/power/supply/power_supply_sysfs.c | 3 +++ include/linux/power_supply.h| 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power index 216d61a22f1e..40213c73bc9c 100644 --- a/Documentation/ABI/testing/sysfs-class-power +++ b/Documentation/ABI/testing/sysfs-class-power @@ -205,7 +205,8 @@ Description: Valid values: "Unknown", "Good", "Overheat", "Dead", "Over voltage", "Unspecified failure", "Cold", "Watchdog timer expire", "Safety timer expire", - "Over current", "Calibration required" + "Over current", "Calibration required", "Warm", + "Cool", "Hot" What: /sys/class/power_supply//precharge_current Date: June 2017 diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c index bc79560229b5..4d6e1d5015d6 100644 --- a/drivers/power/supply/power_supply_sysfs.c +++ b/drivers/power/supply/power_supply_sysfs.c @@ -101,6 +101,9 @@ static const char * const POWER_SUPPLY_HEALTH_TEXT[] = { [POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE] = "Safety timer expire", [POWER_SUPPLY_HEALTH_OVERCURRENT] = "Over current", [POWER_SUPPLY_HEALTH_CALIBRATION_REQUIRED] = "Calibration required", + [POWER_SUPPLY_HEALTH_WARM] = "Warm", + [POWER_SUPPLY_HEALTH_COOL] = "Cool", + [POWER_SUPPLY_HEALTH_HOT] = "Hot", }; static const char * const POWER_SUPPLY_TECHNOLOGY_TEXT[] = { diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index ac1345a48ad0..b5ee35d3c304 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -62,6 +62,9 @@ enum { POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE, POWER_SUPPLY_HEALTH_OVERCURRENT, POWER_SUPPLY_HEALTH_CALIBRATION_REQUIRED, + POWER_SUPPLY_HEALTH_WARM, + POWER_SUPPLY_HEALTH_COOL, + POWER_SUPPLY_HEALTH_HOT, }; enum { -- 2.27.0
[PATCH 4/6] PCI: vmd: Create IRQ allocation helper
Moves the IRQ allocation and SRCU initialization code to a new helper. No functional changes. Reviewed-by: Andy Shevchenko Signed-off-by: Jon Derrick --- drivers/pci/controller/vmd.c | 94 1 file changed, 53 insertions(+), 41 deletions(-) diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index 703c48171993..3214d785fa5d 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -528,6 +528,55 @@ static int vmd_get_bus_number_start(struct vmd_dev *vmd) return 0; } +static irqreturn_t vmd_irq(int irq, void *data) +{ + struct vmd_irq_list *irqs = data; + struct vmd_irq *vmdirq; + int idx; + + idx = srcu_read_lock(>srcu); + list_for_each_entry_rcu(vmdirq, >irq_list, node) + generic_handle_irq(vmdirq->virq); + srcu_read_unlock(>srcu, idx); + + return IRQ_HANDLED; +} + +static int vmd_alloc_irqs(struct vmd_dev *vmd) +{ + struct pci_dev *dev = vmd->dev; + int i, err; + + vmd->msix_count = pci_msix_vec_count(dev); + if (vmd->msix_count < 0) + return -ENODEV; + + vmd->msix_count = pci_alloc_irq_vectors(dev, 1, vmd->msix_count, + PCI_IRQ_MSIX); + if (vmd->msix_count < 0) + return vmd->msix_count; + + vmd->irqs = devm_kcalloc(>dev, vmd->msix_count, sizeof(*vmd->irqs), +GFP_KERNEL); + if (!vmd->irqs) + return -ENOMEM; + + for (i = 0; i < vmd->msix_count; i++) { + err = init_srcu_struct(>irqs[i].srcu); + if (err) + return err; + + INIT_LIST_HEAD(>irqs[i].irq_list); + err = devm_request_irq(>dev, pci_irq_vector(dev, i), + vmd_irq, IRQF_NO_THREAD, + "vmd", >irqs[i]); + if (err) + return err; + } + + return 0; +} + static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) { struct pci_sysdata *sd = >sysdata; @@ -663,24 +712,10 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) return 0; } -static irqreturn_t vmd_irq(int irq, void *data) -{ - struct vmd_irq_list *irqs = data; - struct vmd_irq *vmdirq; - int idx; - - idx = srcu_read_lock(>srcu); - list_for_each_entry_rcu(vmdirq, >irq_list, node) - generic_handle_irq(vmdirq->virq); - srcu_read_unlock(>srcu, idx); - - return IRQ_HANDLED; -} - static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) { struct vmd_dev *vmd; - int i, err; + int err; if (resource_size(>resource[VMD_CFGBAR]) < (1 << 20)) return -ENOMEM; @@ -703,32 +738,9 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32))) return -ENODEV; - vmd->msix_count = pci_msix_vec_count(dev); - if (vmd->msix_count < 0) - return -ENODEV; - - vmd->msix_count = pci_alloc_irq_vectors(dev, 1, vmd->msix_count, - PCI_IRQ_MSIX); - if (vmd->msix_count < 0) - return vmd->msix_count; - - vmd->irqs = devm_kcalloc(>dev, vmd->msix_count, sizeof(*vmd->irqs), -GFP_KERNEL); - if (!vmd->irqs) - return -ENOMEM; - - for (i = 0; i < vmd->msix_count; i++) { - err = init_srcu_struct(>irqs[i].srcu); - if (err) - return err; - - INIT_LIST_HEAD(>irqs[i].irq_list); - err = devm_request_irq(>dev, pci_irq_vector(dev, i), - vmd_irq, IRQF_NO_THREAD, - "vmd", >irqs[i]); - if (err) - return err; - } + err = vmd_alloc_irqs(vmd); + if (err) + return err; spin_lock_init(>cfg_lock); pci_set_drvdata(dev, vmd); -- 2.27.0
[PATCH 3/6] PCI: vmd: Create IRQ Domain configuration helper
Moves the IRQ and MSI Domain configuration code to new helpers. No functional changes. Reviewed-by: Andy Shevchenko Signed-off-by: Jon Derrick --- drivers/pci/controller/vmd.c | 52 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index a462719af12a..703c48171993 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -298,6 +298,34 @@ static struct msi_domain_info vmd_msi_domain_info = { .chip = _msi_controller, }; +static int vmd_create_irq_domain(struct vmd_dev *vmd) +{ + struct fwnode_handle *fn; + + fn = irq_domain_alloc_named_id_fwnode("VMD-MSI", vmd->sysdata.domain); + if (!fn) + return -ENODEV; + + vmd->irq_domain = pci_msi_create_irq_domain(fn, _msi_domain_info, + x86_vector_domain); + if (!vmd->irq_domain) { + irq_domain_free_fwnode(fn); + return -ENODEV; + } + + return 0; +} + +static void vmd_remove_irq_domain(struct vmd_dev *vmd) +{ + if (vmd->irq_domain) { + struct fwnode_handle *fn = vmd->irq_domain->fwnode; + + irq_domain_remove(vmd->irq_domain); + irq_domain_free_fwnode(fn); + } +} + static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus, unsigned int devfn, int reg, int len) { @@ -503,7 +531,6 @@ static int vmd_get_bus_number_start(struct vmd_dev *vmd) static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) { struct pci_sysdata *sd = >sysdata; - struct fwnode_handle *fn; struct resource *res; u32 upper_bits; unsigned long flags; @@ -598,16 +625,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) sd->node = pcibus_to_node(vmd->dev->bus); - fn = irq_domain_alloc_named_id_fwnode("VMD-MSI", vmd->sysdata.domain); - if (!fn) - return -ENODEV; - - vmd->irq_domain = pci_msi_create_irq_domain(fn, _msi_domain_info, - x86_vector_domain); - if (!vmd->irq_domain) { - irq_domain_free_fwnode(fn); - return -ENODEV; - } + ret = vmd_create_irq_domain(vmd); + if (ret) + return ret; pci_add_resource(, >resources[0]); pci_add_resource_offset(, >resources[1], offset[0]); @@ -617,13 +637,13 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) _ops, sd, ); if (!vmd->bus) { pci_free_resource_list(); - irq_domain_remove(vmd->irq_domain); - irq_domain_free_fwnode(fn); + vmd_remove_irq_domain(vmd); return -ENODEV; } vmd_attach_resources(vmd); - dev_set_msi_domain(>bus->dev, vmd->irq_domain); + if (vmd->irq_domain) + dev_set_msi_domain(>bus->dev, vmd->irq_domain); pci_scan_child_bus(vmd->bus); pci_assign_unassigned_bus_resources(vmd->bus); @@ -732,15 +752,13 @@ static void vmd_cleanup_srcu(struct vmd_dev *vmd) static void vmd_remove(struct pci_dev *dev) { struct vmd_dev *vmd = pci_get_drvdata(dev); - struct fwnode_handle *fn = vmd->irq_domain->fwnode; sysfs_remove_link(>dev->dev.kobj, "domain"); pci_stop_root_bus(vmd->bus); pci_remove_root_bus(vmd->bus); vmd_cleanup_srcu(vmd); vmd_detach_resources(vmd); - irq_domain_remove(vmd->irq_domain); - irq_domain_free_fwnode(fn); + vmd_remove_irq_domain(vmd); } #ifdef CONFIG_PM_SLEEP -- 2.27.0
Re: [PATCH 03/15] iio: sx9310: Fix irq handling
On Tue, Jul 28, 2020 at 12:08 PM Andy Shevchenko wrote: > > On Tue, Jul 28, 2020 at 6:14 PM Daniel Campello wrote: > > > > Fixes enable/disable irq handling at various points. The driver needs to > > only enable/disable irqs if there is an actual irq handler installed. > > > - enable_irq(data->client->irq); > > + if (!ret) > > + enable_irq(data->client->irq); > > > > return ret; > > } > > Can it be a usual pattern? > > if (ret) > return ret; > ... > return 0; I think this way is more readable. The alternative would have to be something like this: if (ret) goto out; mutex_unlock(>mutex); enable_irq(data->client->irq); return 0; out: mutex_unlock(>mutex); return ret; > > -- > With Best Regards, > Andy Shevchenko Regards, Daniel Campello
Re: [PATCH v2 8/8] x86/kaslr: Don't use 64-bit mem_vector for 32-bit kernel
On Tue, Jul 28, 2020 at 12:37:12PM -0700, Kees Cook wrote: > On Mon, Jul 27, 2020 at 07:08:01PM -0400, Arvind Sankar wrote: > > Commit > > f28442497b5c ("x86/boot: Fix KASLR and memmap= collision") > > converted mem_vector type to use 64-bit on the 32-bit kernel as well, > > based on Thomas's review [0]. However: > > - the code still doesn't consistently use 64-bit types. For instance, > > mem_avoid_overlap uses 32-bit types when checking for overlaps. This > > is actually okay, as the passed in memory range will have been clipped > > to below 4G, but it is difficult to reason about the safety of the > > code. > > - KASLR on 32-bit can't use memory above 4G anyway, so it's pointless > > to keep track of ranges above 4G. > > > > Switch the type back to unsigned long, and use a helper function to clip > > regions to 4G on 32-bit, when storing mem_avoid, immovable_mem, EFI, > > E820 and command-line memory regions. > > The reason for long long is to avoid having to check for overflows in > any of the offset calculations. Why not just leave this as-is? > The first bullet: there are still unsigned long's in the code that get assigned mem_vector.start etc. Taking into account Ingo's review as well, I'm planning to just make all those u64 as well in v3, instead of this patch.
[PATCH 6/6] PCI: vmd: Disable MSI/X remapping when possible
VMD will retransmit child device MSI/X using its own MSI/X table and requester-id. This limits the number of MSI/X available to the whole child device domain to the number of VMD MSI/X interrupts. Some VMD devices have a mode where this remapping can be disabled, allowing child device interrupts to bypass processing with the VMD MSI/X domain interrupt handler and going straight the child device interrupt handler, allowing for better performance and scaling. The requester-id still gets changed to the VMD endpoint's requester-id, and the interrupt remapping handlers have been updated to properly set IRTE for child device interrupts to the VMD endpoint's context. Some VMD platforms have existing production BIOS which rely on MSI/X remapping and won't explicitly program the MSI/X remapping bit. This re-enables MSI/X remapping on unload. Disabling MSI/X remapping is only available for Icelake Server and client VMD products. Reviewed-by: Andy Shevchenko Signed-off-by: Jon Derrick --- drivers/pci/controller/vmd.c | 58 +++- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index 3214d785fa5d..e8cde2c390b9 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -53,6 +53,12 @@ enum vmd_features { * vendor-specific capability space */ VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP= (1 << 2), + + /* +* Device remaps MSI/X transactions into its MSI/X table and requires +* VMD MSI domain for child device interrupt handling +*/ + VMD_FEAT_REMAPS_MSI = (1 << 3), }; /* @@ -298,6 +304,15 @@ static struct msi_domain_info vmd_msi_domain_info = { .chip = _msi_controller, }; +static void vmd_enable_msi_remapping(struct vmd_dev *vmd, bool enable) +{ + u16 reg; + + pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, ); + reg = enable ? (reg & ~0x2) : (reg | 0x2); + pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg); +} + static int vmd_create_irq_domain(struct vmd_dev *vmd) { struct fwnode_handle *fn; @@ -318,6 +333,13 @@ static int vmd_create_irq_domain(struct vmd_dev *vmd) static void vmd_remove_irq_domain(struct vmd_dev *vmd) { + /* +* Some production BIOS won't enable remapping between soft reboots. +* Ensure remapping is restored before unloading the driver +*/ + if (!vmd->msix_count) + vmd_enable_msi_remapping(vmd, true); + if (vmd->irq_domain) { struct fwnode_handle *fn = vmd->irq_domain->fwnode; @@ -606,6 +628,27 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) return ret; } + /* +* Currently MSI remapping must be enabled in guest passthrough mode +* due to some missing interrupt remapping plumbing. This is probably +* acceptable because the guest is usually CPU-limited and MSI +* remapping doesn't become a performance bottleneck. +*/ + if (features & VMD_FEAT_REMAPS_MSI || offset[0] || offset[1]) { + ret = vmd_alloc_irqs(vmd); + if (ret) + return ret; + } + + /* +* Disable remapping for performance if possible based on if VMD IRQs +* had been allocated. +*/ + if (vmd->msix_count) + vmd_enable_msi_remapping(vmd, true); + else + vmd_enable_msi_remapping(vmd, false); + /* * Certain VMD devices may have a root port configuration option which * limits the bus range to between 0-127, 128-255, or 224-255 @@ -674,9 +717,11 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) sd->node = pcibus_to_node(vmd->dev->bus); - ret = vmd_create_irq_domain(vmd); - if (ret) - return ret; + if (vmd->msix_count) { + ret = vmd_create_irq_domain(vmd); + if (ret) + return ret; + } pci_add_resource(, >resources[0]); pci_add_resource_offset(, >resources[1], offset[0]); @@ -738,10 +783,6 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32))) return -ENODEV; - err = vmd_alloc_irqs(vmd); - if (err) - return err; - spin_lock_init(>cfg_lock); pci_set_drvdata(dev, vmd); err = vmd_enable_domain(vmd, (unsigned long) id->driver_data); @@ -809,7 +850,8 @@ static SIMPLE_DEV_PM_OPS(vmd_dev_pm_ops, vmd_suspend, vmd_resume); static const struct pci_device_id vmd_ids[] = { {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_201D), - .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,}, + .driver_data =
[PATCH 2/6] PCI: vmd: Create bus offset configuration helper
Moves the bus offset configuration discovery code to a new helper. Modifies the bus offset 2-bit decode switch to have a 0 case and a default error case, just in case the field is expanded in future hardware. Reviewed-by: Andy Shevchenko Signed-off-by: Jon Derrick --- drivers/pci/controller/vmd.c | 53 ++-- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index 44b2db789eff..a462719af12a 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -471,6 +471,35 @@ static int vmd_get_phys_offsets(struct vmd_dev *vmd, bool native_hint, return 0; } +static int vmd_get_bus_number_start(struct vmd_dev *vmd) +{ + struct pci_dev *dev = vmd->dev; + u16 reg; + + pci_read_config_word(dev, PCI_REG_VMCAP, ); + if (BUS_RESTRICT_CAP(reg)) { + pci_read_config_word(dev, PCI_REG_VMCONFIG, ); + + switch (BUS_RESTRICT_CFG(reg)) { + case 0: + vmd->busn_start = 0; + break; + case 1: + vmd->busn_start = 128; + break; + case 2: + vmd->busn_start = 224; + break; + default: + pci_err(dev, "Unknown Bus Offset Setting (%d)\n", + BUS_RESTRICT_CFG(reg)); + return -ENODEV; + } + } + + return 0; +} + static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) { struct pci_sysdata *sd = >sysdata; @@ -506,27 +535,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) * limits the bus range to between 0-127, 128-255, or 224-255 */ if (features & VMD_FEAT_HAS_BUS_RESTRICTIONS) { - u16 reg16; - - pci_read_config_word(vmd->dev, PCI_REG_VMCAP, ); - if (BUS_RESTRICT_CAP(reg16)) { - pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, -); - - switch (BUS_RESTRICT_CFG(reg16)) { - case 1: - vmd->busn_start = 128; - break; - case 2: - vmd->busn_start = 224; - break; - case 3: - pci_err(vmd->dev, "Unknown Bus Offset Setting\n"); - return -ENODEV; - default: - break; - } - } + ret = vmd_get_bus_number_start(vmd); + if (ret) + return ret; } res = >dev->resource[VMD_CFGBAR]; -- 2.27.0
[PATCH 0/6] VMD MSI Remapping Bypass
The Intel Volume Management Device acts similar to a PCI-to-PCI bridge in that it changes downstream devices' requester-ids to its own. As VMD supports PCIe devices, it has its own MSI/X table and transmits child device MSI/X by remapping child device MSI/X and handling like a demultiplexer. Some newer VMD devices (Icelake Server and client) have an option to bypass the VMD MSI/X remapping table. This allows for better performance scaling as the child device MSI/X won't be limited by VMD's MSI/X count and IRQ handler. It's expected that most users don't want MSI/X remapping when they can get better performance without this limitation. This set includes some long overdue cleanup of overgrown VMD code and introduces the MSI/X remapping disable. Applies on top of e3beca48a45b ("irqdomain/treewide: Keep firmware node unconditionally allocated") and ec0160891e38 ("irqdomain/treewide: Free firmware node after domain removal") in tip/urgent Jon Derrick (6): PCI: vmd: Create physical offset helper PCI: vmd: Create bus offset configuration helper PCI: vmd: Create IRQ Domain configuration helper PCI: vmd: Create IRQ allocation helper x86/apic/msi: Use Real PCI DMA device when configuring IRTE PCI: vmd: Disable MSI/X remapping when possible arch/x86/kernel/apic/msi.c | 2 +- drivers/pci/controller/vmd.c | 344 +++ 2 files changed, 224 insertions(+), 122 deletions(-) base-commit: ec0160891e387f4771f953b888b1fe951398e5d9 -- 2.27.0
[PATCH 5/6] x86/apic/msi: Use Real PCI DMA device when configuring IRTE
VMD retransmits child device MSI/X with the VMD endpoint's requester-id. In order to support direct interrupt remapping of VMD child devices, ensure that the IRTE is programmed with the VMD endpoint's requester-id using pci_real_dma_dev(). Reviewed-by: Andy Shevchenko Signed-off-by: Jon Derrick --- arch/x86/kernel/apic/msi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c index c2b2911feeef..7ca271b8d891 100644 --- a/arch/x86/kernel/apic/msi.c +++ b/arch/x86/kernel/apic/msi.c @@ -189,7 +189,7 @@ int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) init_irq_alloc_info(, NULL); info.type = X86_IRQ_ALLOC_TYPE_MSI; - info.msi_dev = dev; + info.msi_dev = pci_real_dma_dev(dev); domain = irq_remapping_get_irq_domain(); if (domain == NULL) -- 2.27.0
[PATCH 1/6] PCI: vmd: Create physical offset helper
Moves the guest-passthrough physical offset discovery code to a new helper. No functional changes. Reviewed-by: Andy Shevchenko Signed-off-by: Jon Derrick --- drivers/pci/controller/vmd.c | 105 +-- 1 file changed, 62 insertions(+), 43 deletions(-) diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index f69ef8c89f72..44b2db789eff 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -417,6 +417,60 @@ static int vmd_find_free_domain(void) return domain + 1; } +static int vmd_get_phys_offsets(struct vmd_dev *vmd, bool native_hint, + resource_size_t *offset1, + resource_size_t *offset2) +{ + struct pci_dev *dev = vmd->dev; + u64 phys1, phys2; + + if (native_hint) { + u32 vmlock; + int ret; + + ret = pci_read_config_dword(dev, PCI_REG_VMLOCK, ); + if (ret || vmlock == ~0) + return -ENODEV; + + if (MB2_SHADOW_EN(vmlock)) { + void __iomem *membar2; + + membar2 = pci_iomap(dev, VMD_MEMBAR2, 0); + if (!membar2) + return -ENOMEM; + phys1 = readq(membar2 + MB2_SHADOW_OFFSET); + phys2 = readq(membar2 + MB2_SHADOW_OFFSET + 8); + pci_iounmap(dev, membar2); + } else + return 0; + } else { + /* Hypervisor-Emulated Vendor-Specific Capability */ + int pos = pci_find_capability(dev, PCI_CAP_ID_VNDR); + u32 reg, regu; + + pci_read_config_dword(dev, pos + 4, ); + + /* "SHDW" */ + if (pos && reg == 0x53484457) { + pci_read_config_dword(dev, pos + 8, ); + pci_read_config_dword(dev, pos + 12, ); + phys1 = (u64) regu << 32 | reg; + + pci_read_config_dword(dev, pos + 16, ); + pci_read_config_dword(dev, pos + 20, ); + phys2 = (u64) regu << 32 | reg; + } else + return 0; + } + + *offset1 = dev->resource[VMD_MEMBAR1].start - + (phys1 & PCI_BASE_ADDRESS_MEM_MASK); + *offset2 = dev->resource[VMD_MEMBAR2].start - + (phys2 & PCI_BASE_ADDRESS_MEM_MASK); + + return 0; +} + static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) { struct pci_sysdata *sd = >sysdata; @@ -428,6 +482,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) resource_size_t offset[2] = {0}; resource_size_t membar2_offset = 0x2000; struct pci_bus *child; + int ret; /* * Shadow registers may exist in certain VMD device ids which allow @@ -436,50 +491,14 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) * or 0, depending on an enable bit in the VMD device. */ if (features & VMD_FEAT_HAS_MEMBAR_SHADOW) { - u32 vmlock; - int ret; - membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE; - ret = pci_read_config_dword(vmd->dev, PCI_REG_VMLOCK, ); - if (ret || vmlock == ~0) - return -ENODEV; - - if (MB2_SHADOW_EN(vmlock)) { - void __iomem *membar2; - - membar2 = pci_iomap(vmd->dev, VMD_MEMBAR2, 0); - if (!membar2) - return -ENOMEM; - offset[0] = vmd->dev->resource[VMD_MEMBAR1].start - - (readq(membar2 + MB2_SHADOW_OFFSET) & -PCI_BASE_ADDRESS_MEM_MASK); - offset[1] = vmd->dev->resource[VMD_MEMBAR2].start - - (readq(membar2 + MB2_SHADOW_OFFSET + 8) & -PCI_BASE_ADDRESS_MEM_MASK); - pci_iounmap(vmd->dev, membar2); - } - } - - if (features & VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP) { - int pos = pci_find_capability(vmd->dev, PCI_CAP_ID_VNDR); - u32 reg, regu; - - pci_read_config_dword(vmd->dev, pos + 4, ); - - /* "SHDW" */ - if (pos && reg == 0x53484457) { - pci_read_config_dword(vmd->dev, pos + 8, ); - pci_read_config_dword(vmd->dev, pos + 12, ); - offset[0] = vmd->dev->resource[VMD_MEMBAR1].start - - (((u64) regu << 32 | reg) & -PCI_BASE_ADDRESS_MEM_MASK); - -
Re: [PATCH net-next RFC 01/13] devlink: Add reload level option to devlink reload command
On Tue, 28 Jul 2020 12:18:30 -0700 Jacob Keller wrote: > On 7/28/2020 11:44 AM, Jakub Kicinski wrote: > > From user perspective what's important is what the reset achieves (and > > perhaps how destructive it is). We can define the reset levels as: > > > > $ devlink dev reload pci/:82:00.0 net-ns-respawn > > $ devlink dev reload pci/:82:00.0 driver-param-init > > $ devlink dev reload pci/:82:00.0 fw-activate > > > > combining should be possible when user wants multiple things to happen: > > > > $ devlink dev reload pci/:82:00.0 fw-activate driver-param-init > > Where today "driver-param-init" is the default behavior. But didn't we > just say that mlxsw also does the equivalent of fw-activate? Actually the default should probably be the combination of driver-param-init and net-ns-respawn. My expectations would be that the driver must perform the lowest reset level possible that satisfies the requested functional change. IOW driver may do more, in fact it should be acceptable for the driver to always for a full HW reset (unless --live or other constraint is specified). > > We can also add the "reset level" specifier - for the cases where > > device is misbehaving: > > > > $ devlink dev reload pci/:82:00.0 level [driver|fw|hardware] > > I guess I don't quite see how level fits in? This is orthogonal to the > other settings? Yup, it is, it's already orthogonal to what reload does today, hence the need for the "driver default" hack. > > But I don't think that we can go from the current reload command > > cleanly to just a level reset. The driver-specific default is a bad > > smell which indicates we're changing semantics from what user wants > > to what the reset depth is. Our semantics with the patch as it stands > > are in fact: > > - if you want to load new params or change netns, don't pass the level > >- the "driver default" workaround dictates the right reset level for > >param init; > > - if you want to activate new firmware - select the reset level you'd > >like from the reset level options. > > > > I think I agree, having the "what gets reloaded" as a separate vector > makes sense and avoids confusion. For example for ice hardware, > "fw-activate" really does mean "Do an EMP reset" rather than just a > "device reset" which could be interpreted differently. ice can do > function level reset, or core device reset also. Neither of those two > resets activates firmware. > > Additionally the current function load process in ice doesn't support > driver-init at all. That's something I'd like to see happen but is quite > a significant refactor for how the driver loads. We need to refactor > everything out so that devlink is created early on and factor out > load/unload into handlers that can be called by the devlink reload. As I > have primarily been focused on flash update I sort of left that for the > future because it was a huge task to solve. Cool! That was what I was concerned about, but I didn't know any existing driver already has the problem. "FW reset" is not nearly a clear enough operation. We'd end up with drivers differing and users having to refer to vendor documentation to find out which "reset level" maps to what. I think the components in ethtool-reset try to address the same problem, and they have the notion of per-port, and per-device. In the modern world we lack the per-host notion, but that's still strictly clearer than the limited API proposed here.
Re: [PATCH v2 7/8] x86/kaslr: Clean up slot handling
On Tue, Jul 28, 2020 at 12:34:45PM -0700, Kees Cook wrote: > On Mon, Jul 27, 2020 at 07:08:00PM -0400, Arvind Sankar wrote: > > The number of slots and slot areas can be unsigned int, since on 64-bit, > > the maximum amount of memory is 2^52, the minimum alignment is 2^21, so > > the slot number cannot be greater than 2^31. The slot areas are limited > > by MAX_SLOT_AREA, currently 100. Replace the type used for slot number, > > which is currently a mix of int and unsigned long, with unsigned int > > consistently. > > I think it's good to standardize the type, but let's go to unsigned long > then we don't have to think about this again in the future. Ok, I can do that instead. > > > Drop unnecessary check that number of slots is not zero in > > store_slot_info, it's guaranteed to be at least 1 by the calculation. > > > > Drop unnecessary alignment of image_size to CONFIG_PHYSICAL_ALIGN in > > find_random_virt_addr, it cannot change the result: the largest valid > > slot is the largest n that satisfies > > I view all of these things as robustness checks. It doesn't hurt to do > these checks and it'll avoid crashing into problems if future > refactoring breaks assumptions. Well, at least the first one should really be unnecessary: the previous line sets it as 1 + x. When I see that it actually confuses me: I think I must be missing some edge case where it could be zero. The second one is also unnecessary, but I agree it might require a bit of analysis to see that it is. > > - slots = (KERNEL_IMAGE_SIZE - minimum - image_size) / > > -CONFIG_PHYSICAL_ALIGN + 1; > > + slots = 1 + (KERNEL_IMAGE_SIZE - minimum - image_size) / > > CONFIG_PHYSICAL_ALIGN; > > These are the same -- why change the code? > It's just reformatting now that we can have more than 80-column lines. I think it's clearer this way, more obvious that you're dividing by CONFIG_PHYSICAL_ALIGN and adding one, rather than "did he forget the parentheses here".
Re: [PATCH v1] farsync: use generic power management
On Tue, Jul 28, 2020 at 09:58:10AM +0530, Vaibhav Gupta wrote: > The .suspend() and .resume() callbacks are not defined for this driver. > Still, their power management structure follows the legacy framework. To > bring it under the generic framework, simply remove the binding of > callbacks from "struct pci_driver". FWIW, this commit log is slightly misleading because .suspend and .resume are NULL by default, so this patch actually is a complete no-op as far as code generation is concerned. This change is worthwhile because it simplifies the code a little, but it doesn't convert the driver from legacy to generic power management. This driver doesn't supply a .pm structure, so it doesn't seem to do *any* power management. > Change code indentation from space to tab in "struct pci_driver". > > Signed-off-by: Vaibhav Gupta > --- > drivers/net/wan/farsync.c | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/wan/farsync.c b/drivers/net/wan/farsync.c > index 7916efce7188..15dacfde6b83 100644 > --- a/drivers/net/wan/farsync.c > +++ b/drivers/net/wan/farsync.c > @@ -2636,12 +2636,10 @@ fst_remove_one(struct pci_dev *pdev) > } > > static struct pci_driver fst_driver = { > -.name= FST_NAME, > -.id_table= fst_pci_dev_id, > -.probe = fst_add_one, > -.remove = fst_remove_one, > -.suspend = NULL, > -.resume = NULL, > + .name = FST_NAME, > + .id_table = fst_pci_dev_id, > + .probe = fst_add_one, > + .remove = fst_remove_one, > }; > > static int __init > -- > 2.27.0 >
Re: [PATCH] scsi: sd: add runtime pm to open / release
On Tue, Jul 28, 2020 at 09:02:44AM +0200, Martin Kepplinger wrote: > Hi Alan, > > Any API cleanup is of course welcome. I just wanted to remind you that > the underlying problem: broken block device runtime pm. Your initial > proposed fix "almost" did it and mounting works but during file access, > it still just looks like a runtime_resume is missing somewhere. Well, I have tested that proposed fix several times, and on my system it's working perfectly. When I stop accessing a drive it autosuspends, and when I access it again it gets resumed and works -- as you would expect. > As we need to have that working at some point, I might look into it, but > someone who has experience in the block layer can surely do it more > efficiently. I suspect that any problems you still face are caused by something else. Alan Stern
Re: [PATCH] usb: hso: check for return value in hso_serial_common_create()
From: Rustam Kovhaev Date: Mon, 27 Jul 2020 23:42:17 -0700 > in case of an error tty_register_device_attr() returns ERR_PTR(), > add IS_ERR() check > > Reported-and-tested-by: syzbot+67b2bd0e34f952d03...@syzkaller.appspotmail.com > Link: https://syzkaller.appspot.com/bug?extid=67b2bd0e34f952d0321e > Signed-off-by: Rustam Kovhaev Applied, thank you.
Re: [PATCH v3 3/4] staging: rtl8723bs: include: Further clean up function declarations
On Sun, Jul 26, 2020 at 11:45 PM Aditya Jain wrote: > > On Sun, Jul 26, 2020 at 10:45 PM Joe Perches wrote: > > > > On Sun, 2020-07-26 at 17:02 +0200, Greg KH wrote: > > > On Sun, Jul 26, 2020 at 07:50:12PM +0530, Aditya Jain wrote: > > > > Cleaning up messy multiline function declarations in hal_phy_cfg.h > > [] > > > > diff --git a/drivers/staging/rtl8723bs/include/hal_phy_cfg.h > > > > b/drivers/staging/rtl8723bs/include/hal_phy_cfg.h > > [] > Ok, I'll merge this patch with the previous one. Had separated the two > to make the diffs smaller thinking they'll > be easier to review. But yeah, I get the point. > > > > -void > > > > -PHY_SetSwChnlBWMode8723B( > > > > -struct adapter *Adapter, > > > > -u8 channel, > > > > -enum CHANNEL_WIDTH Bandwidth, > > > > -u8 Offset40, > > > > -u8 Offset80 > > > > +void PHY_SetBWMode8723B(struct adapter *Adapter, > > > > + enum CHANNEL_WIDTH Bandwidth, /* 20M or 40M */ > > > > + unsigned char Offset/* Upper, Lower, or Don't > > > > care */ > > > > > > Those comments should go at the top of the function declaration, in > > > kernel doc format. > > > > Not every external function needs kernel-doc. > > > > This is a realtek staging driver that likely it will never be > > moved out of staging. > > > > > Should I just remove the comments then? Sorry, but I'm a newbie, and > not really sure what the function is or supposed to do. > I can try looking it up if kernel-doc is required here. Hi all, Any directions on how I should continue? > > Regards, > Aditya Jain Thanks and Regards, Aditya Jain
Re: [PATCH v10 3/5] locking/qspinlock: Introduce CNA into the slow path of qspinlock
On 4/3/20 4:59 PM, Alex Kogan wrote: In CNA, spinning threads are organized in two queues, a primary queue for threads running on the same node as the current lock holder, and a secondary queue for threads running on other nodes. After acquiring the MCS lock and before acquiring the spinlock, the lock holder scans the primary queue looking for a thread running on the same node (pre-scan). If found (call it thread T), all threads in the primary queue between the current lock holder and T are moved to the end of the secondary queue. If such T is not found, we make another scan of the primary queue when unlocking the MCS lock (post-scan), starting at the position where pre-scan stopped. If both scans fail to find such T, the MCS lock is passed to the first thread in the secondary queue. If the secondary queue is empty, the lock is passed to the next thread in the primary queue. For more details, see https://arxiv.org/abs/1810.05600. Note that this variant of CNA may introduce starvation by continuously passing the lock to threads running on the same node. This issue will be addressed later in the series. Enabling CNA is controlled via a new configuration option (NUMA_AWARE_SPINLOCKS). By default, the CNA variant is patched in at the boot time only if we run on a multi-node machine in native environment and the new config is enabled. (For the time being, the patching requires CONFIG_PARAVIRT_SPINLOCKS to be enabled as well. However, this should be resolved once static_call() is available.) This default behavior can be overridden with the new kernel boot command-line option "numa_spinlock=on/off" (default is "auto"). Signed-off-by: Alex Kogan Reviewed-by: Steve Sistare Reviewed-by: Waiman Long --- There is also a concern that the worst case latency for a lock transfer can be close to O(n) which can be quite large for large SMP systems. I have a patch on top that modifies the current behavior to limit the number of node scans after the lock is freed. Cheers, Longman >From 1e0d1a7c1d04383367b3d7a086ef3becd2fd81d8 Mon Sep 17 00:00:00 2001 From: Waiman Long Date: Tue, 28 Jul 2020 15:13:47 -0400 Subject: [PATCH 8/9] locking/qspinlock: Limit CNA node scan after the lock is freed When there is a long list of lock waiters, the worst case scenario for CNA is that none of them are from the same NUMA node. All the lock waiters will have to be scanned to figure that out. If the lock becomes free early in the scanning process, it means that there will be a long delay between lock being freed and re-acquired after scanning the full list. The worst case delay is O(n) where n is the number of lock waiters which is limited by the number of cpus in the system. One way to limit the delay is to limit the number of nodes to be scanned after the lock is freed. Assuming random distribution of NUMA nodes in the lock waiters, the chance of not finding a lock waiter in the same NUMA node is ((n-1)/n)^m where n is the number of NUMA nodes and m is the number of lock waiters scanned. If we limit the number of scans to 4n, for example, the chance of not finding same NUMA node lock waiter will be 1.4% and 1.6% for 8 and 16-socket servers respectively. Note that the limit applies only after the lock is freed, so the chance of not finding the right lock waiter is even lower. To make this possible, the lock waiter scanning and lock spinning have to be done at the same time. Since the lock waiter scanning won't stop until the lock is freed with additional look ahead, if necessary. The extra scanning done in cna_lock_handoff() isn't needed anymore. Signed-off-by: Waiman Long --- kernel/locking/qspinlock_cna.h | 130 + 1 file changed, 83 insertions(+), 47 deletions(-) diff --git a/kernel/locking/qspinlock_cna.h b/kernel/locking/qspinlock_cna.h index 03e8ba71a537..89a3905870ea 100644 --- a/kernel/locking/qspinlock_cna.h +++ b/kernel/locking/qspinlock_cna.h @@ -59,7 +59,7 @@ struct cna_node { u16 numa_node; u16 real_numa_node; u32 encoded_tail; /* self */ - u32 partial_order; /* encoded tail or enum val */ + u32 order; /* encoded tail or enum val */ s32 start_time; }; @@ -79,6 +79,12 @@ enum { ((m) + (MSEC_PER_SEC / HZ) - 1) / (MSEC_PER_SEC / HZ) static int intra_node_handoff_threshold __ro_after_init = MSECS_TO_JIFFIES(10); +/* + * Controls how many lookaheads for matching numa node after the lock + * is freed. + */ +static int matching_node_lookahead __ro_after_init; + static inline bool intra_node_threshold_reached(struct cna_node *cn) { s32 current_time = (s32)jiffies; @@ -105,6 +111,11 @@ static void __init cna_init_nodes_per_cpu(unsigned int cpu) */ WARN_ON(cn->encoded_tail < MIN_ENCODED_TAIL); } + /* + * With 4x numa-node lookahead and assuming random node distribution, + * the probability of not finding a matching node is less than 2%. + */ + matching_node_lookahead = 4*nr_node_ids; } static int __init cna_init_nodes(void) @@ -262,33
Re: [PATCH v3 00/19] Introduce partial kernel_read_file() support
Hi Mimi, On 2020-07-28 11:48 a.m., Mimi Zohar wrote: > On Mon, 2020-07-27 at 12:18 -0700, Scott Branden wrote: >> Hi Mimi/Kees, >> >> On 2020-07-27 4:16 a.m., Mimi Zohar wrote: >>> On Fri, 2020-07-24 at 14:36 -0700, Kees Cook wrote: v3: - add reviews/acks - add "IMA: Add support for file reads without contents" patch - trim CC list, in case that's why vger ignored v2 v2: [missing from lkml archives! (CC list too long?) repeating changes >> here] - fix issues in firmware test suite - add firmware partial read patches - various bug fixes/cleanups v1: >> https://lore.kernel.org/lkml/20200717174309.1164575-1-keesc...@chromium.org/ Hi, Here's my tree for adding partial read support in kernel_read_file(), which fixes a number of issues along the way. It's got Scott's firmware and IMA patches ported and everything tests cleanly for me (even with CONFIG_IMA_APPRAISE=y). >>> Thanks, Kees. Other than my comments on the new >>> security_kernel_post_load_data() hook, the patch set is really nice. >>> >>> In addition to compiling with CONFIG_IMA_APPRAISE enabled, have you >>> booted the kernel with the ima_policy=tcb? The tcb policy will add >>> measurements to the IMA measurement list and extend the TPM with the >>> file or buffer data digest. Are you seeing the firmware measurements, >>> in particular the partial read measurement? >> I booted the kernel with ima_policy=tcb. >> >> Unfortunately after enabling the following, fw_run_tests.sh does not run. >> >> mkdir /sys/kernel/security >> mount -t securityfs securityfs /sys/kernel/security >> echo "measure func=FIRMWARE_CHECK" > /sys/kernel/security/ima/policy >> echo "appraise func=FIRMWARE_CHECK appraise_type=imasig" > >> /sys/kernel/security/ima/policy >> ./fw_run_tests.sh >> >> [ 1296.258052] test_firmware: loading 'test-firmware.bin' >> [ 1296.263903] misc test_firmware: loading /lib/firmware/test-firmware.bin >> failed with error -13 >> [ 1296.263905] audit: type=1800 audit(1595905754.266:9): pid=5696 uid=0 >> auid=4294967295 ses=4294967295 subj=kernel op=appraise_data cause=IMA- >> signature-required comm="fw_namespace" name="/lib/firmware/test-firmware.bin" >> dev="tmpfs" ino=4592 res=0 >> [ 1296.297085] misc test_firmware: Direct firmware load for test-firmware.bin >> failed with error -13 >> [ 1296.305947] test_firmware: load of 'test-firmware.bin' failed: -13 > The "appraise" rule verifies the IMA signature. Unless you signed the > firmware > (evmctl) and load the public key on the IMA keyring, that's to be expected. I > assume you are seeing firmware measurements in the IMA measuremenet log. Yes, I see the firmware measurements in the IMA measurement log. I have not signed the firmware nor loaded a public key on the IMA keyring. Therefore everything is working as expected. > > Mimi > Thanks, Scott
Re: [PATCH v3 15/19] IMA: Add support for file reads without contents
On Tue, Jul 28, 2020 at 12:44:50PM -0700, Kees Cook wrote: > On Mon, Jul 27, 2020 at 09:23:34AM -0400, Mimi Zohar wrote: > > On Fri, 2020-07-24 at 14:36 -0700, Kees Cook wrote: > > > From: Scott Branden > > > > > > When the kernel_read_file LSM hook is called with contents=false, IMA > > > can appraise the file directly, without requiring a filled buffer. When > > > such a buffer is available, though, IMA can continue to use it instead > > > of forcing a double read here. > > > > > > Signed-off-by: Scott Branden > > > Link: > > > https://lore.kernel.org/lkml/20200706232309.12010-10-scott.bran...@broadcom.com/ > > > Signed-off-by: Kees Cook > > > > After adjusting the comment below. > > > > Reviewed-by: Mimi Zohar > > Sure! > > Greg, shall I send a v4 with added Reviews and the comment change or is > that minor enough that you're able to do it? v4 is needed, as this series is a mess of reviewes and you will have to redo at least one patch and drop some others, right? thanks, greg k-h
Re: [PATCH v1] farsync: use generic power management
From: Vaibhav Gupta Date: Tue, 28 Jul 2020 09:58:10 +0530 > The .suspend() and .resume() callbacks are not defined for this driver. > Still, their power management structure follows the legacy framework. To > bring it under the generic framework, simply remove the binding of > callbacks from "struct pci_driver". > > Change code indentation from space to tab in "struct pci_driver". > > Signed-off-by: Vaibhav Gupta Applied.
Re: [PATCH] dmaengine: ti: omap-dma: Drop of_match_ptr to fix -Wunused-const-variable
Hi Krzysztof, Thank you for the patch. On Tue, Jul 28, 2020 at 07:09:39PM +0200, Krzysztof Kozlowski wrote: > The of_device_id is included unconditionally by of.h header and used > in the driver as well. Remove of_match_ptr to fix W=1 compile test > warning with !CONFIG_OF: > > drivers/dma/ti/omap-dma.c:1892:34: warning: 'omap_dma_match' defined but > not used [-Wunused-const-variable=] > 1892 | static const struct of_device_id omap_dma_match[] = { > > Signed-off-by: Krzysztof Kozlowski Reviewed-by: Laurent Pinchart > --- > drivers/dma/ti/omap-dma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c > index 918301e17552..c9fe5e3a6b55 100644 > --- a/drivers/dma/ti/omap-dma.c > +++ b/drivers/dma/ti/omap-dma.c > @@ -1904,7 +1904,7 @@ static struct platform_driver omap_dma_driver = { > .remove = omap_dma_remove, > .driver = { > .name = "omap-dma-engine", > - .of_match_table = of_match_ptr(omap_dma_match), > + .of_match_table = omap_dma_match, > }, > }; > -- Regards, Laurent Pinchart
Re: [PATCH net 0/5] net: hns3: fixes for -net
From: Huazhong Tan Date: Tue, 28 Jul 2020 10:16:47 +0800 > There are some bugfixes for the HNS3 ethernet driver. patch#1 fixes > a desc filling bug, patch#2 fixes a false TX timeout issue, and > patch#3~#5 fixes some bugs related to VLAN and FD. Series applied, thank you.
Re: [PATCH v4 4/5] arm64: dts: sdm845: Add OPP tables and power-domains for venus
Quoting Rajendra Nayak (2020-07-27 21:17:28) > > On 7/28/2020 6:22 AM, Stephen Boyd wrote: > > Quoting Viresh Kumar (2020-07-27 08:38:06) > >> On 27-07-20, 17:38, Rajendra Nayak wrote: > >>> On 7/27/2020 11:23 AM, Rajendra Nayak wrote: > On 7/24/2020 7:39 PM, Stanimir Varbanov wrote: > >>> + > >>> + opp-53300 { > >>> + opp-hz = /bits/ 64 <53300>; > >> > >> Is this the highest OPP in table ? > >> > > Actually it comes from videocc, where ftbl_video_cc_venus_clk_src > > defines 53300 but the real calculated freq is 53397. > > I still don't quite understand why the videocc driver returns this > frequency despite this not being in the freq table. > >>> > >>> Ok, so I see the same issue on sc7180 also. clk_round_rate() does seem to > >>> return whats in the freq table, but clk_set_rate() goes ahead and sets it > > > > I'm happy to see clk_round_rate() return the actual rate that would be > > achieved and not just the rate that is in the frequency tables. Would > > that fix the problem? > > It would, but only if I also update the OPP table to have 53397 > instead of 53300 (which I guess is needed anyway) > If this is the actual frequency that's achievable, then perhaps even the clock > freq table should have this? 53397 and not 53300? > That way clk_round_rate() would return the actual rate that's achieved and > we don't need any extra math. Isn't that the reason these freq tables exist > anyway. Yes the freq tables are there in the clk driver so we don't have to do a bunch of math. Fixing them to be accurate has been deemed "hard" from what I recall because the tables are generated from some math function that truncates the lower Hertz values.
Re: linux-next: build failure after merge of the driver-core tree
On Mon, Jul 27, 2020 at 12:17:38PM +0200, Greg KH wrote: > On Mon, Jul 27, 2020 at 04:55:39PM +1000, Stephen Rothwell wrote: > > Hi all, > > > > After merging the driver-core tree, today's linux-next build (x86_64 > > allmodconfig) failed like this: > > > > In file included from include/linux/dmi.h:5, > > from drivers/firmware/efi/embedded-firmware.c:8: > > drivers/firmware/efi/embedded-firmware.c:25:38: error: static declaration > > of 'efi_embedded_fw_list' follows non-static declaration > >25 | EFI_EMBEDDED_FW_VISIBILITY LIST_HEAD(efi_embedded_fw_list); > > | ^~~~ > > include/linux/list.h:24:19: note: in definition of macro 'LIST_HEAD' > >24 | struct list_head name = LIST_HEAD_INIT(name) > > | ^~~~ > > In file included from drivers/firmware/efi/embedded-firmware.c:17: > > drivers/firmware/efi/embedded-firmware.h:16:25: note: previous declaration > > of 'efi_embedded_fw_list' was here > >16 | extern struct list_head efi_embedded_fw_list; > > | ^~~~ > > drivers/firmware/efi/embedded-firmware.c:26:33: error: static declaration > > of 'efi_embedded_fw_checked' follows non-static declaration > >26 | EFI_EMBEDDED_FW_VISIBILITY bool efi_embedded_fw_checked; > > | ^~~ > > In file included from drivers/firmware/efi/embedded-firmware.c:17: > > drivers/firmware/efi/embedded-firmware.h:17:13: note: previous declaration > > of 'efi_embedded_fw_checked' was here > >17 | extern bool efi_embedded_fw_checked; > > | ^~~ > > > > Caused by commit > > > > 2d38dbf89a06 ("test_firmware: Test platform fw loading on non-EFI > > systems") > > > > CONFIG_TEST_FIRMWARE=m for this build. > > > > I have used the driver-core tree from next-20200724 for today. > > Thanks, I've reverted this from my tree now. Ugh, my mistake; sorry for the hassle! I will get this corrected and re-sent. -- Kees Cook
Re: [PATCH] sched/fair: provide u64 read for 32-bits arch helper
Hi, On Tue, Jul 28, 2020 at 02:00:27PM +0200, pet...@infradead.org wrote: > On Tue, Jul 28, 2020 at 01:13:02PM +0200, pet...@infradead.org wrote: > > On Mon, Jul 27, 2020 at 04:23:03PM +0100, Vincent Donnefort wrote: > > > > > For 32-bit architectures, both min_vruntime and last_update_time are using > > > similar access. This patch is simply an attempt to unify their usage by > > > introducing two macros to rely on when accessing those. At the same time, > > > it > > > brings a comment regarding the barriers usage, as per the kernel policy. > > > So > > > overall this is just a clean-up without any functional changes. > > > > Ah, I though there was perhaps the idea to make use of armv7-lpae > > instructions. > > > > Aside of that, I think we need to spend a little time bike-shedding the > > API/naming here: > > > > > +# define u64_32read(val, copy) (val) > > > +# define u64_32read_set_copy(val, copy) do { } while (0) > > > > How about something like: > > > > #ifdef CONFIG_64BIT > > > > #define DEFINE_U64_U32(name)u64 name > > #define u64_u32_load(name) name > > #define u64_u32_store(name, val)name = val > > > > #else > > > > #define DEFINE_U64_U32(name)\ > > struct {\ > > u64 name; \ > > u64 name##_copy;\ > > } > > > > #define u64_u32_load(name) \ > > ({ \ > > u64 val, copy; \ > > do {\ > > val = name; \ > > smb_rmb(); \ > > copy = name##_copy; \ > > } while (val != copy); \ > > wrong order there; we should first read _copy and then the regular one > of course. > > > val; > > }) > > > > #define u64_u32_store(name, val)\ > > do {\ > > typeof(val) __val = (val); \ > > name = __val; \ > > smp_wmb(); \ > > name##_copy = __val;\ > > } while (0) > > > > #endif > > The other approach is making it a full type and inline functions I > suppose. I didn't pick this approach originally, as I thought it would be way too invasive. If the API looks way cleaner, it nonetheless doesn't match last_update_time usage. The variable is declared in sched_avg while its copy is in struct cfs_rq. Moving the copy in sched_avg would mean: * Setting the copy for all struct sched_avg in ___update_load_sum(), instead of just the cfs_rq.avg in update_cfs_rq_load_avg(). * Having the DEFINE_U64_U32 declaration in include/linux/sched.h to cover struct sched_avg. -- Vincent
Re: [PATCH] drivers/net/wan/lapbether: Use needed_headroom instead of hard_header_len
(Reviewing as requested; I'm not familiar with this driver either, or really any WAN driver. It also seems that hard_header_len vs. needed_headroom aren't very well documented, and even I can't guarantee I understand them completely. So take my thoughts with a grain of salt.) Hi, On Sun, Jul 26, 2020 at 04:05:24AM -0700, Xie He wrote: > In net/packet/af_packet.c, the function packet_snd first reserves a > headroom of length (dev->hard_header_len + dev->needed_headroom). > Then if the socket is a SOCK_DGRAM socket, it calls dev_hard_header, > which calls dev->header_ops->create, to create the link layer header. > If the socket is a SOCK_RAW socket, it "un-reserves" a headroom of > length (dev->hard_header_len), and assumes the user to provide the > appropriate link layer header. > > So according to the logic of af_packet.c, dev->hard_header_len should > be the length of the header that would be created by > dev->header_ops->create. I believe I'm with you up to here, but: > However, this driver doesn't provide dev->header_ops, so logically > dev->hard_header_len should be 0. I'm not clear on this part. What's to say you shouldn't be implementing header_ops instead? Note that with WiFi drivers, they're exposing themselves as ARPHRD_ETHER, and only the Ethernet headers are part of the upper "protocol" headers. So my patch deferred to the eth headers. What is the intention with this X25 protocol? I guess the headers added in lapbeth_data_transmit() are supposed to be "invisible", as with this note in af_packet.c? - if device has no dev->hard_header routine, it adds and removes ll header inside itself. In this case ll header is invisible outside of device, but higher levels still should reserve dev->hard_header_len. If that's the case, then yes, I believe this patch should be correct. Brian > So we should use dev->needed_headroom instead of dev->hard_header_len > to request necessary headroom to be allocated. > > Signed-off-by: Xie He
Re: [PATCH v4 4/5] arm64: dts: sdm845: Add OPP tables and power-domains for venus
Quoting Lina Iyer (2020-07-28 09:52:12) > On Mon, Jul 27 2020 at 18:45 -0600, Stephen Boyd wrote: > >Quoting Lina Iyer (2020-07-24 09:28:25) > >> On Fri, Jul 24 2020 at 03:03 -0600, Rajendra Nayak wrote: > >> >Hi Maulik/Lina, > >> > > >> >On 7/23/2020 11:36 PM, Stanimir Varbanov wrote: > >> >>Hi Rajendra, > >> >> > >> >>After applying 2,3 and 4/5 patches on linaro-integration v5.8-rc2 I see > >> >>below messages on db845: > >> >> > >> >>qcom-venus aa0.video-codec: dev_pm_opp_set_rate: failed to find > >> >>current OPP for freq 53397 (-34) > >> >> > >> >>^^^ This one is new. > >> >> > >> >>qcom_rpmh TCS Busy, retrying RPMH message send: addr=0x3 > >> >> > >> >>^^^ and this message is annoying, can we make it pr_debug in rpmh? > >> > > >> How annoyingly often do you see this message? > >> Usually, this is an indication of bad system state either on remote > >> processors in the SoC or in Linux itself. On a smooth sailing build you > >> should not see this 'warning'. > >> > >> >Would you be fine with moving this message to a pr_debug? Its currently > >> >a pr_info_ratelimited() > >> I would rather not, moving this out of sight will mask a lot serious > >> issues that otherwise bring attention to the developers. > >> > > > >I removed this warning message in my patch posted to the list[1]. If > >it's a serious problem then I suppose a timeout is more appropriate, on > >the order of several seconds or so and then a pr_warn() and bail out of > >the async call with an error. > > > The warning used to capture issues that happen within a second and it > helps capture system related issues. Timing out after many seconds > overlooks the system issues that generally tend to resolve itself, but > nevertheless need to be investigated. > Is it correct to read "system related issues" as performance problems where the thread is spinning forever trying to send a message and it can't? So the problem is mostly that it's an unbounded amount of time before the message is sent to rpmh and this printk helps identify those situations where that is happening? Otherwise as you say above it's a bad system state where the rpmh processor has gotten into a bad state like a crash? Can we recover from that? Or is the only recovery a reboot of the system? Does the rpmh processor reboot the system if it crashes?
Re: [PATCH 0/4] arm64: Initial support for Texas Instrument's J7200 Platform
On 28/07/2020 22:19, Grygorii Strashko wrote: On 23/07/2020 11:46, Lokesh Vutla wrote: This series adds initial support for latest new SoC, J7200, from Texas Instruments. The J7200 SoC is a part of the K3 Multicore SoC architecture platform. It is targeted for for automotive gateway, vehicle compute systems, Vehicle-to-Vehicle (V2V) and Vehicle-to-Everything (V2X) applications. The SoC aims to meet the complex processing needs of modern embedded products. See J7200 Technical Reference Manual (SPRUIU1, June 2020) for further details: https://www.ti.com/lit/pdf/spruiu1 Testing: - Boot log: https://pastebin.ubuntu.com/p/FvpzWjf7tw/ - ./scripts/checkpatch --strict - Few warningns about Line length exceeding 100 columns. But these are corresponding to comments - v8make dtbs_check - DT_SCHEMA_FLAGS="-u" DT_SCHEMA_FILES="Documentation/devicetree/bindings/arm/ti/k3.yaml" v8make dtbs_check - DT_SCHEMA_FLAGS="-u" DT_SCHEMA_FILES="Documentation/devicetree/bindings/arm/ti/k3.yaml" v8make dt_binding_check Lokesh Vutla (4): dt-bindings: arm: ti: Add bindings for J7200 SoC dt-bindings: arm: ti: Convert K3 board/soc bindings to DT schema arm64: dts: ti: Add support for J7200 SoC arm64: dts: ti: Add support for J7200 Common Processor Board .../devicetree/bindings/arm/ti/k3.txt | 26 --- .../devicetree/bindings/arm/ti/k3.yaml | 28 +++ MAINTAINERS | 2 +- arch/arm64/boot/dts/ti/Makefile | 3 +- .../dts/ti/k3-j7200-common-proc-board.dts | 64 ++ arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 199 ++ .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 84 arch/arm64/boot/dts/ti/k3-j7200-som-p0.dtsi | 29 +++ arch/arm64/boot/dts/ti/k3-j7200.dtsi | 165 +++ 9 files changed, 572 insertions(+), 28 deletions(-) delete mode 100644 Documentation/devicetree/bindings/arm/ti/k3.txt create mode 100644 Documentation/devicetree/bindings/arm/ti/k3.yaml create mode 100644 arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts create mode 100644 arch/arm64/boot/dts/ti/k3-j7200-main.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-j7200-som-p0.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-j7200.dtsi I have networking enabled on top of this series here. ssh://g...@bitbucket.itg.ti.com/~x1045550/ti-priv-linux-kernel.git ti-linux-5.4.y-for-next-pull But DMA failed for Main domain. 0.781005] ti-udma 3115.dma-controller: NAVSS ti,sci-dev-id read failure -22 [ 0.788684] ti-udma: probe of 3115.dma-controller failed with error -22 http://lcpdresults.itg.ti.com/launcher/results/8013 Is there anything (except my comment for patch 3) which prevents this from been merged? Sry. Pls. ignore this mail. -- Best regards, grygorii
Re: [PATCH 3/4] arm64: dts: ti: Add support for J7200 SoC
Hi Grygorii, On 7/28/20 2:16 PM, Grygorii Strashko wrote: On 23/07/2020 11:46, Lokesh Vutla wrote: The J7200 SoC is a part of the K3 Multicore SoC architecture platform. It is targeted for automotive gateway, vehicle compute systems, Vehicle-to-Vehicle (V2V) and Vehicle-to-Everything (V2X) applications. The SoC aims to meet the complex processing needs of modern embedded products. Some highlights of this SoC are: * Dual Cortex-A72s in a single cluster, two clusters of lockstep capable dual Cortex-R5F MCUs and a Centralized Device Management and Security Controller (DMSC). * Configurable L3 Cache and IO-coherent architecture with high data throughput capable distributed DMA architecture under NAVSS. * Integrated Ethernet switch supporting up to a total of 4 external ports in addition to legacy Ethernet switch of up to 2 ports. * Upto 1 PCIe-GEN3 controller, 1 USB3.0 Dual-role device subsystems, 20 MCANs, 3 McASP, eMMC and SD, OSPI/HyperBus memory controller, I3C and I2C, eCAP/eQEP, eHRPWM among other peripherals. * One hardware accelerator block containing AES/DES/SHA/MD5 called SA2UL management. See J7200 Technical Reference Manual (SPRUIU1, June 2020) for further details: https://www.ti.com/lit/pdf/spruiu1 Signed-off-by: Lokesh Vutla --- arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 199 ++ .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 84 arch/arm64/boot/dts/ti/k3-j7200.dtsi | 165 +++ 3 files changed, 448 insertions(+) create mode 100644 arch/arm64/boot/dts/ti/k3-j7200-main.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-j7200.dtsi diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi new file mode 100644 index ..70c8f7e941fb --- /dev/null +++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi @@ -0,0 +1,199 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Device Tree Source for J7200 SoC Family Main Domain peripherals + * + * Copyright (C) 2020 Texas Instruments Incorporated - https://www.ti.com/ + */ + +_main { + msmc_ram: sram@7000 { + compatible = "mmio-sram"; + reg = <0x0 0x7000 0x0 0x10>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x0 0x7000 0x10>; + + atf-sram@0 { + reg = <0x0 0x2>; + }; + }; + + gic500: interrupt-controller@180 { + compatible = "arm,gic-v3"; + #address-cells = <2>; + #size-cells = <2>; + ranges; + #interrupt-cells = <3>; + interrupt-controller; + reg = <0x00 0x0180 0x00 0x1>, /* GICD */ + <0x00 0x0190 0x00 0x10>; /* GICR */ + + /* vcpumntirq: virtual CPU interface maintenance interrupt */ + interrupts = ; + + gic_its: msi-controller@182 { + compatible = "arm,gic-v3-its"; + reg = <0x00 0x0182 0x00 0x1>; + socionext,synquacer-pre-its = <0x100 0x40>; + msi-controller; + #msi-cells = <1>; + }; + }; + + main_navss: navss@3000 { + compatible = "simple-mfd"; + #address-cells = <2>; + #size-cells = <2>; + ranges = <0x00 0x3000 0x00 0x3000 0x00 0x0c40>; Can we return back ti,sci-dev-id = <199>; here? it's needed for DMA PSI-L pairing. Oh, I wasn't aware of this. Lokesh removed it based on my earlier comments. Is this used by more than one driver? Anytime you see a ti,sci-dev-id, you would also expect to see the ti,sci reference in general for that driver to really leverage it. The lack of a specific binding for this node also makes one think it is not needed. Option: add it as part of dma/net submission. Yeah, I think we should go with this approach on upstream if we aren't respinning the series. Any reason, this couldn't be added to the udma node itself? regards Suman
Re: [PATCH] mm/slab.c: add node spinlock protect in __cache_free_alien
On Tue, 28 Jul 2020, qiang.zh...@windriver.com wrote: > From: Zhang Qiang > > We should add node spinlock protect "n->alien" which may be > assigned to NULL in cpuup_canceled func. cause address access > exception. > Hi, do you have an example NULL pointer dereference where you have hit this? This rather looks like something to fix up in cpuup_canceled() since it's currently manipulating the alien cache for the canceled cpu's node. > Fixes: 18bf854117c6 ("slab: use get_node() and kmem_cache_node() functions") > Signed-off-by: Zhang Qiang > --- > mm/slab.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/mm/slab.c b/mm/slab.c > index a89633603b2d..290523c90b4e 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -759,8 +759,10 @@ static int __cache_free_alien(struct kmem_cache *cachep, > void *objp, > > n = get_node(cachep, node); > STATS_INC_NODEFREES(cachep); > + spin_lock(>list_lock); > if (n->alien && n->alien[page_node]) { > alien = n->alien[page_node]; > + spin_unlock(>list_lock); > ac = >ac; > spin_lock(>lock); > if (unlikely(ac->avail == ac->limit)) { > @@ -769,14 +771,15 @@ static int __cache_free_alien(struct kmem_cache > *cachep, void *objp, > } > ac->entry[ac->avail++] = objp; > spin_unlock(>lock); > - slabs_destroy(cachep, ); > } else { > + spin_unlock(>list_lock); > n = get_node(cachep, page_node); > spin_lock(>list_lock); > free_block(cachep, , 1, page_node, ); > spin_unlock(>list_lock); > - slabs_destroy(cachep, ); > } > + > + slabs_destroy(cachep, ); > return 1; > } > > -- > 2.26.2 > >
Re: [PATCH v2 6/8] x86/kaslr: Simplify process_gb_huge_pages
On Tue, Jul 28, 2020 at 12:27:17PM -0700, Kees Cook wrote: > On Mon, Jul 27, 2020 at 07:07:59PM -0400, Arvind Sankar wrote: > > Short-circuit the whole function on 32-bit. > > > > Replace the loop to determine the number of 1Gb pages with arithmetic. > > > > Fix one minor bug: if the end of the region is aligned on a 1Gb > > boundary, the current code will not use the last available 1Gb page due > > to an off-by-one error. > > > > Signed-off-by: Arvind Sankar > > Can you add some KUnit tests could be written to do validation of the > refactorings? Touching this code is so painful. :) > > -Kees Can I try to do that later -- I've never written a KUnit test, though it's probably a good opportunity to learn how to do one. > > > +++ b/arch/x86/boot/compressed/kaslr.c > > @@ -546,49 +546,43 @@ static void store_slot_info(struct mem_vector > > *region, unsigned long image_size) > > static void > > process_gb_huge_pages(struct mem_vector *region, unsigned long image_size) > > { > > - unsigned long addr, size = 0; > > + unsigned long pud_start, pud_end, gb_huge_pages; > > struct mem_vector tmp; > > - int i = 0; > > > > - if (!max_gb_huge_pages) { > > + if (IS_ENABLED(CONFIG_X86_32) || !max_gb_huge_pages) { > > store_slot_info(region, image_size); > > return; > > } > > Won't max_gb_huge_pages always be false for 32-bit? > > -- > Kees Cook It will, assuming someone doesn't pass bogus command-line arguments to reserve Gb pages on 32-bit. But the IS_ENABLED check allows the compiler to eliminate the entire function at compile time.
Re: [PATCH] mm: memcontrol: don't count limit-setting reclaim as memory pressure
Johannes Weiner writes: When an outside process lowers one of the memory limits of a cgroup (or uses the force_empty knob in cgroup1), direct reclaim is performed in the context of the write(), in order to directly enforce the new limit and have it being met by the time the write() returns. Currently, this reclaim activity is accounted as memory pressure in the cgroup that the writer(!) belongs to. This is unexpected. It specifically causes problems for senpai (https://github.com/facebookincubator/senpai), which is an agent that routinely adjusts the memory limits and performs associated reclaim work in tens or even hundreds of cgroups running on the host. The cgroup that senpai is running in itself will report elevated levels of memory pressure, even though it itself is under no memory shortage or any sort of distress. Move the psi annotation from the central cgroup reclaim function to callsites in the allocation context, and thereby no longer count any limit-setting reclaim as memory pressure. If the newly set limit causes the workload inside the cgroup into direct reclaim, that of course will continue to count as memory pressure. Seems totally reasonable, and the patch looks fine too. Signed-off-by: Johannes Weiner Acked-by: Chris Down
Re: [PATCH v3 15/19] IMA: Add support for file reads without contents
On Mon, Jul 27, 2020 at 09:23:34AM -0400, Mimi Zohar wrote: > On Fri, 2020-07-24 at 14:36 -0700, Kees Cook wrote: > > From: Scott Branden > > > > When the kernel_read_file LSM hook is called with contents=false, IMA > > can appraise the file directly, without requiring a filled buffer. When > > such a buffer is available, though, IMA can continue to use it instead > > of forcing a double read here. > > > > Signed-off-by: Scott Branden > > Link: > > https://lore.kernel.org/lkml/20200706232309.12010-10-scott.bran...@broadcom.com/ > > Signed-off-by: Kees Cook > > After adjusting the comment below. > > Reviewed-by: Mimi Zohar Sure! Greg, shall I send a v4 with added Reviews and the comment change or is that minor enough that you're able to do it? Thanks for the reviews Mimi! -Kees > > > --- > > security/integrity/ima/ima_main.c | 22 -- > > 1 file changed, 16 insertions(+), 6 deletions(-) > > > > diff --git a/security/integrity/ima/ima_main.c > > b/security/integrity/ima/ima_main.c > > index dc4f90660aa6..459e50526a12 100644 > > --- a/security/integrity/ima/ima_main.c > > +++ b/security/integrity/ima/ima_main.c > > @@ -613,11 +613,8 @@ void ima_post_path_mknod(struct dentry *dentry) > > int ima_read_file(struct file *file, enum kernel_read_file_id read_id, > > bool contents) > > { > > - /* Reject all partial reads during appraisal. */ > > - if (!contents) { > > - if (ima_appraise & IMA_APPRAISE_ENFORCE) > > - return -EACCES; > > - } > > + enum ima_hooks func; > > + u32 secid; > > > > /* > > * Do devices using pre-allocated memory run the risk of the > > @@ -626,7 +623,20 @@ int ima_read_file(struct file *file, enum > > kernel_read_file_id read_id, > > * buffers? It may be desirable to include the buffer address > > * in this API and walk all the dma_map_single() mappings to check. > > */ > > - return 0; > > + > > + /* > > +* There will be a call made to ima_post_read_file() with > > +* a filled buffer, so we don't need to perform an extra > > +* read early here. > > +*/ > > + if (contents) > > + return 0; > > + > > + /* Read entire file for all partial reads during appraisal. */ > > In addition to verifying the file signature, the file might be > included in the IMA measurement list or the file hash may be used to > augment the audit record. Please remove "during appraisal" from the > comment. > > > + func = read_idmap[read_id] ?: FILE_CHECK; > > + security_task_getsecid(current, ); > > + return process_measurement(file, current_cred(), secid, NULL, > > + 0, MAY_READ, func); > > } > > > > const int read_idmap[READING_MAX_ID] = { > -- Kees Cook
Re: [PATCH v3 12/19] firmware_loader: Use security_post_load_data()
On Mon, Jul 27, 2020 at 06:57:45AM -0400, Mimi Zohar wrote: > On Fri, 2020-07-24 at 14:36 -0700, Kees Cook wrote: > > Now that security_post_load_data() is wired up, use it instead > > of the NULL file argument style of security_post_read_file(), > > and update the security_kernel_load_data() call to indicate that a > > security_kernel_post_load_data() call is expected. > > > > Wire up the IMA check to match earlier logic. Perhaps a generalized > > change to ima_post_load_data() might look something like this: > > > > return process_buffer_measurement(buf, size, > > kernel_load_data_id_str(load_id), > > read_idmap[load_id] ?: FILE_CHECK, > > 0, NULL); > > > > Signed-off-by: Kees Cook > > process_measurement() measures, verifies a file signature - both > signatures stored as an xattr and as an appended buffer signature - > and augments audit records with the file hash. (Support for measuring, > augmenting audit records, and/or verifying fs-verity signatures has > yet to be added.) > > As explained in my response to 11/19, the file descriptor provides the > file pathname associated with the buffer data. In addition, IMA > policy rules may be defined in terms of other file descriptor info - > uid, euid, uuid, etc. > > Recently support was added for measuring the kexec boot command line, > certificates being loaded onto a keyring, and blacklisted file hashes > (limited to appended signatures). None of these buffers are signed. > process_buffer_measurement() was added for this reason and as a > result is limited to just measuring the buffer data. > > Whether process_measurement() or process_buffer_measurement() should > be modified, needs to be determined. In either case to support the > init_module syscall, would at minimum require the associated file > pathname. Right -- I don't intend to make changes to the init_module() syscall since it's deprecated, so this hook is more of a "fuller LSM coverage for old syscalls" addition. IMA can happily continue to ignore it, which is what I have here, but I thought I'd at least show what it *might* look like. Perhaps BPF LSM is a better example. Does anything need to change for this patch? -- Kees Cook
Re: [PATCH 02/15] iio: sx9310: Update macros declarations
Quoting Daniel Campello (2020-07-28 08:12:45) > Follows spec sheet for macro declarations. > > Signed-off-by: Daniel Campello > --- Reviewed-by: Stephen Boyd
Re: [PATCH v2 5/8] x86/kaslr: Simplify __process_mem_region
On Tue, Jul 28, 2020 at 12:25:16PM -0700, Kees Cook wrote: > On Mon, Jul 27, 2020 at 07:07:58PM -0400, Arvind Sankar wrote: > > Get rid of unnecessary temporary variables and redundant tests in > > __process_mem_region. > > > > Fix one minor bug: in case of an overlap, the beginning of the region > > should be used even if it is exactly image_size, not just strictly > > larger. > > > > Change type of minimum/image_size arguments in process_mem_region to > > unsigned long. These actually can never be above 4G (even on x86_64), > > and they're unsigned long in every other function except this one. > > > > Signed-off-by: Arvind Sankar > > Please split this up (which I think is already planned): > > - bug fix (which looks like a correct fix to me) > - arg type size changes > - refactoring > > I don't currently agree that the refactoring makes things easier to > read, but let's see v3. :) > > -- > Kees Cook Yep
Re: [PATCH 4/4] mm: mmu_notifier: Fix and extend kerneldoc
On Tue, Jul 28, 2020 at 07:11:09PM +0200, Krzysztof Kozlowski wrote: > Fix W=1 compile warnings (invalid kerneldoc): > > mm/mmu_notifier.c:187: warning: Function parameter or member > 'interval_sub' not described in 'mmu_interval_read_bgin' > mm/mmu_notifier.c:708: warning: Function parameter or member > 'subscription' not described in 'mmu_notifier_registr' > mm/mmu_notifier.c:708: warning: Excess function parameter 'mn' > description in 'mmu_notifier_register' > mm/mmu_notifier.c:880: warning: Function parameter or member > 'subscription' not described in 'mmu_notifier_put' > mm/mmu_notifier.c:880: warning: Excess function parameter 'mn' > description in 'mmu_notifier_put' > mm/mmu_notifier.c:982: warning: Function parameter or member 'ops' not > described in 'mmu_interval_notifier_insert' > > Signed-off-by: Krzysztof Kozlowski > --- > mm/mmu_notifier.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) Reviewed-by: Jason Gunthorpe Thanks, Jason
Re: [PATCH 12/15] iio: sx9310: Miscellaneous format fixes
Quoting Daniel Campello (2020-07-28 08:12:55) > Miscellaneous format fixes throughout the whole file. > > Signed-off-by: Daniel Campello > --- Reviewed-by: Stephen Boyd
Re: [PATCH 11/15] iio: sx9310: Use variable to hold >dev
Quoting Daniel Campello (2020-07-28 08:12:54) > Improves readability by storing >dev in a local variable. > > Signed-off-by: Daniel Campello > --- Reviewed-by: Stephen Boyd