Re: [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Tue, Sep 08, 2020 at 06:36:19AM +0100, Christoph Hellwig wrote: > On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote: > > Yeah we talked about passing an attr to map_sg to disable merging at > > the following microconfernce: > > https://linuxplumbersconf.org/event/7/contributions/846/ > > As far as I can remember everyone seemed happy with that solution. I > > won't be working on this though as I don't have any more time to > > dedicate to this. It seems Lu Baolu will take over this. > > I'm absolutely again passing a flag. Tha just invites further > abuse. We need a PCI ID based quirk or something else that can't > be as easily abused. Also, I looked at i915 and there are just three dma_map_sg callers. The two dmabuf related ones are fixed by Marek in his series, leaving just the one in i915_gem_gtt_prepare_pages, which does indeed look very fishy. But if that one is so hard to fix it can just be replaced by an open coded for_each_sg loop that contains manual dma_map_page calls.
[PATCH 2/3] dfl: add dfl bus support to MODULE_DEVICE_TABLE()
Device Feature List (DFL) is a linked list of feature headers within the device MMIO space. It is used by FPGA to enumerate multiple sub features within it. Each feature can be uniquely identified by DFL type and feature id, which can be read out from feature headers. A dfl bus helps DFL framework modularize DFL device drivers for different sub features. The dfl bus matches its devices and drivers by DFL type and feature id. This patch add dfl bus support to MODULE_DEVICE_TABLE() by adding info about struct dfl_device_id in devicetable-offsets.c and add a dfl entry point in file2alias.c. Signed-off-by: Xu Yilun Signed-off-by: Wu Hao Signed-off-by: Matthew Gerlach Signed-off-by: Russ Weight --- scripts/mod/devicetable-offsets.c | 4 scripts/mod/file2alias.c | 13 + 2 files changed, 17 insertions(+) diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c index 27007c1..d8350ee 100644 --- a/scripts/mod/devicetable-offsets.c +++ b/scripts/mod/devicetable-offsets.c @@ -243,5 +243,9 @@ int main(void) DEVID(mhi_device_id); DEVID_FIELD(mhi_device_id, chan); + DEVID(dfl_device_id); + DEVID_FIELD(dfl_device_id, type); + DEVID_FIELD(dfl_device_id, feature_id); + return 0; } diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index 2417dd1..0753fc2 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -1368,6 +1368,18 @@ static int do_mhi_entry(const char *filename, void *symval, char *alias) return 1; } +/* Looks like: dfl:tNfN */ +static int do_dfl_entry(const char *filename, void *symval, char *alias) +{ + DEF_FIELD(symval, dfl_device_id, type); + DEF_FIELD(symval, dfl_device_id, feature_id); + + sprintf(alias, "dfl:t%01Xf%03X", type, feature_id); + + add_wildcard(alias); + return 1; +} + /* Does namelen bytes of name exactly match the symbol? */ static bool sym_is(const char *name, unsigned namelen, const char *symbol) { @@ -1442,6 +1454,7 @@ static const struct devtable devtable[] = { {"tee", SIZE_tee_client_device_id, do_tee_entry}, {"wmi", SIZE_wmi_device_id, do_wmi_entry}, {"mhi", SIZE_mhi_device_id, do_mhi_entry}, + {"dfl", SIZE_dfl_device_id, do_dfl_entry}, }; /* Create MODULE_ALIAS() statements. -- 2.7.4
[PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h
In order to support MODULE_DEVICE_TABLE() for dfl device driver, this patch moves struct dfl_device_id to mod_devicetable.h Signed-off-by: Xu Yilun Signed-off-by: Wu Hao Signed-off-by: Matthew Gerlach Signed-off-by: Russ Weight --- drivers/fpga/dfl.h | 13 + include/linux/mod_devicetable.h | 12 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h index 5dc758f..d5b0760 100644 --- a/drivers/fpga/dfl.h +++ b/drivers/fpga/dfl.h @@ -26,6 +26,7 @@ #include #include #include +#include /* maximum supported number of ports */ #define MAX_DFL_FPGA_PORT_NUM 4 @@ -526,18 +527,6 @@ enum dfl_id_type { }; /** - * struct dfl_device_id - dfl device identifier - * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type. - * @feature_id: contains 12 bits feature identifier local to its DFL FIU type. - * @driver_data: driver specific data. - */ -struct dfl_device_id { - u8 type; - u16 feature_id; - unsigned long driver_data; -}; - -/** * struct dfl_device - represent an dfl device on dfl bus * * @dev: generic device interface. diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index 5b08a47..407d8dc 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -838,4 +838,16 @@ struct mhi_device_id { kernel_ulong_t driver_data; }; +/** + * struct dfl_device_id - dfl device identifier + * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type. + * @feature_id: contains 12 bits feature identifier local to its DFL FIU type. + * @driver_data: driver specific data. + */ +struct dfl_device_id { + __u8 type; + __u16 feature_id; + kernel_ulong_t driver_data; +}; + #endif /* LINUX_MOD_DEVICETABLE_H */ -- 2.7.4
Re: [PATCH v4 1/1] drm: allow limiting the scatter list size.
On Mon, Sep 07, 2020 at 03:53:02PM +0200, Daniel Vetter wrote: > On Mon, Sep 7, 2020 at 1:24 PM Gerd Hoffmann wrote: > > > > Add drm_device argument to drm_prime_pages_to_sg(), so we can > > call dma_max_mapping_size() to figure the segment size limit > > and call into __sg_alloc_table_from_pages() with the correct > > limit. > > > > This fixes virtio-gpu with sev. Possibly it'll fix other bugs > > too given that drm seems to totaly ignore segment size limits > > so far ... > > > > v2: place max_segment in drm driver not gem object. > > v3: move max_segment next to the other gem fields. > > v4: just use dma_max_mapping_size(). > > > > Signed-off-by: Gerd Hoffmann > > Uh, are you sure this works in all cases for virtio? Sure, I've tested it ;) > The comments I've found suggest very much not ... Or is that all very > old stuff only that no one cares about anymore? I think these days it is possible to override dma_ops per device, which in turn allows virtio to deal with the quirks without the rest of the kernel knowing about these details. I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just use the dma api path unconditionally and depend on virtio core having setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. take care, Gerd
Re: [BUG RT] dump-capture kernel not executed for panic in interrupt context
Hi Peter On 9/7/2020 6:23 PM, pet...@infradead.org wrote: According to the original comment in __crash_kexec, the mutex was used to prevent a sys_kexec_load, while crash_kexec is executed. Your proposed patch does not lock the mutex in crash_kexec. Sure, but any mutex taker will (spin) wait for panic_cpu==CPU_INVALID. And if the mutex is already held, we'll not run __crash_kexec() just like the trylock() would do today. Yes you are right, it should work. This does not cover the original use case anymore. The only thing that is protected now are two panicing cores at the same time. I'm not following. AFAICT it does exactly what the old code did. Although maybe I didn't replace all kexec_mutex users, I now see that thing isn't static. Same thing here. Actually, this implementation feels even more hacky to me It's more minimal ;-) It's simpler in that it only provides the required semantics (as I understand them) and does not attempt to implement a more general trylock() like primitive that isn't needed. Here I cannot agree with you. There is a second trylock in kernel_kexec, that cannot be protected using the panic_cpu, but it actually could still use mutex_trylock and check the panic_cpu. This should work I guess: int kexec_trylock(void) { if (!mutex_trylock(_mutex)) { return 0; } smp_mb(); if (panic_cpu != PANIC_CPU_INVALID) { mutex_unlock(_mutex); return 0; } return 1; } Or do I miss something now? All functions protected by mutex_lock cannot be executed, after kexec_trylock resturned 1. kexec_crash will execute up to mutex_is_locked and then roll back. The only thing that can go wrong now is: kexec_trylock executes up to smb_mb. At the same time kexec_crash executes mutex_is_locked, which returns false now and then before panic_cpu is reset, kexec_trylock executes the panic_cpu check, and returns. Now both functions did not get the lock and nothing is executed. Does that sound right to you? If you have no further objections I will post it here Jörg
Re: [PATCH] ath11k: fix a double free and a memory leak
t...@redhat.com wrote: > clang static analyzer reports this problem > > mac.c:6204:2: warning: Attempt to free released memory > kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels); > ^ > > The channels pointer is allocated in ath11k_mac_setup_channels_rates() > When it fails midway, it cleans up the memory it has already allocated. > So the error handling needs to skip freeing the memory. > > There is a second problem. > ath11k_mac_setup_channels_rates(), allocates 3 channels. err_free > misses releasing ar->mac.sbands[NL80211_BAND_6GHZ].channels > > Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") > Signed-off-by: Tom Rix > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 7e8453e35e40 ath11k: fix a double free and a memory leak -- https://patchwork.kernel.org/patch/11759745/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 1/2] dt: bindings: Add new regulator as optional property for WCN3990
Rakesh Pillai wrote: > Add an additional regulator supply as an optional > property for WCN3990. > > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1 > > Signed-off-by: Rakesh Pillai > Reviewed-by: Douglas Anderson > Acked-by: Rob Herring > Signed-off-by: Kalle Valo 2 patches applied to ath-next branch of ath.git, thanks. 8f1553694551 dt: bindings: Add new regulator as optional property for WCN3990 9e69fe31ca9a ath10k: Add support for chain1 regulator supply voting -- https://patchwork.kernel.org/patch/11628309/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v8 3/3] binder: add transaction latency tracer
On Mon, 2020-09-07 at 16:09 +0200, Greg Kroah-Hartman wrote: > On Mon, Sep 07, 2020 at 09:51:15PM +0800, Frankie Chang wrote: > > On Mon, 2020-09-07 at 14:25 +0200, Greg Kroah-Hartman wrote: > > > On Mon, Sep 07, 2020 at 08:00:55PM +0800, Frankie Chang wrote: > > > > From: "Frankie.Chang" > > > > > > > > Record start/end timestamp for binder transaction. > > > > When transaction is completed or transaction is free, > > > > it would be checked if transaction latency over threshold > > > > (default 2 sec), if yes, printing related information for tracing. > > > > > > > > /* Implement details */ > > > > - Add latency tracer module to monitor transaction > > > > by attaching to new tracepoints introduced > > > > when transactions are allocated and freed. > > > > The trace_binder_txn_latency_free would not be enabled > > > > by default. Monitoring which transaction is too slow to > > > > cause some of exceptions is important. So we hook the > > > > tracepoint to call the monitor function. > > > > > > > > - Since some of modules would trigger timeout NE > > > > if their binder transaction don't finish in time, > > > > such as audio timeout (5 sec), even BT command > > > > timeout (2 sec), etc. > > > > Therefore, setting the timeout threshold as default > > > > 2 seconds could be helpful to debug. > > > > But this timeout threshold is configurable, to let > > > > all users determine the more suitable threshold. > > > > > > > > - The reason why printing the related information to > > > > kernel information log but not trace buffer is that > > > > some abnormal transactions may be pending for a long > > > > time ago, they could not be recorded due to buffer > > > > limited. > > > > > > > > Signed-off-by: Frankie.Chang > > > > Acked-by: Todd Kjos > > > > --- > > > > drivers/android/Kconfig |8 +++ > > > > drivers/android/Makefile|1 + > > > > drivers/android/binder.c|6 ++ > > > > drivers/android/binder_internal.h | 13 > > > > drivers/android/binder_latency_tracer.c | 112 > > > > +++ > > > > drivers/android/binder_trace.h | 26 ++- > > > > 6 files changed, 163 insertions(+), 3 deletions(-) > > > > create mode 100644 drivers/android/binder_latency_tracer.c > > > > > > > > diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig > > > > index 53b22e2..8aadaf4 100644 > > > > --- a/drivers/android/Kconfig > > > > +++ b/drivers/android/Kconfig > > > > @@ -54,6 +54,14 @@ config ANDROID_BINDER_IPC_SELFTEST > > > > exhaustively with combinations of various buffer sizes and > > > > alignments. > > > > > > > > +config BINDER_TRANSACTION_LATENCY_TRACKING > > > > + tristate "Android Binder transaction tracking" > > > > + help > > > > + Used for track abnormal binder transaction which is over > > > > threshold, > > > > + when the transaction is done or be free, this transaction > > > > would be > > > > + checked whether it executed overtime. > > > > + If yes, printing out the detailed info. > > > > + > > > > endif # if ANDROID > > > > > > > > endmenu > > > > diff --git a/drivers/android/Makefile b/drivers/android/Makefile > > > > index c9d3d0c9..c2ffdb6 100644 > > > > --- a/drivers/android/Makefile > > > > +++ b/drivers/android/Makefile > > > > @@ -4,3 +4,4 @@ ccflags-y += -I$(src) # needed for > > > > trace events > > > > obj-$(CONFIG_ANDROID_BINDERFS) += binderfs.o > > > > obj-$(CONFIG_ANDROID_BINDER_IPC) += binder.o binder_alloc.o > > > > obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o > > > > +obj-$(CONFIG_BINDER_TRANSACTION_LATENCY_TRACKING) += > > > > binder_latency_tracer.o > > > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > > > > index 0cab900..65ad38c 100644 > > > > --- a/drivers/android/binder.c > > > > +++ b/drivers/android/binder.c > > > > @@ -2674,6 +2674,7 @@ static void binder_transaction(struct binder_proc > > > > *proc, > > > > return_error_line = __LINE__; > > > > goto err_alloc_t_failed; > > > > } > > > > + trace_binder_txn_latency_alloc(t); > > > > INIT_LIST_HEAD(>fd_fixups); > > > > binder_stats_created(BINDER_STAT_TRANSACTION); > > > > spin_lock_init(>lock); > > > > @@ -5177,6 +5178,7 @@ static void > > > > print_binder_transaction_ilocked(struct seq_file *m, > > > >to_proc ? to_proc->pid : 0, > > > >t->to_thread ? t->to_thread->pid : 0, > > > >t->code, t->flags, t->priority, t->need_reply); > > > > + trace_binder_txn_latency_info(m, t); > > > > spin_unlock(>lock); > > > > > > > > if (proc != to_proc) { > > > > @@ -5818,4 +5820,8 @@ static int __init binder_init(void) > > > > #define CREATE_TRACE_POINTS > > > > #include
linux-next: build failure after merge of the rcu tree
Hi all, After merging the rcu tree, today's linux-next build (x86_64 allmodconfig) failed like this: ERROR: modpost: "resched_cpu" [kernel/scftorture.ko] undefined! Caused by commit 20c881d0592c ("scftorture: Add an alternative IPI vector") I have reverted that commit for today. -- Cheers, Stephen Rothwell pgpBJnn723mXz.pgp Description: OpenPGP digital signature
Re: [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote: > Yeah we talked about passing an attr to map_sg to disable merging at > the following microconfernce: > https://linuxplumbersconf.org/event/7/contributions/846/ > As far as I can remember everyone seemed happy with that solution. I > won't be working on this though as I don't have any more time to > dedicate to this. It seems Lu Baolu will take over this. I'm absolutely again passing a flag. Tha just invites further abuse. We need a PCI ID based quirk or something else that can't be as easily abused.
[PATCHv3] soc: qcom: llcc: Support chipsets that can write to llcc registers
From: "Isaac J. Manjarres" Older chipsets may not be allowed to configure certain LLCC registers as that is handled by the secure side software. However, this is not the case for newer chipsets and they must configure these registers according to the contents of the SCT table, while keeping in mind that older targets may not have these capabilities. So add support to allow such configuration of registers to enable capacity based allocation and power collapse retention for capable chipsets. Reason for choosing capacity based allocation rather than the default way based allocation is because capacity based allocation allows more finer grain partition and provides more flexibility in configuration. As for the retention through power collapse, it has an advantage where the cache hits are more when we wake up from power collapse although it does burn more power but the exact power numbers are not known at the moment. Signed-off-by: Isaac J. Manjarres (sai: use existing config instead of dt property and commit msg change) Signed-off-by: Sai Prakash Ranjan --- Changes in v3: * Drop separate table and use existing qcom_llcc_config (Doug) * More descriptive commit msg (Doug) * Directly set the config instead of '|=' (Doug) Changes in v2: * Fix build errors reported by kernel test robot. --- drivers/soc/qcom/llcc-qcom.c | 23 +++ include/linux/soc/qcom/llcc-qcom.h | 2 ++ 2 files changed, 25 insertions(+) diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c index 429b5a60a1ba..b908656ce519 100644 --- a/drivers/soc/qcom/llcc-qcom.c +++ b/drivers/soc/qcom/llcc-qcom.c @@ -45,6 +45,9 @@ #define LLCC_TRP_ATTR0_CFGn(n)(0x21000 + SZ_8 * n) #define LLCC_TRP_ATTR1_CFGn(n)(0x21004 + SZ_8 * n) +#define LLCC_TRP_SCID_DIS_CAP_ALLOC 0x21F00 +#define LLCC_TRP_PCB_ACT 0x21F04 + #define BANK_OFFSET_STRIDE 0x8 /** @@ -89,6 +92,7 @@ struct llcc_slice_config { struct qcom_llcc_config { const struct llcc_slice_config *sct_data; int size; + bool need_llcc_cfg; }; static const struct llcc_slice_config sc7180_data[] = { @@ -122,11 +126,13 @@ static const struct llcc_slice_config sdm845_data[] = { static const struct qcom_llcc_config sc7180_cfg = { .sct_data = sc7180_data, .size = ARRAY_SIZE(sc7180_data), + .need_llcc_cfg = true, }; static const struct qcom_llcc_config sdm845_cfg = { .sct_data = sdm845_data, .size = ARRAY_SIZE(sdm845_data), + .need_llcc_cfg = false, }; static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER; @@ -327,6 +333,7 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev) u32 attr0_val; u32 max_cap_cacheline; u32 sz; + u32 disable_cap_alloc, retain_pc; int ret = 0; const struct llcc_slice_config *llcc_table; struct llcc_slice_desc desc; @@ -369,6 +376,21 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev) attr0_val); if (ret) return ret; + + if (drv_data->need_llcc_config) { + disable_cap_alloc = llcc_table[i].dis_cap_alloc << llcc_table[i].slice_id; + ret = regmap_write(drv_data->bcast_regmap, + LLCC_TRP_SCID_DIS_CAP_ALLOC, disable_cap_alloc); + if (ret) + return ret; + + retain_pc = llcc_table[i].retain_on_pc << llcc_table[i].slice_id; + ret = regmap_write(drv_data->bcast_regmap, + LLCC_TRP_PCB_ACT, retain_pc); + if (ret) + return ret; + } + if (llcc_table[i].activate_on_init) { desc.slice_id = llcc_table[i].slice_id; ret = llcc_slice_activate(); @@ -474,6 +496,7 @@ static int qcom_llcc_probe(struct platform_device *pdev) drv_data->cfg = llcc_cfg; drv_data->cfg_size = sz; + drv_data->need_llcc_config = cfg->need_llcc_cfg; mutex_init(_data->lock); platform_set_drvdata(pdev, drv_data); diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h index 90b864655822..52c780085f61 100644 --- a/include/linux/soc/qcom/llcc-qcom.h +++ b/include/linux/soc/qcom/llcc-qcom.h @@ -73,6 +73,7 @@ struct llcc_edac_reg_data { * @bitmap: Bit map to track the active slice ids * @offsets: Pointer to the bank offsets array * @ecc_irq: interrupt for llcc cache error detection and reporting + * @need_llcc_config: check if llcc configuration is required */ struct llcc_drv_data { struct regmap *regmap; @@ -85,6 +86,7 @@ struct llcc_drv_data { unsigned long *bitmap; u32 *offsets;
Re: [RFC PATCH v2 0/3] mm/gup: fix gup_fast with dynamic page table folding
Le 07/09/2020 à 22:12, Mike Rapoport a écrit : On Mon, Sep 07, 2020 at 08:00:55PM +0200, Gerald Schaefer wrote: This is v2 of an RFC previously discussed here: https://lore.kernel.org/lkml/20200828140314.8556-1-gerald.schae...@linux.ibm.com/ Patch 1 is a fix for a regression in gup_fast on s390, after our conversion to common gup_fast code. It will introduce special helper functions pXd_addr_end_folded(), which have to be used in places where pagetable walk is done w/o lock and with READ_ONCE, so currently only in gup_fast. Patch 2 is an attempt to make that more generic, i.e. change pXd_addr_end() themselves by adding an extra pXd value parameter. That was suggested by Jason during v1 discussion, because he is already thinking of some other places where he might want to switch to the READ_ONCE logic for pagetable walks. In general, that would be the cleanest / safest solution, but there is some impact on other architectures and common code, hence the new and greatly enlarged recipient list. Patch 3 is a "nice to have" add-on, which makes pXd_addr_end() inline functions instead of #defines, so that we get some type checking for the new pXd value parameter. Not sure about Fixes/stable tags for the generic solution. Only patch 1 fixes a real bug on s390, and has Fixes/stable tags. Patches 2 + 3 might still be nice to have in stable, to ease future backports, but I guess "nice to have" does not really qualify for stable backports. I also think that adding pXd parameter to pXd_addr_end() is a cleaner way and with this patch 1 is not really required. I would even merge patches 2 and 3 into a single patch and use only it as the fix. Why not merging patches 2 and 3, but I would keep patch 1 separate but after the generic changes, so that we first do the generic changes, then we do the specific S390 use of it. Christophe
Re: [RFC PATCH v2 3/3] mm: make generic pXd_addr_end() macros inline functions
Le 07/09/2020 à 20:00, Gerald Schaefer a écrit : From: Alexander Gordeev Since pXd_addr_end() macros take pXd page-table entry as a parameter it makes sense to check the entry type on compile. Even though most archs do not make use of page-table entries in pXd_addr_end() calls, checking the type in traversal code paths could help to avoid subtle bugs. Signed-off-by: Alexander Gordeev Signed-off-by: Gerald Schaefer --- include/linux/pgtable.h | 36 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index 67ebc22cf83d..d9e7d16c2263 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -656,31 +656,35 @@ static inline int arch_unmap_one(struct mm_struct *mm, */ #ifndef pgd_addr_end -#define pgd_addr_end(pgd, addr, end) \ -({ unsigned long __boundary = ((addr) + PGDIR_SIZE) & PGDIR_MASK; \ - (__boundary - 1 < (end) - 1)? __boundary: (end); \ -}) +#define pgd_addr_end pgd_addr_end I think that #define is pointless, usually there is no such #define for the default case. +static inline unsigned long pgd_addr_end(pgd_t pgd, unsigned long addr, unsigned long end) +{ unsigned long __boundary = (addr + PGDIR_SIZE) & PGDIR_MASK; + return (__boundary - 1 < end - 1) ? __boundary : end; +} Please use the standard layout, ie entry { and exit } alone on their line, and space between local vars declaration and the rest. Also remove the leading __ in front of var names as it's not needed once it is not macros anymore. f_name() { some_local_var; do_something(); } #endif #ifndef p4d_addr_end -#define p4d_addr_end(p4d, addr, end) \ -({ unsigned long __boundary = ((addr) + P4D_SIZE) & P4D_MASK; \ - (__boundary - 1 < (end) - 1)? __boundary: (end); \ -}) +#define p4d_addr_end p4d_addr_end +static inline unsigned long p4d_addr_end(p4d_t p4d, unsigned long addr, unsigned long end) +{ unsigned long __boundary = (addr + P4D_SIZE) & P4D_MASK; + return (__boundary - 1 < end - 1) ? __boundary : end; +} #endif #ifndef pud_addr_end -#define pud_addr_end(pud, addr, end) \ -({ unsigned long __boundary = ((addr) + PUD_SIZE) & PUD_MASK; \ - (__boundary - 1 < (end) - 1)? __boundary: (end); \ -}) +#define pud_addr_end pud_addr_end +static inline unsigned long pud_addr_end(pud_t pud, unsigned long addr, unsigned long end) +{ unsigned long __boundary = (addr + PUD_SIZE) & PUD_MASK; + return (__boundary - 1 < end - 1) ? __boundary : end; +} #endif #ifndef pmd_addr_end -#define pmd_addr_end(pmd, addr, end) \ -({ unsigned long __boundary = ((addr) + PMD_SIZE) & PMD_MASK; \ - (__boundary - 1 < (end) - 1)? __boundary: (end); \ -}) +#define pmd_addr_end pmd_addr_end +static inline unsigned long pmd_addr_end(pmd_t pmd, unsigned long addr, unsigned long end) +{ unsigned long __boundary = (addr + PMD_SIZE) & PMD_MASK; + return (__boundary - 1 < end - 1) ? __boundary : end; +} #endif /*
Re: [RFC PATCH v2 2/3] mm: make pXd_addr_end() functions page-table entry aware
Le 07/09/2020 à 20:00, Gerald Schaefer a écrit : From: Alexander Gordeev Unlike all other page-table abstractions pXd_addr_end() do not take into account a particular table entry in which context the functions are called. On architectures with dynamic page-tables folding that might lead to lack of necessary information that is difficult to obtain other than from the table entry itself. That already led to a subtle memory corruption issue on s390. By letting pXd_addr_end() functions know about the page-table entry we allow archs not only make extra checks, but also optimizations. As result of this change the pXd_addr_end_folded() functions used in gup_fast traversal code become unnecessary and get replaced with universal pXd_addr_end() variants. The arch-specific updates not only add dereferencing of page-table entry pointers, but also small changes to the code flow to make those dereferences possible, at least for x86 and powerpc. Also for arm64, but in way that should not have any impact. [...] Signed-off-by: Alexander Gordeev Signed-off-by: Gerald Schaefer --- arch/arm/include/asm/pgtable-2level.h| 2 +- arch/arm/mm/idmap.c | 6 ++-- arch/arm/mm/mmu.c| 8 ++--- arch/arm64/kernel/hibernate.c| 16 ++ arch/arm64/kvm/mmu.c | 16 +- arch/arm64/mm/kasan_init.c | 8 ++--- arch/arm64/mm/mmu.c | 25 +++ arch/powerpc/mm/book3s64/radix_pgtable.c | 7 ++--- arch/powerpc/mm/hugetlbpage.c| 6 ++-- You forgot arch/powerpc/mm/book3s64/subpage_prot.c it seems. arch/s390/include/asm/pgtable.h | 8 ++--- arch/s390/mm/page-states.c | 8 ++--- arch/s390/mm/pageattr.c | 8 ++--- arch/s390/mm/vmem.c | 8 ++--- arch/sparc/mm/hugetlbpage.c | 6 ++-- arch/um/kernel/tlb.c | 8 ++--- arch/x86/mm/init_64.c| 15 - arch/x86/mm/kasan_init_64.c | 16 +- include/asm-generic/pgtable-nop4d.h | 2 +- include/asm-generic/pgtable-nopmd.h | 2 +- include/asm-generic/pgtable-nopud.h | 2 +- include/linux/pgtable.h | 26 --- mm/gup.c | 8 ++--- mm/ioremap.c | 8 ++--- mm/kasan/init.c | 17 +- mm/madvise.c | 4 +-- mm/memory.c | 40 mm/mlock.c | 18 --- mm/mprotect.c| 8 ++--- mm/pagewalk.c| 8 ++--- mm/swapfile.c| 8 ++--- mm/vmalloc.c | 16 +- 31 files changed, 165 insertions(+), 173 deletions(-) Christophe
Re: [PATCH V3] arm64/cpuinfo: Define HWCAP name arrays per their actual bit definitions
On 09/07/2020 05:46 PM, Will Deacon wrote: > On Mon, Aug 17, 2020 at 05:34:23PM +0530, Anshuman Khandual wrote: >> HWCAP name arrays (hwcap_str, compat_hwcap_str, compat_hwcap2_str) that are >> scanned for /proc/cpuinfo are detached from their bit definitions making it >> vulnerable and difficult to correlate. It is also bit problematic because >> during /proc/cpuinfo dump these arrays get traversed sequentially assuming >> they reflect and match actual HWCAP bit sequence, to test various features >> for a given CPU. This redefines name arrays per their HWCAP bit definitions >> . It also warns after detecting any feature which is not expected on arm64. >> >> Cc: Catalin Marinas >> Cc: Will Deacon >> Cc: Mark Brown >> Cc: Dave Martin >> Cc: Ard Biesheuvel >> Cc: Mark Rutland >> Cc: Suzuki K Poulose >> Cc: linux-arm-ker...@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual >> --- >> This applies on 5.9-rc1 >> >> Mark, since the patch has changed I have dropped your Acked-by: tag. Are you >> happy to give a new one ? >> >> Changes in V3: >> >> - Moved name arrays to (arch/arm64/kernel/cpuinfo.c) to prevent a build >> warning >> - Replaced string values with NULL for all compat features not possible on >> arm64 >> - Changed compat_hwcap_str[] iteration on size as some NULL values are >> expected >> - Warn once after detecting any feature on arm64 that is not expected >> >> Changes in V2: (https://patchwork.kernel.org/patch/11533755/) >> >> - Defined COMPAT_KERNEL_HWCAP[2] and updated the name arrays per Mark >> - Updated the commit message as required >> >> Changes in V1: (https://patchwork.kernel.org/patch/11532945/) >> >> arch/arm64/include/asm/hwcap.h | 9 +++ >> arch/arm64/kernel/cpuinfo.c| 172 >> ++--- >> 2 files changed, 100 insertions(+), 81 deletions(-) > > [...] > >> +[KERNEL_HWCAP_FP] = "fp", >> +[KERNEL_HWCAP_ASIMD]= "asimd", >> +[KERNEL_HWCAP_EVTSTRM] = "evtstrm", >> +[KERNEL_HWCAP_AES] = "aes", > > It would be nice if the cap and the string were generated by the same > macro, along the lines of: > > #define KERNEL_HWCAP(c) [KERNEL_HWCAP_##c] = #c, > > Does making the constants mixed case break anything, or is it just really > churny to do? Currently all existing HWCAP feature strings are lower case, above change will make them into upper case instead. I could not find a method to force convert #c into lower case constant strings in the macro definition. Would not changing the HWCAP string case here, break user interface ? > >> @@ -166,9 +167,18 @@ static int c_show(struct seq_file *m, void *v) >> seq_puts(m, "Features\t:"); >> if (compat) { >> #ifdef CONFIG_COMPAT >> -for (j = 0; compat_hwcap_str[j]; j++) >> -if (compat_elf_hwcap & (1 << j)) >> +for (j = 0; j < ARRAY_SIZE(compat_hwcap_str); j++) { >> +if (compat_elf_hwcap & (1 << j)) { >> +/* >> + * Warn once if any feature should not >> + * have been present on arm64 platform. >> + */ >> +if (WARN_ON_ONCE(!compat_hwcap_str[j])) >> +continue; >> + >> seq_printf(m, " %s", >> compat_hwcap_str[j]); >> +} >> +} >> >> for (j = 0; compat_hwcap2_str[j]; j++) > > Hmm, I find this pretty confusing now as compat_hwcap_str is not NULL > terminated and must be traversed with a loop bounded by ARRAY_SIZE(...), Right. Thats because unlike before, it can now have some intermediate NULL entries. Hence NULL sentinel based traversal wont be possible any more. > whereas compat_hwcap2_str *is* NULL terminated and is traversed until you > hit the sentinel. > > I think hwcap_str, compat_hwcap_str and compat_hwcap2_str should be > identical in this regard. Sure, will make the traversal based on ARRAY_SIZE() for all three arrays here, to make that uniform. > > Will >
RE: [PATCH V2 2/3] pinctrl: imx: Support building SCU pinctrl core driver as module
> Subject: RE: [PATCH V2 2/3] pinctrl: imx: Support building SCU pinctrl core > driver as module > > > From: Anson Huang > > Sent: Monday, September 7, 2020 8:33 PM > > > > Change PINCTR_IMX_SCU to tristate, remove unnecessary #ifdef and add > > module author, description and license to support building SCU pinctrl > > core driver as module. > > > > Signed-off-by: Anson Huang > > --- > > Changes since V1: > > - split V1 [1/2] patch to 2 patches, this patch supports building SCU > > pinctrl core > > driver as module; > > - remove unnecessary #ifdef check and #else block. > > --- > > drivers/pinctrl/freescale/Kconfig | 2 +- > > drivers/pinctrl/freescale/pinctrl-imx.h | 20 > > drivers/pinctrl/freescale/pinctrl-scu.c | 5 + > > 3 files changed, 6 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/pinctrl/freescale/Kconfig > > b/drivers/pinctrl/freescale/Kconfig > > index 08fcf5c..452c499 100644 > > --- a/drivers/pinctrl/freescale/Kconfig > > +++ b/drivers/pinctrl/freescale/Kconfig > > @@ -7,7 +7,7 @@ config PINCTRL_IMX > > select REGMAP > > > > config PINCTRL_IMX_SCU > > - bool > > + tristate "IMX SCU pinctrl core driver" > > depends on IMX_SCU > > select PINCTRL_IMX > > > > [...] > > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h > > b/drivers/pinctrl/freescale/pinctrl-imx.h > > index 40927ca..fd8c4b6 100644 > > --- a/drivers/pinctrl/freescale/pinctrl-imx.h > > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h > > @@ -144,7 +144,6 @@ struct imx_pinctrl_soc_info { int > > imx_pinctrl_probe(struct platform_device *pdev, > > const struct imx_pinctrl_soc_info *info); > > > > -#ifdef CONFIG_PINCTRL_IMX_SCU > > #define BM_PAD_CTL_GP_ENABLE BIT(30) > > #define BM_PAD_CTL_IFMUX_ENABLEBIT(31) > > #define BP_PAD_CTL_IFMUX 27 > > @@ -157,23 +156,4 @@ int imx_pinconf_set_scu(struct pinctrl_dev > > *pctldev, unsigned pin_id, void imx_pinctrl_parse_pin_scu(struct > imx_pinctrl *ipctl, > >unsigned int *pin_id, struct imx_pin *pin, > >const __be32 **list_p); > > -#else > > -static inline int imx_pinconf_get_scu(struct pinctrl_dev *pctldev, > > - unsigned pin_id, unsigned long *config) > > -{ > > - return -EINVAL; > > -} > > -static inline int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, > > - unsigned pin_id, unsigned long *configs, > > - unsigned num_configs) > > -{ > > - return -EINVAL; > > -} > > -static inline void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl, > > - unsigned int *pin_id, > > - struct imx_pin *pin, > > - const __be32 **list_p) > > -{ > > -} > > -#endif > > #endif /* __DRIVERS_PINCTRL_IMX_H */ > > Should this part of changes go to Patch 1? OK, will send V3. Anson
Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding
Le 07/09/2020 à 20:00, Gerald Schaefer a écrit : From: Alexander Gordeev Commit 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast code") introduced a subtle but severe bug on s390 with gup_fast, due to dynamic page table folding. The question "What would it require for the generic code to work for s390" has already been discussed here https://lkml.kernel.org/r/20190418100218.0a4afd51@mschwideX1 and ended with a promising approach here https://lkml.kernel.org/r/20190419153307.4f2911b5@mschwideX1 which in the end unfortunately didn't quite work completely. We tried to mimic static level folding by changing pgd_offset to always calculate top level page table offset, and do nothing in folded pXd_offset. What has been overlooked is that PxD_SIZE/MASK and thus pXd_addr_end do not reflect this dynamic behaviour, and still act like static 5-level page tables. [...] Fix this by introducing new pXd_addr_end_folded helpers, which take an additional pXd entry value parameter, that can be used on s390 to determine the correct page table level and return corresponding end / boundary. With that, the pointer iteration will always happen in gup_pgd_range for s390. No change for other architectures introduced. Not sure pXd_addr_end_folded() is the best understandable name, allthough I don't have any alternative suggestion at the moment. Maybe could be something like pXd_addr_end_fixup() as it will disappear in the next patch, or pXd_addr_end_gup() ? Also, if it happens to be acceptable to get patch 2 in stable, I think you should switch patch 1 and patch 2 to avoid the step through pXd_addr_end_folded() Fixes: 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast code") Cc: # 5.2+ Reviewed-by: Gerald Schaefer Signed-off-by: Alexander Gordeev Signed-off-by: Gerald Schaefer --- arch/s390/include/asm/pgtable.h | 42 + include/linux/pgtable.h | 16 + mm/gup.c| 8 +++ 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h index 7eb01a5459cd..027206e4959d 100644 --- a/arch/s390/include/asm/pgtable.h +++ b/arch/s390/include/asm/pgtable.h @@ -512,6 +512,48 @@ static inline bool mm_pmd_folded(struct mm_struct *mm) } #define mm_pmd_folded(mm) mm_pmd_folded(mm) +/* + * With dynamic page table levels on s390, the static pXd_addr_end() functions + * will not return corresponding dynamic boundaries. This is no problem as long + * as only pXd pointers are passed down during page table walk, because + * pXd_offset() will simply return the given pointer for folded levels, and the + * pointer iteration over a range simply happens at the correct page table + * level. + * It is however a problem with gup_fast, or other places walking the page + * tables w/o locks using READ_ONCE(), and passing down the pXd values instead + * of pointers. In this case, the pointer given to pXd_offset() is a pointer to + * a stack variable, which cannot be used for pointer iteration at the correct + * level. Instead, the iteration then has to happen by going up to pgd level + * again. To allow this, provide pXd_addr_end_folded() functions with an + * additional pXd value parameter, which can be used on s390 to determine the + * folding level and return the corresponding boundary. + */ +static inline unsigned long rste_addr_end_folded(unsigned long rste, unsigned long addr, unsigned long end) What does 'rste' stands for ? Isn't this line a bit long ? +{ + unsigned long type = (rste & _REGION_ENTRY_TYPE_MASK) >> 2; + unsigned long size = 1UL << (_SEGMENT_SHIFT + type * 11); + unsigned long boundary = (addr + size) & ~(size - 1); + + /* +* FIXME The below check is for internal testing only, to be removed +*/ + VM_BUG_ON(type < (_REGION_ENTRY_TYPE_R3 >> 2)); + + return (boundary - 1) < (end - 1) ? boundary : end; +} + +#define pgd_addr_end_folded pgd_addr_end_folded +static inline unsigned long pgd_addr_end_folded(pgd_t pgd, unsigned long addr, unsigned long end) +{ + return rste_addr_end_folded(pgd_val(pgd), addr, end); +} + +#define p4d_addr_end_folded p4d_addr_end_folded +static inline unsigned long p4d_addr_end_folded(p4d_t p4d, unsigned long addr, unsigned long end) +{ + return rste_addr_end_folded(p4d_val(p4d), addr, end); +} + static inline int mm_has_pgste(struct mm_struct *mm) { #ifdef CONFIG_PGSTE diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index e8cbc2e795d5..981c4c2a31fe 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -681,6 +681,22 @@ static inline int arch_unmap_one(struct mm_struct *mm, }) #endif +#ifndef pgd_addr_end_folded +#define pgd_addr_end_folded(pgd, addr, end)pgd_addr_end(addr, end) +#endif + +#ifndef p4d_addr_end_folded +#define p4d_addr_end_folded(p4d, addr, end)p4d_addr_end(addr,
Re: [PATCH v2 0/7] PHY: Prepare Cadence Torrent PHY driver to support multilink configurations
On 27-08-20, 15:28, Swapnil Jakhade wrote: > Cadence Torrent PHY is a multiprotocol PHY supporting different multilink > PHY configurations including DisplayPort, PCIe, USB, SGMII, QSGMII etc. > Existing Torrent PHY driver supports only DisplayPort. This patch series > prepares Torrent PHY driver so that different multilink configurations can > be supported. It also updates DT bindings accordingly. This doesn't affect > ABI as Torrent PHY driver has never been functional, and therefore do not > exist in any active use case. > > Support for different multilink configurations with register sequences for > protocols above will be added in a separate patch series. Series looks good to me. > This patch series is dependent on PHY attributes patch series [1]. I did not see any obvious depends in the series, if it is not maybe good to rebase and send without dependency -- ~Vinod
RE: [PATCH V2 1/3] pinctrl: imx: Use function callbacks for SCU related functions
> Subject: RE: [PATCH V2 1/3] pinctrl: imx: Use function callbacks for SCU > related functions > > > From: Anson Huang > > Sent: Monday, September 7, 2020 8:33 PM > > > > Use function callbacks for SCU related functions in pinctrl-imx.c in > > order to support the scenario of PINCTRL_IMX is built in while > > PINCTRL_IMX_SCU is built as module, all drivers using SCU pinctrl > > driver need to initialize the SCU related function callback. > > > > Signed-off-by: Anson Huang > > --- > > Changes since V1: > > - split V1 [1/2] patch to 2 patches, this patch does the change of > > using function > > callbacks for SCU related functions. > > --- > > drivers/pinctrl/freescale/pinctrl-imx.c | 8 +++ > > drivers/pinctrl/freescale/pinctrl-imx.h | 37 > > + > > drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 3 +++ > > drivers/pinctrl/freescale/pinctrl-imx8qm.c | 3 +++ > > drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 3 +++ > > 5 files changed, 35 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c > > b/drivers/pinctrl/freescale/pinctrl-imx.c > > index 507e4af..b80c450 100644 > > --- a/drivers/pinctrl/freescale/pinctrl-imx.c > > +++ b/drivers/pinctrl/freescale/pinctrl-imx.c > > @@ -373,7 +373,7 @@ static int imx_pinconf_get(struct pinctrl_dev > *pctldev, > > const struct imx_pinctrl_soc_info *info = ipctl->info; > > > > if (info->flags & IMX_USE_SCU) > > - return imx_pinconf_get_scu(pctldev, pin_id, config); > > + return info->imx_pinconf_get(pctldev, pin_id, config); > > else > > return imx_pinconf_get_mmio(pctldev, pin_id, config); } @@ > -423,7 > > +423,7 @@ static int imx_pinconf_set(struct pinctrl_dev *pctldev, > > const struct imx_pinctrl_soc_info *info = ipctl->info; > > > > if (info->flags & IMX_USE_SCU) > > - return imx_pinconf_set_scu(pctldev, pin_id, > > + return info->imx_pinconf_set(pctldev, pin_id, > >configs, num_configs); > > else > > return imx_pinconf_set_mmio(pctldev, pin_id, @@ -440,7 +440,7 > @@ > > static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev, > > int ret; > > > > if (info->flags & IMX_USE_SCU) { > > - ret = imx_pinconf_get_scu(pctldev, pin_id, ); > > + ret = info->imx_pinconf_get(pctldev, pin_id, ); > > if (ret) { > > dev_err(ipctl->dev, "failed to get %s pinconf\n", > > pin_get_name(pctldev, pin_id)); > > @@ -629,7 +629,7 @@ static int imx_pinctrl_parse_groups(struct > > device_node *np, > > for (i = 0; i < grp->num_pins; i++) { > > pin = &((struct imx_pin *)(grp->data))[i]; > > if (info->flags & IMX_USE_SCU) > > - imx_pinctrl_parse_pin_scu(ipctl, >pins[i], > > + info->imx_pinctrl_parse_pin(ipctl, >pins[i], > > pin, ); > > else > > imx_pinctrl_parse_pin_mmio(ipctl, >pins[i], diff > > --git > > a/drivers/pinctrl/freescale/pinctrl-imx.h > > b/drivers/pinctrl/freescale/pinctrl-imx.h > > index 333d32b..40927ca 100644 > > --- a/drivers/pinctrl/freescale/pinctrl-imx.h > > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h > > @@ -75,6 +75,21 @@ struct imx_cfg_params_decode { > > bool invert; > > }; > > > > +/** > > + * @dev: a pointer back to containing device > > + * @base: the offset to the controller in virtual memory */ struct > > +imx_pinctrl { > > + struct device *dev; > > + struct pinctrl_dev *pctl; > > + void __iomem *base; > > + void __iomem *input_sel_base; > > + const struct imx_pinctrl_soc_info *info; > > + struct imx_pin_reg *pin_regs; > > + unsigned int group_index; > > + struct mutex mutex; > > +}; > > + > > You seems missed my question in the former patch review. > Could you clarify a bit why need move this part code? Please check the mail, I have replied it yestoday as below, the function needs to use imx_pinctrl structure, so it needs to be moved, otherwise, build will fail. > Any reason to move this part of code? It is because below function callback added in imx_pinctrl_soc_info structure need to use imx_pinctrl, otherwise, build will fail. + void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl, Anson
[v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Mon, Sep 07, 2020 at 06:00:29PM +0200, Corentin Labbe wrote: > > The put_unaligned should be _le32. > > This fix the modprobe tcrypt fail. Thanks. Yes the original code was correct. ---8<--- This patch also fixes the incorrect endianness markings in the sun4i-ss driver. It should have no effect in the genereated code. Instead of using cpu_to_Xe32 followed by a memcpy, this patch converts the final hash write to use put_unaligned_X instead. Reported-by: kernel test robot Signed-off-by: Herbert Xu diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c index dc35edd90034..1dff48558f53 100644 --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c @@ -9,6 +9,7 @@ * You could find the datasheet in Documentation/arm/sunxi.rst */ #include "sun4i-ss.h" +#include #include /* This is a totally arbitrary value */ @@ -196,7 +197,7 @@ static int sun4i_hash(struct ahash_request *areq) struct sg_mapping_iter mi; int in_r, err = 0; size_t copied = 0; - __le32 wb = 0; + u32 wb = 0; dev_dbg(ss->dev, "%s %s bc=%llu len=%u mode=%x wl=%u h0=%0x", __func__, crypto_tfm_alg_name(areq->base.tfm), @@ -408,7 +409,7 @@ static int sun4i_hash(struct ahash_request *areq) nbw = op->len - 4 * nwait; if (nbw) { - wb = cpu_to_le32(*(u32 *)(op->buf + nwait * 4)); + wb = le32_to_cpup((__le32 *)(op->buf + nwait * 4)); wb &= GENMASK((nbw * 8) - 1, 0); op->byte_count += nbw; @@ -417,7 +418,7 @@ static int sun4i_hash(struct ahash_request *areq) /* write the remaining bytes of the nbw buffer */ wb |= ((1 << 7) << (nbw * 8)); - bf[j++] = le32_to_cpu(wb); + ((__le32 *)bf)[j++] = cpu_to_le32(wb); /* * number of space to pad to obtain 64o minus 8(size) minus 4 (final 1) @@ -479,16 +480,16 @@ static int sun4i_hash(struct ahash_request *areq) /* Get the hash from the device */ if (op->mode == SS_OP_SHA1) { for (i = 0; i < 5; i++) { + v = readl(ss->base + SS_MD0 + i * 4); if (ss->variant->sha1_in_be) - v = cpu_to_le32(readl(ss->base + SS_MD0 + i * 4)); + put_unaligned_le32(v, areq->result + i * 4); else - v = cpu_to_be32(readl(ss->base + SS_MD0 + i * 4)); - memcpy(areq->result + i * 4, , 4); + put_unaligned_be32(v, areq->result + i * 4); } } else { for (i = 0; i < 4; i++) { - v = cpu_to_le32(readl(ss->base + SS_MD0 + i * 4)); - memcpy(areq->result + i * 4, , 4); + v = readl(ss->base + SS_MD0 + i * 4); + put_unaligned_le32(v, areq->result + i * 4); } } -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] cpufreq: armada-37xx: Add missing MODULE_DEVICE_TABLE
On 07-09-20, 15:27, Pali Rohár wrote: > CONFIG_ARM_ARMADA_37XX_CPUFREQ is tristate option and therefore this > cpufreq driver can be compiled as a module. This patch adds missing > MODULE_DEVICE_TABLE which generates correct modalias for automatic > loading of this cpufreq driver when is compiled as an external module. > > Reviewed-by: Andrew Lunn > Signed-off-by: Pali Rohár > Fixes: 92ce45fb875d7 ("cpufreq: Add DVFS support for Armada 37xx") > --- > drivers/cpufreq/armada-37xx-cpufreq.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/cpufreq/armada-37xx-cpufreq.c > b/drivers/cpufreq/armada-37xx-cpufreq.c > index df1c941260d1..46f33c3a7316 100644 > --- a/drivers/cpufreq/armada-37xx-cpufreq.c > +++ b/drivers/cpufreq/armada-37xx-cpufreq.c > @@ -484,6 +484,12 @@ static int __init armada37xx_cpufreq_driver_init(void) > /* late_initcall, to guarantee the driver is loaded after A37xx clock driver > */ > late_initcall(armada37xx_cpufreq_driver_init); > > +static const struct of_device_id armada37xx_cpufreq_of_match[] = { > + { .compatible = "marvell,armada-3700-nb-pm" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, armada37xx_cpufreq_of_match); > + > MODULE_AUTHOR("Gregory CLEMENT "); > MODULE_DESCRIPTION("Armada 37xx cpufreq driver"); > MODULE_LICENSE("GPL"); Applied. Thanks. -- viresh
Re: [PATCH v6 1/9] kernel: Support TIF_SYSCALL_INTERCEPT flag
Christian Brauner writes: > On Fri, Sep 04, 2020 at 04:31:39PM -0400, Gabriel Krisman Bertazi wrote: >> index afe01e232935..3511c98a7849 100644 >> --- a/include/linux/sched.h >> +++ b/include/linux/sched.h >> @@ -959,7 +959,11 @@ struct task_struct { >> kuid_t loginuid; >> unsigned intsessionid; >> #endif >> -struct seccomp seccomp; >> + >> +struct { >> +unsigned intsyscall_intercept; >> +struct seccomp seccomp; >> +}; > > If there's no specific reason to do this I'd not wrap this in an > anonymous struct. It doesn't really buy anything and there doesn't seem > to be precedent in struct task_struct right now. Also, if this somehow > adds padding it seems you might end up increasing the size of struct > task_struct more than necessary by accident? (I might be wrong > though.) Hi Christian, Thanks for your review on this and on the other patches of this series. I wrapped these to prevent struct layout randomization from separating the flags field from seccomp, as they are going to be used together and I was trying to reduce overhead to seccomp entry due to two cache misses when reading this structure. Measuring it seccomp_benchmark didn't show any difference with the unwrapped version, so perhaps it was a bit of premature optimization? >> diff --git a/include/linux/syscall_intercept.h >> b/include/linux/syscall_intercept.h >> new file mode 100644 >> index ..725d157699da >> --- /dev/null >> +++ b/include/linux/syscall_intercept.h >> @@ -0,0 +1,70 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Copyright (C) 2020 Collabora Ltd. >> + */ >> +#ifndef _SYSCALL_INTERCEPT_H >> +#define _SYSCALL_INTERCEPT_H >> + >> +#include >> +#include >> +#include >> + >> +#define SYSINT_SECCOMP 0x1 > > > > Can we maybe use a better name for this? I noone minds the extra > characters I'd suggest: > SYSCALL_INTERCEPT_SECCOMP > or > SYS_INTERCEPT_SECCOMP > > > will do. Thanks, -- Gabriel Krisman Bertazi
Re: [dyndbg] 70f06a871f: kernel_BUG_at_lib/dynamic_debug.c
Got it. will investigate asap On Wed, Sep 2, 2020 at 3:42 AM kernel test robot wrote: > > Greeting, > > FYI, we noticed the following commit (built with gcc-9): > > commit: 70f06a871f5d40ca8f977eb412358ab03b6804da ("[PATCH v3 3/3] dyndbg: fix > problem parsing format="foo bar"") > url: > https://github.com/0day-ci/linux/commits/Jim-Cromie/dyndbg-cleanups-for-5-9/20200901-022403 > base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git > f75aef392f869018f78cfedf3c320a6b3fcfda6b > > in testcase: kernel-selftests > with following parameters: > > group: kselftests-livepatch > > test-description: The kernel contains a set of "self tests" under the > tools/testing/selftests/ directory. These are intended to be small unit tests > to exercise individual code paths in the kernel. > test-url: https://www.kernel.org/doc/Documentation/kselftest.txt > > > on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 8G > > caused below changes (please refer to attached dmesg/kmsg for entire > log/backtrace): > > > +--+++ > | | 12aeaa9439 | 70f06a871f | > +--+++ > | boot_successes | 6 | 2 | > | boot_failures| 0 | 4 | > | kernel_BUG_at_lib/dynamic_debug.c| 0 | 4 | > | invalid_opcode:#[##] | 0 | 4 | > | RIP:ddebug_exec_query| 0 | 4 | > | Kernel_panic-not_syncing:Fatal_exception | 0 | 4 | > +--+++ > > > If you fix the issue, kindly add following tag > Reported-by: kernel test robot > > > [ 78.796907] kernel BUG at lib/dynamic_debug.c:267! > [ 78.799930] invalid opcode: [#1] PREEMPT SMP PTI > [ 78.801632] CPU: 1 PID: 1068 Comm: test-livepatch. Tainted: G > K 5.9.0-rc3-3-g70f06a871f5d4 #1 > [ 78.803877] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > 1.12.0-1 04/01/2014 > [ 78.805929] RIP: 0010:ddebug_exec_query+0x684/0x760 > [ 78.807642] Code: 0f 85 c0 08 00 00 4c 89 7c 24 28 e9 2d fc ff ff 4c 89 fe > 48 8d 7c 24 10 e8 a9 f8 ff ff 85 c0 0f 84 18 fc ff ff e9 c1 07 00 00 <0f> 0b > 8b 15 ac 37 e2 02 85 d2 0f 85 c1 00 00 00 45 31 e4 48 c7 c6 > [ 78.812169] RSP: 0018:b325411c7d78 EFLAGS: 00010246 > [ 78.815411] RAX: 003d RBX: 9f4dab69286c RCX: > > [ 78.821166] RDX: 003d RSI: 9f4dab692868 RDI: > 9f4dab69286c > [ 78.823160] RBP: 0004 R08: 9f4dab69286e R09: > 0001 > [ 78.825172] R10: R11: 0246 R12: > 0004 > [ 78.827212] R13: 0004 R14: R15: > > [ 78.829228] FS: 7f024c29c740() GS:9f4e77d0() > knlGS: > [ 78.831343] CS: 0010 DS: ES: CR0: 80050033 > [ 78.833243] CR2: 7f024c45c8a0 CR3: 00016836 CR4: > 000406e0 > [ 78.835312] DR0: DR1: DR2: > > [ 78.837371] DR3: DR6: fffe0ff0 DR7: > 0400 > [ 78.839398] Call Trace: > [ 78.840935] ? __might_fault+0x36/0x80 > [ 78.842574] ddebug_exec_queries+0x6a/0x100 > [ 78.844322] ddebug_proc_write+0x4e/0x80 > [ 78.845985] full_proxy_write+0x56/0x80 > [ 78.847621] vfs_write+0xec/0x240 > [ 78.849189] ksys_write+0x68/0xe0 > [ 78.850738] do_syscall_64+0x33/0x40 > [ 78.852347] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [ 78.854106] RIP: 0033:0x7f024c389504 > [ 78.855634] Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 > 00 00 00 48 8d 05 f9 61 0d 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d > 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 49 89 d4 55 48 89 f5 53 > [ 78.860364] RSP: 002b:7fff5962d2b8 EFLAGS: 0246 ORIG_RAX: > 0001 > [ 78.864609] RAX: ffda RBX: 00bc RCX: > 7f024c389504 > [ 78.866696] RDX: 00bc RSI: 5582ce63dcd0 RDI: > 0001 > [ 78.868751] RBP: 5582ce63dcd0 R08: fff0 R09: > 7f024c419e80 > [ 78.870832] R10: 5582ce63dd8c R11: 0246 R12: > 7f024c45b760 > [ 78.872933] R13: 00bc R14: 7f024c456760 R15: > 00bc > [ 78.875077] Modules linked in: intel_rapl_msr intel_rapl_common snd_pcm > sr_mod cdrom sg crct10dif_pclmul ppdev crc32_pclmul snd_timer bochs_drm > crc32c_intel ata_generic ghash_clmulni_intel snd pata_acpi drm_vram_helper > aesni_intel drm_ttm_helper crypto_simd ttm cryptd ata_piix glue_helper > soundcore joydev pcspkr serio_raw parport_pc libata parport ipmi_devintf > ipmi_msghandler floppy
linux-next: manual merge of the mmc tree with the samsung-krzk tree
Hi all, Today's linux-next merge of the mmc tree got a conflict in: drivers/mmc/host/Kconfig between commits: cb6c03019cdd ("ARM: exynos: stop selecting PLAT_SAMSUNG") db8230d29c3a ("ARM: s5pv210: don't imply CONFIG_PLAT_SAMSUNG") from the samsung-krzk tree and commit: 54d8454436a2 ("mmc: host: Enable compile testing of multiple drivers") from the mmc tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc drivers/mmc/host/Kconfig index 0d7c61d8d1d9,dc646359b4ff.. --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@@ -289,7 -301,8 +301,8 @@@ config MMC_SDHCI_TEGR config MMC_SDHCI_S3C tristate "SDHCI support on Samsung S3C SoC" - depends on MMC_SDHCI && (PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS) + depends on MMC_SDHCI - depends on PLAT_SAMSUNG || COMPILE_TEST ++ depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST help This selects the Secure Digital Host Controller Interface (SDHCI) often referrered to as the HSMMC block in some of the Samsung S3C pgpTrLpHdousc.pgp Description: OpenPGP digital signature
Re: [PATCH] SELinux: Measure state and hash of policy using IMA
On 9/7/20 3:32 PM, Stephen Smalley wrote: Signed-off-by: Lakshmi Ramasubramanian Suggested-by: Stephen Smalley Reported-by: kernel test robot # error: implicit declaration of function 'vfree' Reported-by: kernel test robot # error: implicit declaration of function 'crypto_alloc_shash' Reported-by: kernel test robot # sparse: symbol 'security_read_selinux_policy' was not declared. Should it be static? Not sure these Reported-by lines are useful since they were just on submitted versions of the patch not on an actual merged commit. I'll remove them when I update the patch. diff --git a/security/selinux/measure.c b/security/selinux/measure.c new file mode 100644 index ..caf9107937d9 --- /dev/null +++ b/security/selinux/measure.c +void selinux_measure_state(struct selinux_state *state, bool policy_mutex_held) +{ + + if (!policy_mutex_held) + mutex_lock(>policy_mutex); + + rc = security_read_policy_kernel(state, , _len); + + if (!policy_mutex_held) + mutex_unlock(>policy_mutex); This kind of conditional taking of a mutex is generally frowned upon in my experience. You should likely just always take the mutex in the callers of selinux_measure_state() instead. In some cases, it may be the caller of the caller. Arguably selinuxfs could be taking it around all state modifying operations (e.g. enforce, checkreqprot) not just policy modifying ones although it isn't strictly for that purpose. Since currently policy_mutex is not used to synchronize access to state variables (enforce, checkreqprot, etc.) I am wondering if selinux_measure_state() should measure only state if policy_mutex is not held by the caller - similar to how we skip measuring policy if initialization is not yet completed. /* * Measure SELinux policy only after initialization is * completed. */ if (!initialized) goto out; -lakshmi
Re: [RFC PATCH v2 0/3] mm/gup: fix gup_fast with dynamic page table folding
Le 07/09/2020 à 20:00, Gerald Schaefer a écrit : This is v2 of an RFC previously discussed here: https://lore.kernel.org/lkml/20200828140314.8556-1-gerald.schae...@linux.ibm.com/ Patch 1 is a fix for a regression in gup_fast on s390, after our conversion to common gup_fast code. It will introduce special helper functions pXd_addr_end_folded(), which have to be used in places where pagetable walk is done w/o lock and with READ_ONCE, so currently only in gup_fast. Patch 2 is an attempt to make that more generic, i.e. change pXd_addr_end() themselves by adding an extra pXd value parameter. That was suggested by Jason during v1 discussion, because he is already thinking of some other places where he might want to switch to the READ_ONCE logic for pagetable walks. In general, that would be the cleanest / safest solution, but there is some impact on other architectures and common code, hence the new and greatly enlarged recipient list. Patch 3 is a "nice to have" add-on, which makes pXd_addr_end() inline functions instead of #defines, so that we get some type checking for the new pXd value parameter. Not sure about Fixes/stable tags for the generic solution. Only patch 1 fixes a real bug on s390, and has Fixes/stable tags. Patches 2 + 3 might still be nice to have in stable, to ease future backports, but I guess "nice to have" does not really qualify for stable backports. If one day you have to backport a fix that requires patch 2 and/or 3, just mark it "depends-on:" and the patches will go in stable at the relevant time. Christophe
Re: [PATCH v3 0/7] set clang minimum version to 10.0.1
On Mon, Sep 07, 2020 at 12:12:30PM -0400, Arvind Sankar wrote: > On Wed, Sep 02, 2020 at 03:59:04PM -0700, Nick Desaulniers wrote: > > Adds a compile time #error to compiler-clang.h setting the effective > > minimum supported version to clang 10.0.1. A separate patch has already > > been picked up into the Documentation/ tree also confirming the version. > > > > Is 10.0.1 actually required or could it just check major version? I have > 10.0.0 currently and at least x86 seems to be building fine. > > Thanks. There was a decent amount of effort put in to testing LLVM 10.0.1 and making sure that it could handle the kernel. I know of a few backend errors that were fixed and backported to 10.0.1: https://github.com/ClangBuiltLinux/linux/issues/944 https://github.com/ClangBuiltLinux/linux/issues/954 Plus there was this rather nasty ld.lld crash in 10.0.0 that just x86_64_defconfig triggers with mainline: https://github.com/ClangBuiltLinux/linux/issues/962 I do not have any strong opinions around checking just major version but I would prefer that we stick with 10.0.1 because it has been tested against several kernel configs unlike 10.0.0. However, I know that Kees mentioned that Ubuntu 20.04 shipped clang 10.0.0 and there is no 10.0.1 available yet. Presumably it is coming down the pipeline from Debian since 10.0.1 appears to be in testing? I suppose if 10.0.0 is shipped in multiple places without an easy upgrade path to 10.0.1, we should consider softening up this version check, at least for the time being. I just worry about duplicate reports. Cheers, Nathan
[PATCH 4/4] perf test: Add multiply cgroup event test
It'll multiply given events for cgroups A, B and C. $ ./perf test -v 68 68: Event multiplication for cgroups : --- start --- test child forked, pid 983140 metric expr 1 / IPC for CPI metric expr instructions / cycles for IPC found event instructions found event cycles adding {instructions,cycles}:W copying metric event for cgroup 'A': instructions (idx=0) copying metric event for cgroup 'B': instructions (idx=0) copying metric event for cgroup 'C': instructions (idx=0) test child finished with 0 end Event multiplication for cgroups: Ok Cc: John Garry Signed-off-by: Namhyung Kim --- tools/perf/builtin-stat.c | 2 +- tools/perf/tests/Build | 1 + tools/perf/tests/builtin-test.c| 4 + tools/perf/tests/multiply-cgroup.c | 203 + tools/perf/tests/tests.h | 1 + tools/perf/util/cgroup.c | 19 ++- tools/perf/util/cgroup.h | 2 +- 7 files changed, 223 insertions(+), 9 deletions(-) create mode 100644 tools/perf/tests/multiply-cgroup.c diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 55a7dc175cdf..c231972f3581 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -2262,7 +2262,7 @@ int cmd_stat(int argc, const char **argv) if (multiply_cgroup && stat_config.cgroups) { if (evlist__multiply_cgroup(evsel_list, stat_config.cgroups, - _config.metric_events) < 0) + _config.metric_events, true) < 0) goto out; } diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build index 84352fc49a20..e685a3441853 100644 --- a/tools/perf/tests/Build +++ b/tools/perf/tests/Build @@ -60,6 +60,7 @@ perf-y += api-io.o perf-y += demangle-java-test.o perf-y += pfm.o perf-y += parse-metric.o +perf-y += multiply-cgroup.o $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build $(call rule_mkdir) diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index d328caaba45d..de48b9d28230 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -341,6 +341,10 @@ static struct test generic_tests[] = { .desc = "Parse and process metrics", .func = test__parse_metric, }, + { + .desc = "Event multiplication for cgroups", + .func = test__multiply_cgroup_events, + }, { .func = NULL, }, diff --git a/tools/perf/tests/multiply-cgroup.c b/tools/perf/tests/multiply-cgroup.c new file mode 100644 index ..89af1644cd6a --- /dev/null +++ b/tools/perf/tests/multiply-cgroup.c @@ -0,0 +1,203 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "tests.h" +#include "debug.h" +#include "evlist.h" +#include "cgroup.h" +#include "rblist.h" +#include "metricgroup.h" +#include "parse-events.h" +#include "pmu-events/pmu-events.h" +#include +#include +#include + +static int test_multiply_events(struct evlist *evlist, + struct rblist *metric_events) +{ + int i, ret = TEST_FAIL; + int nr_events; + bool was_group_event; + int nr_members; /* for the first evsel only */ + const char cgrp_str[] = "A,B,C"; + const char *cgrp_name[] = { "A", "B", "C" }; + int nr_cgrps = ARRAY_SIZE(cgrp_name); + char **ev_name; + struct evsel *evsel; + + TEST_ASSERT_VAL("evlist is empty", !perf_evlist__empty(evlist)); + + nr_events = evlist->core.nr_entries; + ev_name = calloc(nr_events, sizeof(*ev_name)); + if (ev_name == NULL) { + pr_debug("memory allocation failure\n"); + return TEST_FAIL; + } + i = 0; + evlist__for_each_entry(evlist, evsel) { + ev_name[i] = strdup(evsel->name); + if (ev_name[i] == NULL) { + pr_debug("memory allocation failure\n"); + goto out; + } + i++; + } + /* remember grouping info */ + was_group_event = evsel__is_group_event(evlist__first(evlist)); + nr_members = evlist__first(evlist)->core.nr_members; + + ret = evlist__multiply_cgroup(evlist, cgrp_str, metric_events, false); + if (ret < 0) { + pr_debug("failed to multiply cgroup\n"); + goto out; + } + + ret = TEST_FAIL; + if (evlist->core.nr_entries != nr_events * nr_cgrps) { + pr_debug("event count doesn't match\n"); + goto out; + } + + i = 0; + evlist__for_each_entry(evlist, evsel) { + if (strcmp(evsel->name, ev_name[i % nr_events])) { + pr_debug("event name doesn't match:\n"); + pr_debug(" evsel[%d]: %s\n expected: %s\n", +
[PATCH 3/4] perf tools: Copy metric events properly when multiply cgroups
The metricgroup__copy_metric_events() is to handle metrics events when multiplying event for cgroups. As the metric events keep pointers to evsel, it should be refreshed when events are cloned during the operation. The perf_stat__collect_metric_expr() is also called in case an event has a metric directly. During the copy, it references evsel by index as the evlist now has cloned evsels for the given cgroup. Cc: John Garry Cc: Kajol Jain Cc: Ian Rogers Signed-off-by: Namhyung Kim --- tools/perf/builtin-stat.c | 3 +- tools/perf/util/cgroup.c | 15 ++- tools/perf/util/cgroup.h | 4 +- tools/perf/util/evlist.c | 11 + tools/perf/util/evlist.h | 1 + tools/perf/util/metricgroup.c | 77 +++ tools/perf/util/metricgroup.h | 6 +++ 7 files changed, 114 insertions(+), 3 deletions(-) diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 6724d23ce2e7..55a7dc175cdf 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -2261,7 +2261,8 @@ int cmd_stat(int argc, const char **argv) goto out; if (multiply_cgroup && stat_config.cgroups) { - if (evlist__multiply_cgroup(evsel_list, stat_config.cgroups) < 0) + if (evlist__multiply_cgroup(evsel_list, stat_config.cgroups, + _config.metric_events) < 0) goto out; } diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c index 4892f9496bc2..1ba61b5d66b4 100644 --- a/tools/perf/util/cgroup.c +++ b/tools/perf/util/cgroup.c @@ -3,6 +3,9 @@ #include "evsel.h" #include "cgroup.h" #include "evlist.h" +#include "rblist.h" +#include "metricgroup.h" +#include "stat.h" #include #include #include @@ -198,10 +201,12 @@ int parse_cgroups(const struct option *opt, const char *str, return 0; } -int evlist__multiply_cgroup(struct evlist *evlist, const char *str) +int evlist__multiply_cgroup(struct evlist *evlist, const char *str, + struct rblist *metric_events) { struct evlist *orig_list, *tmp_list; struct evsel *pos, *evsel, *leader; + struct rblist orig_metric_events; struct cgroup *cgrp = NULL; const char *p, *e, *eos = str + strlen(str); int ret = -1; @@ -221,6 +226,8 @@ int evlist__multiply_cgroup(struct evlist *evlist, const char *str) /* save original events and init evlist */ perf_evlist__splice_list_tail(orig_list, >core.entries); evlist->core.nr_entries = 0; + orig_metric_events = *metric_events; + rblist__init(metric_events); for (;;) { p = strchr(str, ','); @@ -256,6 +263,11 @@ int evlist__multiply_cgroup(struct evlist *evlist, const char *str) cgroup__put(cgrp); nr_cgroups++; + perf_stat__collect_metric_expr(tmp_list); + if (metricgroup__copy_metric_events(tmp_list, cgrp, metric_events, + _metric_events) < 0) + break; + perf_evlist__splice_list_tail(evlist, _list->core.entries); tmp_list->core.nr_entries = 0; @@ -267,6 +279,7 @@ int evlist__multiply_cgroup(struct evlist *evlist, const char *str) } evlist__delete(orig_list); evlist__delete(tmp_list); + rblist__exit(_metric_events); return ret; } diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h index 9a842f243dfb..87dde992a172 100644 --- a/tools/perf/util/cgroup.h +++ b/tools/perf/util/cgroup.h @@ -23,9 +23,11 @@ struct cgroup *cgroup__get(struct cgroup *cgroup); void cgroup__put(struct cgroup *cgroup); struct evlist; +struct rblist; struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char *name); -int evlist__multiply_cgroup(struct evlist *evlist, const char *cgroups); +int evlist__multiply_cgroup(struct evlist *evlist, const char *cgroups, + struct rblist *metric_events); void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup *cgroup); diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index e3fa3bf7498a..457df8ce1fd9 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -1866,3 +1866,14 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd) return err; } + +struct evsel *evlist__get_evsel(struct evlist *evlist, int idx) +{ + struct evsel *evsel; + + evlist__for_each_entry(evlist, evsel) { + if (evsel->idx == idx) + return evsel; + } + return NULL; +} diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index c73f7f7f120b..57f5fd5e6290 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -381,4 +381,5 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
[PATCHSET 0/4] perf stat: Add --multiply-cgroup option
Hello, When we profile cgroup events with perf stat, it's very annoying to specify events and cgroups on the command line as it requires the mapping between events and cgroups. (Note that perf record can use cgroup sampling but it's not usable for perf stat). I guess most cases we just want to use a same set of events (N) for all cgroups (M), but we need to specify NxM events and NxM cgroups. This is not good especially when profiling large number of cgroups: say M=200. So I added --multiply-cgroup option to make it easy for that case. It will create NxM events from N events and M cgroups. One more upside is that it can handle metrics too. For example, the following example measures IPC metric for 3 cgroups $ cat perf-multi-cgrp.sh #!/bin/sh METRIC=${1:-IPC} CGROUP_DIR=/sys/fs/cgroup/perf_event sudo mkdir $CGROUP_DIR/A $CGROUP_DIR/B $CGROUP_DIR/C # add backgroupd workload for each cgroup echo $$ | sudo tee $CGROUP_DIR/A/cgroup.procs > /dev/null yes > /dev/null & echo $$ | sudo tee $CGROUP_DIR/B/cgroup.procs > /dev/null yes > /dev/null & echo $$ | sudo tee $CGROUP_DIR/C/cgroup.procs > /dev/null yes > /dev/null & # run 'perf stat' in the root cgroup echo $$ | sudo tee $CGROUP_DIR/cgroup.procs > /dev/null perf stat -a -M $METRIC --multiply-cgroup -G A,B,C sleep 1 kill %1 %2 %3 sudo rmdir $CGROUP_DIR/A $CGROUP_DIR/B $CGROUP_DIR/C $ ./perf-multi-cgrp.sh IPC Performance counter stats for 'system wide': 11,284,850,010 inst_retired.any A # 2.71 IPC 4,157,915,982 cpu_clk_unhalted.thread A 11,342,188,640 inst_retired.any B # 2.72 IPC 4,173,014,732 cpu_clk_unhalted.thread B 11,135,863,604 inst_retired.any C # 2.67 IPC 4,171,375,184 cpu_clk_unhalted.thread C 1.011948803 seconds time elapsed The code is available at 'perf/cgroup-multiply-v1' branch on git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git Thanks Namhyung Namhyung Kim (4): perf evsel: Add evsel__clone() function perf stat: Add --multiply-cgroup option perf tools: Copy metric events properly when multiply cgroups perf test: Add multiply cgroup event test tools/perf/builtin-stat.c | 21 ++- tools/perf/tests/Build | 1 + tools/perf/tests/builtin-test.c| 4 + tools/perf/tests/multiply-cgroup.c | 203 + tools/perf/tests/tests.h | 1 + tools/perf/util/cgroup.c | 106 ++- tools/perf/util/cgroup.h | 4 + tools/perf/util/evlist.c | 11 ++ tools/perf/util/evlist.h | 1 + tools/perf/util/evsel.c| 57 tools/perf/util/evsel.h| 1 + tools/perf/util/metricgroup.c | 77 +++ tools/perf/util/metricgroup.h | 6 + tools/perf/util/stat.h | 1 + 14 files changed, 488 insertions(+), 6 deletions(-) create mode 100644 tools/perf/tests/multiply-cgroup.c -- 2.28.0.526.ge36021eeef-goog
[PATCH 1/4] perf evsel: Add evsel__clone() function
The evsel__clone() is to create an exactly same evsel from same attributes. Note that metric events will be handled by later patch. It will be used by perf stat to generate separate events for each cgroup. Signed-off-by: Namhyung Kim --- tools/perf/util/evsel.c | 57 + tools/perf/util/evsel.h | 1 + 2 files changed, 58 insertions(+) diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index fd865002cbbd..4f50f9499973 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -331,6 +331,63 @@ struct evsel *evsel__new_cycles(bool precise) goto out; } +/** + * evsel__clone - create a new evsel copied from @orig + * @orig: original evsel + * + * The assumption is that @orig is not configured nor opened yet. + * So we only care about the attributes that can be set while it's parsed. + */ +struct evsel *evsel__clone(struct evsel *orig) +{ + struct evsel *evsel; + struct evsel_config_term *pos, *tmp; + + BUG_ON(orig->core.fd); + + evsel = evsel__new(>core.attr); + if (evsel == NULL) + return NULL; + + *evsel = *orig; + evsel->evlist = NULL; + INIT_LIST_HEAD(>core.node); + + evsel->core.cpus = perf_cpu_map__get(orig->core.cpus); + evsel->core.own_cpus = perf_cpu_map__get(orig->core.own_cpus); + evsel->core.threads = perf_thread_map__get(orig->core.threads); + if (orig->name) + evsel->name = strdup(orig->name); + if (orig->group_name) + evsel->group_name = strdup(orig->group_name); + if (orig->pmu_name) + evsel->pmu_name = strdup(orig->pmu_name); + + INIT_LIST_HEAD(>config_terms); + list_for_each_entry(pos, >config_terms, list) { + tmp = malloc(sizeof(*tmp)); + if (tmp == NULL) { + evsel__delete(evsel); + evsel = NULL; + break; + } + + *tmp = *pos; + if (tmp->free_str) { + tmp->val.str = strdup(pos->val.str); + if (tmp->val.str == NULL) { + evsel__delete(evsel); + evsel = NULL; + free(tmp); + break; + } + } + list_add_tail(>list, >config_terms); + } + + return evsel; +} + /* * Returns pointer with encoded error via interface. */ diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h index 35e3f6d66085..507c31d6a389 100644 --- a/tools/perf/util/evsel.h +++ b/tools/perf/util/evsel.h @@ -169,6 +169,7 @@ static inline struct evsel *evsel__new(struct perf_event_attr *attr) return evsel__new_idx(attr, 0); } +struct evsel *evsel__clone(struct evsel *orig); struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx); /* -- 2.28.0.526.ge36021eeef-goog
[PATCH 2/4] perf stat: Add --multiply-cgroup option
The --multiply-cgroup option is a syntax sugar to monitor large number of cgroups easily. Current command line requires to list all the events and cgroups even if users want to monitor same events for each cgroup. This patch addresses that usage by copying given events for each cgroup on user's behalf. For instance, if they want to monitor 6 events for 200 cgroups each they should write 1200 event names (with -e) AND 1200 cgroup names (with -G) on the command line. But with this change, they can just specify 6 events and 200 cgroups plus one more option. A simpler example below: It wants to measure 3 events for 2 cgroups ('a' and 'b'). The result is that total 6 events are counted like below. $ ./perf stat -a -e cpu-clock,cycles,instructions --multiply-cgroup -G a,b sleep 1 Performance counter stats for 'system wide': 988.18 msec cpu-clock a #0.987 CPUs utilized 3,153,761,702 cyclesa #3.200 GHz (100.00%) 8,067,769,847 instructions a #2.57 insn per cycle (100.00%) 982.71 msec cpu-clock b #0.982 CPUs utilized 3,136,093,298 cyclesb #3.182 GHz (99.99%) 8,109,619,327 instructions b #2.58 insn per cycle (99.99%) 1.001228054 seconds time elapsed Signed-off-by: Namhyung Kim --- tools/perf/builtin-stat.c | 20 +- tools/perf/util/cgroup.c | 78 +++ tools/perf/util/cgroup.h | 2 + tools/perf/util/stat.h| 1 + 4 files changed, 100 insertions(+), 1 deletion(-) diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 483a28ef4ec4..6724d23ce2e7 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -1068,6 +1068,16 @@ static int parse_control_option(const struct option *opt, return 0; } +static int parse_stat_cgroups(const struct option *opt, + const char *str, int unset) +{ + stat_config.cgroups = strdup(str); + if (!stat_config.cgroups) + return -1; + + return parse_cgroups(opt, str, unset); +} + static struct option stat_options[] = { OPT_BOOLEAN('T', "transaction", _run, "hardware transaction statistics"), @@ -,7 +1121,9 @@ static struct option stat_options[] = { OPT_STRING('x', "field-separator", _config.csv_sep, "separator", "print counts with custom separator"), OPT_CALLBACK('G', "cgroup", _list, "name", -"monitor event in cgroup name only", parse_cgroups), +"monitor event in cgroup name only", parse_stat_cgroups), + OPT_BOOLEAN(0, "multiply-cgroup", _cgroup, + "multiply the event list by cgroups"), OPT_STRING('o', "output", _name, "file", "output file name"), OPT_BOOLEAN(0, "append", _file, "append to the output file"), OPT_INTEGER(0, "log-fd", _fd, @@ -2248,6 +2260,11 @@ int cmd_stat(int argc, const char **argv) if (add_default_attributes()) goto out; + if (multiply_cgroup && stat_config.cgroups) { + if (evlist__multiply_cgroup(evsel_list, stat_config.cgroups) < 0) + goto out; + } + target__validate(); if ((stat_config.aggr_mode == AGGR_THREAD) && (target.system_wide)) @@ -2412,6 +2429,7 @@ int cmd_stat(int argc, const char **argv) evlist__delete(evsel_list); + free(stat_config.cgroups); metricgroup__rblist_exit(_config.metric_events); runtime_stat_delete(_config); diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c index 050dea9f1e88..4892f9496bc2 100644 --- a/tools/perf/util/cgroup.c +++ b/tools/perf/util/cgroup.c @@ -12,6 +12,7 @@ #include int nr_cgroups; +bool multiply_cgroup; static int open_cgroup(const char *name) { @@ -156,6 +157,10 @@ int parse_cgroups(const struct option *opt, const char *str, return -1; } + /* delay processing cgroups after it sees all events */ + if (multiply_cgroup) + return 0; + for (;;) { p = strchr(str, ','); e = p ? p : eos; @@ -193,6 +198,79 @@ int parse_cgroups(const struct option *opt, const char *str, return 0; } +int evlist__multiply_cgroup(struct evlist *evlist, const char *str) +{ + struct evlist *orig_list, *tmp_list; + struct evsel *pos, *evsel, *leader; + struct cgroup *cgrp = NULL; + const char *p, *e, *eos = str + strlen(str); + int ret = -1; + + if (evlist->core.nr_entries == 0) { + fprintf(stderr, "must define events before cgroups\n"); + return -EINVAL; + } + + orig_list = evlist__new(); + tmp_list = evlist__new(); + if
Re: [PATCH] seccomp: kill process instead of thread for unknown actions
On Mon, Aug 31, 2020 at 12:37 PM Kees Cook wrote: > > On Fri, Aug 28, 2020 at 09:56:13PM -0400, Rich Felker wrote: > > Asynchronous termination of a thread outside of the userspace thread > > library's knowledge is an unsafe operation that leaves the process in > > an inconsistent, corrupt, and possibly unrecoverable state. In order > > to make new actions that may be added in the future safe on kernels > > not aware of them, change the default action from > > SECCOMP_RET_KILL_THREAD to SECCOMP_RET_KILL_PROCESS. > > > > Signed-off-by: Rich Felker > > --- > > > > This fundamental problem with SECCOMP_RET_KILL_THREAD, and that it > > should be considered unsafe and deprecated, was recently noted/fixed > > seccomp in the man page and its example. Here I've only changed the > > default action for new/unknown action codes. Ideally the behavior for > > strict seccomp mode would be changed too but I think that breaks > > stability policy; in any case it's less likely to be an issue since > > strict mode is hard or impossible to use reasonably in a multithreaded > > process. > > > > Unfortunately changing this now won't help older kernels where unknown > > new actions would still be handled unsafely, but at least it makes it > > so the problem will fade away over time. > > I think this is probably fine to change now. I'd always wanted to > "upgrade" the default to KILL_PROCESS, but wanted to wait for > KILL_PROCESS to exist at all for a while first. :) > > I'm not aware of any filter generators (e.g. libseccomp, Chrome) that > depend on unknown filter return values to cause a KILL_THREAD, and > everything I've seen indicates that they aren't _accidentally_ depending > on it either (i.e. they both produce "valid" filters). It's possible > that something out there doesn't, and in that case, we likely need to > make a special case for whatever bad filter value it chose, but we can > cross that bridge when we come to it. > > I've added Kyle and Robert to CC as well, as they have noticed subtle > changes to seccomp behavior in the past. I *think* this change should be > fine, but perhaps they will see something I don't. :) I can't think of anything here that would break stuff, though I do believe rr needs some changes to how it handles this (I don't think our current behavior is an accurate emulation of the kernel). - Kyle > > > > kernel/seccomp.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/seccomp.c b/kernel/seccomp.c > > index d653d8426de9..ce1875fa6b39 100644 > > --- a/kernel/seccomp.c > > +++ b/kernel/seccomp.c > > @@ -910,10 +910,10 @@ static int __seccomp_filter(int this_syscall, const > > struct seccomp_data *sd, > > seccomp_init_siginfo(, this_syscall, data); > > do_coredump(); > > } > > - if (action == SECCOMP_RET_KILL_PROCESS) > > - do_group_exit(SIGSYS); > > - else > > + if (action == SECCOMP_RET_KILL_THREAD) > > do_exit(SIGSYS); > > + else > > + do_group_exit(SIGSYS); > > I need to think a little more, but I suspect we should change the coredump > logic (above the quoted code) too... (i.e. "action == > SECCOMP_RET_KILL_PROCESS" > -> "action != SECCOMP_RET_KILL_THREAD") > > > } > > > > unreachable(); > > -- > > 2.21.0 > > > > Thanks! > > -Kees > > -- > Kees Cook
[PATCH v2] kbuild: preprocess module linker script
There was a request to preprocess the module linker script like we do for the vmlinux one. (https://lkml.org/lkml/2020/8/21/512) The difference between vmlinux.lds and module.lds is that the latter is needed for external module builds, thus must be cleaned up by 'make mrproper' instead of 'make clean'. Also, it must be created by 'make modules_prepare'. You cannot put it in arch/$(SRCARCH)/kernel/, which is cleaned up by 'make clean'. I moved arch/$(SRCARCH)/kernel/module.lds to arch/$(SRCARCH)/include/asm/module.lds.h, which is included from scripts/module.lds.S. scripts/module.lds is fine because 'make clean' keeps all the build artifacts under scripts/. You can add arch-specific sections in . Signed-off-by: Masahiro Yamada Tested-by: Jessica Yu Acked-by: Will Deacon --- Changes in v2: - Fix the race between the two targets 'scripts' and 'asm-generic' Makefile | 10 ++ arch/arm/Makefile | 4 .../{kernel/module.lds => include/asm/module.lds.h}| 2 ++ arch/arm64/Makefile| 4 .../{kernel/module.lds => include/asm/module.lds.h}| 2 ++ arch/ia64/Makefile | 1 - arch/ia64/{module.lds => include/asm/module.lds.h} | 0 arch/m68k/Makefile | 1 - .../{kernel/module.lds => include/asm/module.lds.h}| 0 arch/powerpc/Makefile | 1 - .../{kernel/module.lds => include/asm/module.lds.h}| 0 arch/riscv/Makefile| 3 --- .../{kernel/module.lds => include/asm/module.lds.h}| 3 ++- arch/um/include/asm/Kbuild | 1 + include/asm-generic/Kbuild | 1 + include/asm-generic/module.lds.h | 10 ++ scripts/.gitignore | 1 + scripts/Makefile | 3 +++ scripts/Makefile.modfinal | 5 ++--- scripts/{module-common.lds => module.lds.S}| 3 +++ scripts/package/builddeb | 2 +- 21 files changed, 34 insertions(+), 23 deletions(-) rename arch/arm/{kernel/module.lds => include/asm/module.lds.h} (72%) rename arch/arm64/{kernel/module.lds => include/asm/module.lds.h} (76%) rename arch/ia64/{module.lds => include/asm/module.lds.h} (100%) rename arch/m68k/{kernel/module.lds => include/asm/module.lds.h} (100%) rename arch/powerpc/{kernel/module.lds => include/asm/module.lds.h} (100%) rename arch/riscv/{kernel/module.lds => include/asm/module.lds.h} (84%) create mode 100644 include/asm-generic/module.lds.h rename scripts/{module-common.lds => module.lds.S} (93%) diff --git a/Makefile b/Makefile index 37739ee53f27..97b1dae1783b 100644 --- a/Makefile +++ b/Makefile @@ -505,7 +505,6 @@ KBUILD_CFLAGS_KERNEL := KBUILD_AFLAGS_MODULE := -DMODULE KBUILD_CFLAGS_MODULE := -DMODULE KBUILD_LDFLAGS_MODULE := -export KBUILD_LDS_MODULE := $(srctree)/scripts/module-common.lds KBUILD_LDFLAGS := CLANG_FLAGS := @@ -1395,7 +1394,7 @@ endif # using awk while concatenating to the final file. PHONY += modules -modules: $(if $(KBUILD_BUILTIN),vmlinux) modules_check +modules: $(if $(KBUILD_BUILTIN),vmlinux) modules_check modules_prepare $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost PHONY += modules_check @@ -1412,6 +1411,7 @@ targets += modules.order # Target to prepare building external modules PHONY += modules_prepare modules_prepare: prepare + $(Q)$(MAKE) $(build)=scripts scripts/module.lds # Target to install modules PHONY += modules_install @@ -1743,7 +1743,9 @@ help: @echo ' clean - remove generated files in module directory only' @echo '' -PHONY += prepare +# no-op for external module builds +PHONY += prepare modules_prepare + endif # KBUILD_EXTMOD # Single targets @@ -1776,7 +1778,7 @@ MODORDER := .modules.tmp endif PHONY += single_modpost -single_modpost: $(single-no-ko) +single_modpost: $(single-no-ko) modules_prepare $(Q){ $(foreach m, $(single-ko), echo $(extmod-prefix)$m;) } > $(MODORDER) $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 4e877354515f..a0cb15de9677 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -16,10 +16,6 @@ LDFLAGS_vmlinux += --be8 KBUILD_LDFLAGS_MODULE += --be8 endif -ifeq ($(CONFIG_ARM_MODULE_PLTS),y) -KBUILD_LDS_MODULE += $(srctree)/arch/arm/kernel/module.lds -endif - GZFLAGS:=-9 #KBUILD_CFLAGS +=-pipe diff --git a/arch/arm/kernel/module.lds b/arch/arm/include/asm/module.lds.h similarity index 72% rename from arch/arm/kernel/module.lds rename to arch/arm/include/asm/module.lds.h index 79cb6af565e5..0e7cb4e314b4 100644 --- a/arch/arm/kernel/module.lds +++
Re: [PATCH] EDAC: sb_edac: simplify switch statement
On Mon, Sep 07, 2020 at 08:32:25AM -0700, t...@redhat.com wrote: > From: Tom Rix > > clang static analyzer reports this problem > > sb_edac.c:959:2: warning: Undefined or garbage value > returned to caller > return type; > ^~~ > > This is a false positive. > > However by initializing the type to DEV_UNKNOWN the 3 case can be > removed from the switch, saving a comparison and jump. > > Signed-off-by: Tom Rix Some maintainers have a preference for a default case statement but presumably this case statement has been cutting it as is so: Reviewed-by: Nathan Chancellor > --- > drivers/edac/sb_edac.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > index 68f2fe4df333..93daa4297f2e 100644 > --- a/drivers/edac/sb_edac.c > +++ b/drivers/edac/sb_edac.c > @@ -939,12 +939,9 @@ static enum dev_type sbridge_get_width(struct > sbridge_pvt *pvt, u32 mtr) > > static enum dev_type __ibridge_get_width(u32 mtr) > { > - enum dev_type type; > + enum dev_type type = DEV_UNKNOWN; > > switch (mtr) { > - case 3: > - type = DEV_UNKNOWN; > - break; > case 2: > type = DEV_X16; > break; > -- > 2.18.1 > > -- > You received this message because you are subscribed to the Google Groups > "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to clang-built-linux+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/clang-built-linux/20200907153225.7294-1-trix%40redhat.com.
Re: [PATCH] ath11k: fix a double free and a memory leak
On Sun, Sep 06, 2020 at 02:26:25PM -0700, t...@redhat.com wrote: > From: Tom Rix > > clang static analyzer reports this problem > > mac.c:6204:2: warning: Attempt to free released memory > kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels); > ^ > > The channels pointer is allocated in ath11k_mac_setup_channels_rates() > When it fails midway, it cleans up the memory it has already allocated. > So the error handling needs to skip freeing the memory. > > There is a second problem. > ath11k_mac_setup_channels_rates(), allocates 3 channels. err_free > misses releasing ar->mac.sbands[NL80211_BAND_6GHZ].channels > > Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") > Signed-off-by: Tom Rix Reviewed-by: Nathan Chancellor > --- > drivers/net/wireless/ath/ath11k/mac.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/mac.c > b/drivers/net/wireless/ath/ath11k/mac.c > index f4a085baff38..f1a964b01a83 100644 > --- a/drivers/net/wireless/ath/ath11k/mac.c > +++ b/drivers/net/wireless/ath/ath11k/mac.c > @@ -6089,7 +6089,7 @@ static int __ath11k_mac_register(struct ath11k *ar) > ret = ath11k_mac_setup_channels_rates(ar, > cap->supported_bands); > if (ret) > - goto err_free; > + goto err; > > ath11k_mac_setup_ht_vht_cap(ar, cap, _cap); > ath11k_mac_setup_he_cap(ar, cap); > @@ -6203,7 +6203,8 @@ static int __ath11k_mac_register(struct ath11k *ar) > err_free: > kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels); > kfree(ar->mac.sbands[NL80211_BAND_5GHZ].channels); > - > + kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels); > +err: > SET_IEEE80211_DEV(ar->hw, NULL); > return ret; > } > -- > 2.18.1 >
Re: [PATCH 1/6] phy: phy-bcm-ns-usb3: convert to readl_poll_timeout_atomic()
On 25-08-20, 10:03, Chunfeng Yun wrote: > Use readl_poll_timeout_atomic() to simplify code Applied all, thanks -- ~Vinod
Re: [PATCH] mwifiex: remove function pointer check
On Sun, Sep 06, 2020 at 01:05:48PM -0700, t...@redhat.com wrote: > From: Tom Rix > > clang static analyzer reports this problem > > init.c:739:8: warning: Called function pointer > is null (null dereference) > ret = adapter->if_ops.check_fw_status( ... > ^ > > In mwifiex_dnld_fw, there is an earlier check for check_fw_status(), > The check was introduced for usb support at the same time this > check in _mwifiex_fw_dpc() was made > > if (adapter->if_ops.dnld_fw) { > ret = adapter->if_ops.dnld_fw(adapter, ); > } else { > ret = mwifiex_dnld_fw(adapter, ); > } > > And a dnld_fw function initialized as part the usb's > mwifiex_if_ops. > > The other instances of mwifiex_if_ops for pci and sdio > both set check_fw_status. > > So the first check is not needed and can be removed. > > Fixes: 4daffe354366 ("mwifiex: add support for Marvell USB8797 chipset") > Signed-off-by: Tom Rix Indeed, on the surface, mwifiex_dnld_fw assumes that check_fw_status() cannot be NULL because it will always be called at the end of the function even if the first check is skipped. Reviewed-by: Nathan Chancellor > --- > drivers/net/wireless/marvell/mwifiex/init.c | 14 ++ > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/wireless/marvell/mwifiex/init.c > b/drivers/net/wireless/marvell/mwifiex/init.c > index 82d69bc3aaaf..f006a3d72b40 100644 > --- a/drivers/net/wireless/marvell/mwifiex/init.c > +++ b/drivers/net/wireless/marvell/mwifiex/init.c > @@ -695,14 +695,12 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter, > int ret; > u32 poll_num = 1; > > - if (adapter->if_ops.check_fw_status) { > - /* check if firmware is already running */ > - ret = adapter->if_ops.check_fw_status(adapter, poll_num); > - if (!ret) { > - mwifiex_dbg(adapter, MSG, > - "WLAN FW already running! Skip FW dnld\n"); > - return 0; > - } > + /* check if firmware is already running */ > + ret = adapter->if_ops.check_fw_status(adapter, poll_num); > + if (!ret) { > + mwifiex_dbg(adapter, MSG, > + "WLAN FW already running! Skip FW dnld\n"); > + return 0; > } > > /* check if we are the winner for downloading FW */ > -- > 2.18.1 > > -- > You received this message because you are subscribed to the Google Groups > "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to clang-built-linux+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/clang-built-linux/20200906200548.18053-1-trix%40redhat.com.
Lieber Freund (Assalamu Alaikum),?
-- Lieber Freund (Assalamu Alaikum), Ich bin vor einer privaten Suche auf Ihren E-Mail-Kontakt gestoßen Ihre Hilfe. Mein Name ist Aisha Al-Qaddafi, eine alleinerziehende Mutter und eine Witwe mit drei Kindern. Ich bin die einzige leibliche Tochter des Spätlibyschen Präsident (verstorbener Oberst Muammar Gaddafi). Ich habe Investmentfonds im Wert von siebenundzwanzig Millionen fünfhunderttausend United State Dollar ($ 27.500.000.00) und ich brauche eine vertrauenswürdige Investition Manager / Partner aufgrund meines aktuellen Flüchtlingsstatus bin ich jedoch Möglicherweise interessieren Sie sich für die Unterstützung von Investitionsprojekten in Ihrem Land Von dort aus können wir in naher Zukunft Geschäftsbeziehungen aufbauen. Ich bin bereit, mit Ihnen über das Verhältnis zwischen Investition und Unternehmensgewinn zu verhandeln Basis für die zukünftige Investition Gewinne zu erzielen. Wenn Sie bereit sind, dieses Projekt in meinem Namen zu bearbeiten, antworten Sie bitte dringend Damit ich Ihnen mehr Informationen über die Investmentfonds geben kann. Ihre dringende Antwort wird geschätzt. schreibe mir an diese email adresse ( ayishagdda...@mail.ru ) zur weiteren Diskussion. Freundliche Grüße Frau Aisha Al-Qaddafi
linux-next: build warning after merge of the drm-misc tree
Hi all, After merging the drm-misc tree, today's linux-next build (x86_64 allmodconfig) produced this warning: WARNING: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/panel/panel-samsung-s6e63m0.o Introduced by commit b7b23e447687 ("drm/panel: s6e63m0: Break out SPI transport") -- Cheers, Stephen Rothwell pgpZwOVv9fR6h.pgp Description: OpenPGP digital signature
Re: [PATCH net v2] hv_netvsc: Fix hibernation for mlx5 VF driver
On Mon, 7 Sep 2020 00:13:39 -0700 Dexuan Cui wrote: > mlx5_suspend()/resume() keep the network interface, so during hibernation > netvsc_unregister_vf() and netvsc_register_vf() are not called, and hence > netvsc_resume() should call netvsc_vf_changed() to switch the data path > back to the VF after hibernation. Note: after we close and re-open the > vmbus channel of the netvsc NIC in netvsc_suspend() and netvsc_resume(), > the data path is implicitly switched to the netvsc NIC. Similarly, > netvsc_suspend() should not call netvsc_unregister_vf(), otherwise the VF > can no longer be used after hibernation. > > For mlx4, since the VF network interafce is explicitly destroyed and > re-created during hibernation (see mlx4_suspend()/resume()), hv_netvsc > already explicitly switches the data path from and to the VF automatically > via netvsc_register_vf() and netvsc_unregister_vf(), so mlx4 doesn't need > this fix. Note: mlx4 can still work with the fix because in > netvsc_suspend()/resume() ndev_ctx->vf_netdev is NULL for mlx4. > > Fixes: 0efeea5fb153 ("hv_netvsc: Add the support of hibernation") > Signed-off-by: Dexuan Cui Applied, thanks!
linux-next: manual merge of the drm-intel tree with Linus' tree
Hi all, Today's linux-next merge of the drm-intel tree got a conflict in: drivers/gpu/drm/i915/display/intel_panel.c between commit: f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external PWM controller") from Linus' tree and commit: 6b51e7d23aa8 ("drm/i915: panel: Honor the VBT PWM frequency for devs with an external PWM controller") from the drm-intel tree. I fixed it up (I just used the latter) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell pgp1s1y9mf56S.pgp Description: OpenPGP digital signature
Re: gcc crashes with general protection faults in 5.9.0-rc3-00091-ge28f0104343d
Following up my yesterdays mail: This is 5.9.0-rc3-00091-ge28f0104343d on Lenovo t460s that has ran fine up to 5.8.0. Today I tried reproducing my linking problem with git kernel on my laptop and got segmentation faults in gcc. This is probably the corresponding dmesg part: 0xdead0400 loks like some kind of poisoning. [307299.392045] general protection fault, probably for non-canonical address 0xdead0400: [#1] SMP PTI Was not reproducible in 5.9-rc4 while recompiling the kernel in a loop for 8 hours. -- Meelis Roos
[net-next] net: smsc911x: Remove unused variables
Fixes the following W=1 kernel build warning(s): drivers/net/ethernet/smsc/smsc911x.c: In function ‘smsc911x_rx_fastforward’: drivers/net/ethernet/smsc/smsc911x.c:1199:16: warning: variable ‘temp’ set but not used [-Wunused-but-set-variable] drivers/net/ethernet/smsc/smsc911x.c: In function ‘smsc911x_eeprom_write_location’: drivers/net/ethernet/smsc/smsc911x.c:2058:6: warning: variable ‘temp’ set but not used [-Wunused-but-set-variable] Signed-off-by: Wei Xu --- drivers/net/ethernet/smsc/smsc911x.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index fc168f8..823d9a7 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -1196,9 +1196,8 @@ smsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int pktwords) SMSC_WARN(pdata, hw, "Timed out waiting for " "RX FFWD to finish, RX_DP_CTRL: 0x%08X", val); } else { - unsigned int temp; while (pktwords--) - temp = smsc911x_reg_read(pdata, RX_DATA_FIFO); + smsc911x_reg_read(pdata, RX_DATA_FIFO); } } @@ -2055,7 +2054,6 @@ static int smsc911x_eeprom_write_location(struct smsc911x_data *pdata, u8 address, u8 data) { u32 op = E2P_CMD_EPC_CMD_ERASE_ | address; - u32 temp; int ret; SMSC_TRACE(pdata, drv, "address 0x%x, data 0x%x", address, data); @@ -2066,7 +2064,7 @@ static int smsc911x_eeprom_write_location(struct smsc911x_data *pdata, smsc911x_reg_write(pdata, E2P_DATA, (u32)data); /* Workaround for hardware read-after-write restriction */ - temp = smsc911x_reg_read(pdata, BYTE_TEST); + smsc911x_reg_read(pdata, BYTE_TEST); ret = smsc911x_eeprom_send_cmd(pdata, op); } -- 2.8.1
Re: linux-next: build warning after merge of the net-next tree
On Tue, 8 Sep 2020 13:00:00 +1000 Stephen Rothwell wrote: > Hi all, > > After merging the net-next tree, today's linux-next build (powerpc > ppc64_defconfig) produced this warning: > > net/bridge/br_multicast.c: In function 'br_multicast_find_port': > net/bridge/br_multicast.c:1818:21: warning: unused variable 'br' > [-Wunused-variable] > 1818 | struct net_bridge *br = mp->br; > | ^~ > > Introduced by commit > > 0436862e417e ("net: bridge: mcast: support for IGMPv3/MLDv2 > ALLOW_NEW_SOURCES report") > > Maybe turning mlock_dereference into a static inline function would help. Or perhaps provide a better definition of whatever is making the reference disappear? RCU_LOCKDEP_WARN()? Thanks for the report!
[PATCH 2/2] venus: core: vote for video-mem icc path and change avg, peak bw
Currently we are voting for venus0-ebi path during buffer processing with an average bandwidth of all the instances and unvoting during session release. While video streaming when we try to do XO-SD using the command "echo mem > /sys/power/state command" , device is not entering to suspend state and from interconnect summary seeing votes for venus0-ebi Corrected this by voting for venus0-ebi path in venus_runtime_resume and unvote during venus_runtime_suspend. Signed-off-by: Mansur Alisha Shaik --- drivers/media/platform/qcom/venus/core.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index 4857bbd..79d8600 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -373,6 +373,10 @@ static __maybe_unused int venus_runtime_suspend(struct device *dev) if (ret) return ret; + ret = icc_set_bw(core->video_path, 0, 0); + if (ret) + return ret; + return ret; } @@ -382,7 +386,11 @@ static __maybe_unused int venus_runtime_resume(struct device *dev) const struct venus_pm_ops *pm_ops = core->pm_ops; int ret; - ret = icc_set_bw(core->cpucfg_path, 0, kbps_to_icc(1000)); + ret = icc_set_bw(core->video_path, kbps_to_icc(2), 0); + if (ret) + return ret; + + ret = icc_set_bw(core->cpucfg_path, kbps_to_icc(1000), 0); if (ret) return ret; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 1/2] venus: core: change clk enable and disable order in resume and suspend
Currently video driver is voting after clk enable and un voting before clk disable. Basically we should vote before clk enable and un vote after clk disable. Corrected this by changing the order of clk enable and clk disable. Signed-off-by: Mansur Alisha Shaik --- drivers/media/platform/qcom/venus/core.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index c5af428..4857bbd 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -363,13 +363,16 @@ static __maybe_unused int venus_runtime_suspend(struct device *dev) if (ret) return ret; + if (pm_ops->core_power) { + ret = pm_ops->core_power(dev, POWER_OFF); + if (ret) + return ret; + } + ret = icc_set_bw(core->cpucfg_path, 0, 0); if (ret) return ret; - if (pm_ops->core_power) - ret = pm_ops->core_power(dev, POWER_OFF); - return ret; } @@ -379,16 +382,16 @@ static __maybe_unused int venus_runtime_resume(struct device *dev) const struct venus_pm_ops *pm_ops = core->pm_ops; int ret; + ret = icc_set_bw(core->cpucfg_path, 0, kbps_to_icc(1000)); + if (ret) + return ret; + if (pm_ops->core_power) { ret = pm_ops->core_power(dev, POWER_ON); if (ret) return ret; } - ret = icc_set_bw(core->cpucfg_path, 0, kbps_to_icc(1000)); - if (ret) - return ret; - return hfi_core_resume(core, false); } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 0/2] Venus - change clk enable, disable order and change bw values
The intention of this patchset is to correct clock enable and disable order and vote for venus-ebi and cpucfg paths with average bandwidht instad of peakbandwidht since with current implementation we are seeing "video_cc_venus_ctl_axi_clk status stuck at 'off' " warnings and XO-SD failures while streaming. Mansur Alisha Shaik (2): venus: core: change clk enable and disable order in resume and suspend venus: core: vote for video-mem icc path and change avg, peak bw drivers/media/platform/qcom/venus/core.c | 23 +-- 1 file changed, 17 insertions(+), 6 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [patch V2 00/46] x86, PCI, XEN, genirq ...: Prepare for device MSI
On Wed, Aug 26, 2020 at 01:16:28PM +0200, Thomas Gleixner wrote: > This is the second version of providing a base to support device MSI (non > PCI based) and on top of that support for IMS (Interrupt Message Storm) > based devices in a halfways architecture independent way. Booted with quick testing on a 32 socket, 1536 CPU, 12 TB memory Cascade Lake system and a 8 socket, 144 CPU, 3 TB memory Cooper Lake system without any obvious regression. -- Russ Anderson, SuperDome Flex Linux Kernel Group Manager HPE - Hewlett Packard Enterprise (formerly SGI) r...@hpe.com
Re: [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is active
Am 2020-09-06 um 12:08 p.m. schrieb Deucher, Alexander: > [AMD Official Use Only - Internal Distribution Only] > >> -Original Message- >> From: Joerg Roedel >> Sent: Friday, September 4, 2020 6:06 AM >> To: Deucher, Alexander >> Cc: jroe...@suse.de; Kuehling, Felix ; >> io...@lists.linux-foundation.org; Huang, Ray ; >> Koenig, Christian ; Lendacky, Thomas >> ; Suthikulpanit, Suravee >> ; linux-kernel@vger.kernel.org >> Subject: Re: [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is >> active >> >> On Fri, Aug 28, 2020 at 03:47:07PM +, Deucher, Alexander wrote: >>> Ah, right, So CZ and ST are not an issue. Raven is paired with Zen based >> CPUs. >> >> Okay, so for the Raven case, can you add code to the amdgpu driver which >> makes it fail to initialize on Raven when SME is active? There is a global >> checking function for that, so that shouldn't be hard to do. >> > Sure. How about the attached patch? The patch is Acked-by: Felix Kuehling Thanks, Felix > > Alex >
linux-next: build failure after merge of the nand tree
Hi all, After merging the nand tree, today's linux-next build (arm multi_v7_defconfig) failed like this: drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c: In function 'common_nfc_set_geometry': drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c:513:33: error: 'chip' undeclared (first use in this function) 513 | nanddev_get_ecc_requirements(>base); | ^~~~ drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c:513:33: note: each undeclared identifier is reported only once for each function it appears in Caused by commit aa5faaa5f95c ("mtd: rawnand: Use nanddev_get/set_ecc_requirements() when relevant") I have used the nand tree from next-20200903 for today. -- Cheers, Stephen Rothwell pgpeKNLZq1p88.pgp Description: OpenPGP digital signature
Re: [PATCH 1/1] watchdog: remove unneeded inclusion of
On 2020/9/8 10:40, Guenter Roeck wrote: > On 9/7/20 12:50 AM, Leizhen (ThunderTown) wrote: >> Hi, Wim Van Sebroeck, Guenter Roeck: >> What's your opinion? Guenter Roeck given "Reviewed-by" two weeks ago. >> > > The patch is in my watchdog-next branch, and Wim usually picks it up > from there. Oh, thanks. > > Guenter > >> >> On 2020/8/27 21:40, Guenter Roeck wrote: >>> On 8/26/20 11:21 PM, Zhen Lei wrote: There has been no reference to "struct sched_param" since commit 94beddacb53c ("sched,watchdog: Convert to sched_set_fifo()"), so there's no need to include any more, delete it. Signed-off-by: Zhen Lei >>> >>> Reviewed-by: Guenter Roeck >>> --- drivers/watchdog/watchdog_dev.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index 6798addabd5a067..0f18fa2433310b0 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c @@ -43,8 +43,6 @@ #include/* For watchdog specific items */ #include /* For copy_to_user/put_user/... */ -#include/* For struct sched_param */ - #include "watchdog_core.h" #include "watchdog_pretimeout.h" >>> >>> >>> >> > > >
Re: [PATCH v12 1/9] x86: kdump: move CRASH_ALIGN to 2M
On 2020/9/8 9:21, Dave Young wrote: > Hi, > > On 09/07/20 at 09:47pm, Chen Zhou wrote: >> CONFIG_PHYSICAL_ALIGN can be selected from 2M to 16M and default >> value is 2M, so move CRASH_ALIGN to 2M, with smaller value reservation >> can have more chance to succeed. > Seems still some misunderstanding about the change :( I'm sorry if I > did not explain it clearly. > > Previously I missed the PHYSICAL_ALIGN can change according to .config > I mean we should change the value to CONFIG_PHYSICAL_ALIGN for X86 > And I suggest to move back to keep using 16M. And do not change it in > this series. Hi Dave, Sorry, i misunderstood about this. Ok, this patch will keep the value of CRASH_ALIGN as it is, just move CRASH_ALIGN to header asm/kexec.h and replace the hard-coded alignment with macro CRASH_ALIGN in function reserve_crashkernel(). Thanks, Chen Zhou > >> And replace the hard-coded alignment with macro CRASH_ALIGN in function >> reserve_crashkernel(). >> >> Suggested-by: Dave Young >> Signed-off-by: Chen Zhou >> --- >> arch/x86/include/asm/kexec.h | 3 +++ >> arch/x86/kernel/setup.c | 5 + >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h >> index 6802c59e8252..83f200dd54a1 100644 >> --- a/arch/x86/include/asm/kexec.h >> +++ b/arch/x86/include/asm/kexec.h >> @@ -18,6 +18,9 @@ >> >> # define KEXEC_CONTROL_CODE_MAX_SIZE2048 >> >> +/* 2M alignment for crash kernel regions */ >> +#define CRASH_ALIGN SZ_2M >> + >> #ifndef __ASSEMBLY__ >> >> #include >> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >> index 3511736fbc74..296294ad0dd8 100644 >> --- a/arch/x86/kernel/setup.c >> +++ b/arch/x86/kernel/setup.c >> @@ -402,9 +402,6 @@ static void __init >> memblock_x86_reserve_range_setup_data(void) >> >> #ifdef CONFIG_KEXEC_CORE >> >> -/* 16M alignment for crash kernel regions */ >> -#define CRASH_ALIGN SZ_16M >> - >> /* >> * Keep the crash kernel below this limit. >> * >> @@ -530,7 +527,7 @@ static void __init reserve_crashkernel(void) >> >> start = memblock_find_in_range(crash_base, >> crash_base + crash_size, >> - crash_size, 1 << 20); >> + crash_size, CRASH_ALIGN); >> if (start != crash_base) { >> pr_info("crashkernel reservation failed - memory is in >> use.\n"); >> return; >> -- >> 2.20.1 >> > Thanks > Dave > > > . >
Re: [PATCH v1 02/10] powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE on iommu_*_coherent()
On 04/09/2020 16:04, Leonardo Bras wrote: On Thu, 2020-09-03 at 14:41 +1000, Alexey Kardashevskiy wrote: I am new to this, so I am trying to understand how a memory page mapped as DMA, and used for something else could be a problem. From the device prospective, there is PCI space and everything from 0 till 1<<64 is accessible and what is that mapped to - the device does not know. PHB's IOMMU is the thing to notice invalid access and raise EEH but PHB only knows about PCI->physical memory mapping (with IOMMU pages) but nothing about the host kernel pages. Does this help? Thanks, According to our conversation on Slack: 1- There is a problem if a hypervisor gives to it's VMs contiguous memory blocks that are not aligned to IOMMU pages, because then an iommu_map_page() could map some memory in this VM and some memory in other VM / process. 2- To guarantee this, we should have system pagesize >= iommu_pagesize One way to get (2) is by doing this in enable_ddw(): if ((query.page_size & 4) && PAGE_SHIFT >= 24) { You won't ever (well, soon) see PAGE_SHIFT==24, it is either 4K or 64K. However 16MB IOMMU pages is fine - if hypervisor uses huge pages for VMs RAM, it also then advertises huge IOMMU pages in ddw-query. So for the 1:1 case there must be no "PAGE_SHIFT >= 24". page_shift = 24; /* 16MB */ } else if ((query.page_size & 2) && PAGE_SHIFT >= 16 ) { page_shift = 16; /* 64kB */ } else if (query.page_size & 1 && PAGE_SHIFT >= 12) { page_shift = 12; /* 4kB */ [...] Another way of solving this, would be adding in LoPAR documentation that the blocksize of contiguous memory the hypervisor gives a VM should always be aligned to IOMMU pagesize offered. I think this is assumed already by the design of the DDW API. I think the best approach would be first sending the above patch, which is faster, and then get working into adding that to documentation, so hypervisors guarantee this. If this gets into the docs, we can revert the patch. What do you think? I think we diverted from the original patch :) I am not quite sure what you were fixing there. Thanks, -- Alexey
Re: [PATCH v2] Revert "ALSA: hda: Add support for Loongson 7A1000 controller"
On 09/08/2020 08:37 AM, Huacai Chen wrote: Hi, all This patch should be backported to 5.4. Hi, Commit 61eee4a7fc40 ("ALSA: hda: Add support for Loongson 7A1000 controller") has been not yet merged into 5.4, so no need to backport. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/pci/hda/hda_intel.c?h=v5.4 Thanks, Tiezhu Huacai On Tue, Aug 25, 2020 at 6:03 PM Takashi Iwai wrote: On Tue, 25 Aug 2020 11:39:48 +0200, Tiezhu Yang wrote: This reverts commit 61eee4a7fc40 ("ALSA: hda: Add support for Loongson 7A1000 controller") to fix the following error on the Loongson LS7A platform: rcu: INFO: rcu_preempt self-detected stall on CPU NMI backtrace for cpu 0 CPU: 0 PID: 68 Comm: kworker/0:2 Not tainted 5.8.0+ #3 Hardware name: , BIOS Workqueue: events azx_probe_work [snd_hda_intel] Call Trace: [] show_stack+0x9c/0x130 [] dump_stack+0xb0/0xf0 [] nmi_cpu_backtrace+0x134/0x140 [] nmi_trigger_cpumask_backtrace+0x190/0x200 [] rcu_dump_cpu_stacks+0x12c/0x190 [] rcu_sched_clock_irq+0xa2c/0xfc8 [] update_process_times+0x2c/0xb8 [] tick_sched_timer+0x40/0xb8 [] __hrtimer_run_queues+0x118/0x1d0 [] hrtimer_interrupt+0x12c/0x2d8 [] c0_compare_interrupt+0x74/0xa0 [] __handle_irq_event_percpu+0xa8/0x198 [] handle_irq_event_percpu+0x30/0x90 [] handle_percpu_irq+0x88/0xb8 [] generic_handle_irq+0x44/0x60 [] do_IRQ+0x18/0x28 [] plat_irq_dispatch+0x64/0x100 [] handle_int+0x140/0x14c [] irq_exit+0xf8/0x100 Because AZX_DRIVER_GENERIC can not work well for Loongson LS7A HDA controller, it needs some workarounds which are not merged into the upstream kernel at this time, so it should revert this patch now. Fixes: 61eee4a7fc40 ("ALSA: hda: Add support for Loongson 7A1000 controller") Cc: # 5.9-rc1+ Signed-off-by: Tiezhu Yang --- v2: update commit message Applied now. Thanks. Takashi
[PATCH] staging: qlge: fix quoted string split across lines
Fixed a coding style issue by merging split quoted strings in qlge_main.c to fix checkpatch warnings. Signed-off-by: Ross Schmidt --- drivers/staging/qlge/qlge_main.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c index 2028458bea6f..e4c9f5d3bfdd 100644 --- a/drivers/staging/qlge/qlge_main.c +++ b/drivers/staging/qlge/qlge_main.c @@ -2079,9 +2079,9 @@ static void ql_process_chip_ae_intr(struct ql_adapter *qdev, break; case PCI_ERR_ANON_BUF_RD: - netdev_err(qdev->ndev, "PCI error occurred when reading " - "anonymous buffers from rx_ring %d.\n", - ib_ae_rsp->q_id); + netdev_err(qdev->ndev, + "PCI error occurred when reading anonymous buffers from rx_ring %d.\n", + ib_ae_rsp->q_id); ql_queue_asic_error(qdev); break; @@ -2415,8 +2415,7 @@ static irqreturn_t qlge_isr(int irq, void *dev_id) ql_queue_asic_error(qdev); netdev_err(qdev->ndev, "Got fatal error, STS = %x.\n", var); var = ql_read32(qdev, ERR_STS); - netdev_err(qdev->ndev, "Resetting chip. " - "Error Status Register = 0x%x\n", var); + netdev_err(qdev->ndev, "Resetting chip. Error Status Register = 0x%x\n", var); return IRQ_HANDLED; } @@ -3739,8 +3738,7 @@ static void ql_display_dev_info(struct net_device *ndev) struct ql_adapter *qdev = netdev_priv(ndev); netif_info(qdev, probe, qdev->ndev, - "Function #%d, Port %d, NIC Roll %d, NIC Rev = %d, " - "XG Roll = %d, XG Rev = %d.\n", + "Function #%d, Port %d, NIC Roll %d, NIC Rev = %d, XG Roll = %d, XG Rev = %d.\n", qdev->func, qdev->port, qdev->chip_rev_id & 0x000f, -- 2.26.2
linux-next: build failure after merge of the bpf-next tree
Hi all, After merging the bpf-next tree, today's linux-next build (powerpcle perf) failed like this: util/bpf-loader.c: In function 'config_bpf_program': util/bpf-loader.c:331:2: error: 'bpf_program__title' is deprecated: BPF program title is confusing term; please use bpf_program__section_name() instead [-Werror=deprecated-declarations] 331 | config_str = bpf_program__title(prog, false); | ^~ In file included from util/bpf-loader.c:10: tools/lib/bpf/libbpf.h:203:13: note: declared here 203 | const char *bpf_program__title(const struct bpf_program *prog, bool needs_copy); | ^~ util/bpf-loader.c: In function 'preproc_gen_prologue': util/bpf-loader.c:457:3: error: 'bpf_program__title' is deprecated: BPF program title is confusing term; please use bpf_program__section_name() instead [-Werror=deprecated-declarations] 457 | title = bpf_program__title(prog, false); | ^ In file included from util/bpf-loader.c:10: tools/lib/bpf/libbpf.h:203:13: note: declared here 203 | const char *bpf_program__title(const struct bpf_program *prog, bool needs_copy); | ^~ cc1: all warnings being treated as errors Caused or exposed by commit 521095842027 ("libbpf: Deprecate notion of BPF program "title" in favor of "section name"") I have used the bpf-next tree from next-20200903 for today. -- Cheers, Stephen Rothwell pgp1eivFUvzwi.pgp Description: OpenPGP digital signature
[PATCH net-next 3/7] net: hns3: fix a typo in struct hclge_mac
From: Guangbin Huang The member link of struct hclge_mac stores the link status of MAC and PHY if PHY exists, but its annotation uses word "exit", so fix it. Signed-off-by: Guangbin Huang Signed-off-by: Huazhong Tan --- drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h index 9bbdd45..33e1af1 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h @@ -266,7 +266,7 @@ struct hclge_mac { u32 fec_mode; /* active fec mode */ u32 user_fec_mode; u32 fec_ability; - int link; /* store the link status of mac & phy (if phy exit) */ + int link; /* store the link status of mac & phy (if phy exists) */ struct phy_device *phydev; struct mii_bus *mdio_bus; phy_interface_t phy_if; -- 2.7.4
[PATCH net-next 1/7] net: hns3: narrow two local variable range in hclgevf_reset_prepare_wait()
Since variable send_msg and ret only used in if branch, so move their definition into the if branch. Signed-off-by: Huazhong Tan --- drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c index e972138..20dd04c 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c @@ -1788,10 +1788,10 @@ static int hclgevf_reset_prepare_wait(struct hclgevf_dev *hdev) { #define HCLGEVF_RESET_SYNC_TIME 100 - struct hclge_vf_to_pf_msg send_msg; - int ret = 0; - if (hdev->reset_type == HNAE3_VF_FUNC_RESET) { + struct hclge_vf_to_pf_msg send_msg; + int ret; + hclgevf_build_send_msg(_msg, HCLGE_MBX_RESET, 0); ret = hclgevf_send_mbx_msg(hdev, _msg, true, NULL, 0); if (ret) { @@ -1806,10 +1806,10 @@ static int hclgevf_reset_prepare_wait(struct hclgevf_dev *hdev) /* inform hardware that preparatory work is done */ msleep(HCLGEVF_RESET_SYNC_TIME); hclgevf_reset_handshake(hdev, true); - dev_info(>pdev->dev, "prepare reset(%d) wait done, ret:%d\n", -hdev->reset_type, ret); + dev_info(>pdev->dev, "prepare reset(%d) wait done\n", +hdev->reset_type); - return ret; + return 0; } static void hclgevf_dump_rst_info(struct hclgevf_dev *hdev) -- 2.7.4
[PATCH net-next 7/7] net: hns3: remove some unused function hns3_update_promisc_mode()
From: Guojia Liao hns3_update_promisc_mode is defined, but not be used, so remove it. Signed-off-by: Guojia Liao Signed-off-by: Huazhong Tan --- drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 14 -- drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 1 - 2 files changed, 15 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c index 1d66f84..93825a4 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c @@ -623,20 +623,6 @@ void hns3_request_update_promisc_mode(struct hnae3_handle *handle) ops->request_update_promisc_mode(handle); } -int hns3_update_promisc_mode(struct net_device *netdev, u8 promisc_flags) -{ - struct hns3_nic_priv *priv = netdev_priv(netdev); - struct hnae3_handle *h = priv->ae_handle; - - if (h->ae_algo->ops->set_promisc_mode) { - return h->ae_algo->ops->set_promisc_mode(h, - promisc_flags & HNAE3_UPE, - promisc_flags & HNAE3_MPE); - } - - return 0; -} - void hns3_enable_vlan_filter(struct net_device *netdev, bool enable) { struct hns3_nic_priv *priv = netdev_priv(netdev); diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h index cef6f9a..98ca6ea 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h @@ -601,7 +601,6 @@ void hns3_set_vector_coalesce_rl(struct hns3_enet_tqp_vector *tqp_vector, u32 rl_value); void hns3_enable_vlan_filter(struct net_device *netdev, bool enable); -int hns3_update_promisc_mode(struct net_device *netdev, u8 promisc_flags); void hns3_request_update_promisc_mode(struct hnae3_handle *handle); #ifdef CONFIG_HNS3_DCB -- 2.7.4
[PATCH net-next 0/7] net: hns3: misc updates
There are some misc updates for the HNS3 ethernet driver. #1 narrows two local variable range in hclgevf_reset_prepare_wait(). #2 adds reset failure check in periodic service task. #3~#7 adds some cleanups. Guangbin Huang (2): net: hns3: skip periodic service task if reset failed net: hns3: fix a typo in struct hclge_mac Guojia Liao (1): net: hns3: remove some unused function hns3_update_promisc_mode() Huazhong Tan (4): net: hns3: narrow two local variable range in hclgevf_reset_prepare_wait() net: hns3: remove unused field 'io_base' in struct hns3_enet_ring net: hns3: remove unused field 'tc_num_last_time' in struct hclge_dev net: hns3: remove some unused macros related to queue drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 16 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 7 --- drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 3 +++ drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 3 +-- .../net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c| 15 +-- 5 files changed, 13 insertions(+), 31 deletions(-) -- 2.7.4
[PATCH net-next 2/7] net: hns3: skip periodic service task if reset failed
From: Guangbin Huang When reset fails, if there are some pending jobs for the periodic service task, it does not do anything except print error each time the task is scheduled. So skip the periodic service task if reset failed. Signed-off-by: Guangbin Huang Signed-off-by: Huazhong Tan --- drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 3 +++ drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c index d553ed7..40d68a4 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c @@ -3944,6 +3944,9 @@ static void hclge_periodic_service_task(struct hclge_dev *hdev) { unsigned long delta = round_jiffies_relative(HZ); + if (test_bit(HCLGE_STATE_RST_FAIL, >state)) + return; + /* Always handle the link updating to make sure link state is * updated when it is triggered by mbx. */ diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c index 20dd04c..20dd50d 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c @@ -2186,6 +2186,9 @@ static void hclgevf_periodic_service_task(struct hclgevf_dev *hdev) unsigned long delta = round_jiffies_relative(HZ); struct hnae3_handle *handle = >nic; + if (test_bit(HCLGEVF_STATE_RST_FAIL, >state)) + return; + if (time_is_after_jiffies(hdev->last_serv_processed + HZ)) { delta = jiffies - hdev->last_serv_processed; -- 2.7.4
[PATCH net-next 5/7] net: hns3: remove unused field 'tc_num_last_time' in struct hclge_dev
'tc_num_last_time' is defined, but never used, so remove it. Reported-by: Jian Shen Signed-off-by: Huazhong Tan --- drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h index 33e1af1..3975332 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h @@ -749,7 +749,6 @@ struct hclge_dev { u16 num_tx_desc;/* desc num of per tx queue */ u16 num_rx_desc;/* desc num of per rx queue */ u8 hw_tc_map; - u8 tc_num_last_time; enum hclge_fc_mode fc_mode_last_time; u8 support_sfp_query; -- 2.7.4
[PATCH net-next 6/7] net: hns3: remove some unused macros related to queue
There are several macros related queue defined, but never used, so remove them. Signed-off-by: Huazhong Tan --- drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h index 0c146e7..cef6f9a 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h @@ -42,13 +42,8 @@ enum hns3_nic_state { #define HNS3_RING_TX_RING_PKTNUM_RECORD_REG0x0006C #define HNS3_RING_TX_RING_EBD_OFFSET_REG 0x00070 #define HNS3_RING_TX_RING_BD_ERR_REG 0x00074 -#define HNS3_RING_PREFETCH_EN_REG 0x0007C -#define HNS3_RING_CFG_VF_NUM_REG 0x00080 -#define HNS3_RING_ASID_REG 0x0008C #define HNS3_RING_EN_REG 0x00090 -#define HNS3_TX_REG_OFFSET 0x40 - #define HNS3_RX_HEAD_SIZE 256 #define HNS3_TX_TIMEOUT (5 * HZ) -- 2.7.4
[PATCH net-next 4/7] net: hns3: remove unused field 'io_base' in struct hns3_enet_ring
'io_base' has been defined and initialized, but never used, so remove it. Signed-off-by: Huazhong Tan --- drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 2 -- drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 1 - 2 files changed, 3 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c index 47ab2a5..1d66f84 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c @@ -3670,12 +3670,10 @@ static void hns3_ring_get_cfg(struct hnae3_queue *q, struct hns3_nic_priv *priv, ring = >ring[q->tqp_index]; desc_num = priv->ae_handle->kinfo.num_tx_desc; ring->queue_index = q->tqp_index; - ring->io_base = (u8 __iomem *)q->io_base + HNS3_TX_REG_OFFSET; } else { ring = >ring[q->tqp_index + queue_num]; desc_num = priv->ae_handle->kinfo.num_rx_desc; ring->queue_index = q->tqp_index; - ring->io_base = q->io_base; } hnae3_set_bit(ring->flag, HNAE3_RING_TYPE_B, ring_type); diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h index 9922c5f..0c146e7 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h @@ -380,7 +380,6 @@ struct ring_stats { }; struct hns3_enet_ring { - u8 __iomem *io_base; /* base io address for the ring */ struct hns3_desc *desc; /* dma map address space */ struct hns3_desc_cb *desc_cb; struct hns3_enet_ring *next; -- 2.7.4
linux-next: build warning after merge of the net-next tree
Hi all, After merging the net-next tree, today's linux-next build (powerpc ppc64_defconfig) produced this warning: net/bridge/br_multicast.c: In function 'br_multicast_find_port': net/bridge/br_multicast.c:1818:21: warning: unused variable 'br' [-Wunused-variable] 1818 | struct net_bridge *br = mp->br; | ^~ Introduced by commit 0436862e417e ("net: bridge: mcast: support for IGMPv3/MLDv2 ALLOW_NEW_SOURCES report") Maybe turning mlock_dereference into a static inline function would help. -- Cheers, Stephen Rothwell pgpzzgVqLf76v.pgp Description: OpenPGP digital signature
[MPTCP][PATCH v2 net 2/2] mptcp: fix subflow's remote_id issues
This patch set the init remote_id to zero, otherwise it will be a random number. Then it added the missing subflow's remote_id setting code both in __mptcp_subflow_connect and in subflow_ulp_clone. Fixes: 01cacb00b35cb ("mptcp: add netlink-based PM") Fixes: ec3edaa7ca6ce ("mptcp: Add handling of outgoing MP_JOIN requests") Fixes: f296234c98a8f ("mptcp: Add handling of incoming MP_JOIN requests") Signed-off-by: Geliang Tang --- net/mptcp/pm_netlink.c | 2 +- net/mptcp/subflow.c| 7 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 3e70d848033d..bd88e9c0bf71 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -181,9 +181,9 @@ static void check_work_pending(struct mptcp_sock *msk) static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) { + struct mptcp_addr_info remote = { 0 }; struct sock *sk = (struct sock *)msk; struct mptcp_pm_addr_entry *local; - struct mptcp_addr_info remote; struct pm_nl_pernet *pernet; pernet = net_generic(sock_net((struct sock *)msk), pm_nl_pernet_id); diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index e8cac2655c82..9ead43f79023 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1063,6 +1063,7 @@ int __mptcp_subflow_connect(struct sock *sk, int ifindex, struct mptcp_sock *msk = mptcp_sk(sk); struct mptcp_subflow_context *subflow; struct sockaddr_storage addr; + int remote_id = remote->id; int local_id = loc->id; struct socket *sf; struct sock *ssk; @@ -1107,10 +1108,11 @@ int __mptcp_subflow_connect(struct sock *sk, int ifindex, goto failed; mptcp_crypto_key_sha(subflow->remote_key, _token, NULL); - pr_debug("msk=%p remote_token=%u local_id=%d", msk, remote_token, -local_id); + pr_debug("msk=%p remote_token=%u local_id=%d remote_id=%d", msk, +remote_token, local_id, remote_id); subflow->remote_token = remote_token; subflow->local_id = local_id; + subflow->remote_id = remote_id; subflow->request_join = 1; subflow->request_bkup = 1; mptcp_info2sockaddr(remote, ); @@ -1347,6 +1349,7 @@ static void subflow_ulp_clone(const struct request_sock *req, new_ctx->fully_established = 1; new_ctx->backup = subflow_req->backup; new_ctx->local_id = subflow_req->local_id; + new_ctx->remote_id = subflow_req->remote_id; new_ctx->token = subflow_req->token; new_ctx->thmac = subflow_req->thmac; } -- 2.17.1
Re: [PATCH v5 00/21] kprobes: Unify kretprobe trampoline handlers and make kretprobe lockless
On Mon, 07 Sep 2020 13:44:19 -0400 f...@redhat.com (Frank Ch. Eigler) wrote: > Masami Hiramatsu writes: > > > Sorry, for noticing this point, I Cc'd to systemtap. Is systemtap taking > > care of spinlock too? > > On PRREMPT_RT configurations, systemtap uses the raw_spinlock_t > types/functions, to keep its probe handlers as atomic as we can make them. OK, if the lock is only used in the probe handlers, there should be no problem. Even if a probe hits in the NMI which happens in another kprobe handler, the probe does not call its handler (because we don't support nested kprobes* yet). But maybe you'll get warnings if you enable the lockdep. * https://lkml.kernel.org/r/158894789510.14896.13461271606820304664.stgit@devnote2 It seems that we need more work for the nested kprobes. Thank you, -- Masami Hiramatsu
[MPTCP][PATCH v2 net 1/2] mptcp: fix subflow's local_id issues
In mptcp_pm_nl_get_local_id, skc_local is the same as msk_local, so it always return 0. Thus every subflow's local_id is 0. It's incorrect. This patch fixed this issue. Also, we need to ignore the zero address here, like 0.0.0.0 in IPv4. When we use the zero address as a local address, it means that we can use any one of the local addresses. The zero address is not a new address, we don't need to add it to PM, so this patch added a new function address_zero to check whether an address is the zero address, if it is, we ignore this address. Fixes: 01cacb00b35cb ("mptcp: add netlink-based PM") Signed-off-by: Geliang Tang --- net/mptcp/pm_netlink.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 2c208d2e65cd..3e70d848033d 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -66,6 +66,16 @@ static bool addresses_equal(const struct mptcp_addr_info *a, return a->port == b->port; } +static bool address_zero(const struct mptcp_addr_info *addr) +{ + struct mptcp_addr_info zero; + + memset(, 0, sizeof(zero)); + zero.family = addr->family; + + return addresses_equal(addr, , false); +} + static void local_address(const struct sock_common *skc, struct mptcp_addr_info *addr) { @@ -323,10 +333,13 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc) * addr */ local_address((struct sock_common *)msk, _local); - local_address((struct sock_common *)msk, _local); + local_address((struct sock_common *)skc, _local); if (addresses_equal(_local, _local, false)) return 0; + if (address_zero(_local)) + return 0; + pernet = net_generic(sock_net((struct sock *)msk), pm_nl_pernet_id); rcu_read_lock(); -- 2.17.1
[MPTCP][PATCH v2 net 0/2] mptcp: fix subflow's local_id/remote_id issues
v2: - add Fixes tags; - simply with 'return addresses_equal'; - use 'reversed Xmas tree' way. Geliang Tang (2): mptcp: fix subflow's local_id issues mptcp: fix subflow's remote_id issues net/mptcp/pm_netlink.c | 17 +++-- net/mptcp/subflow.c| 7 +-- 2 files changed, 20 insertions(+), 4 deletions(-) -- 2.17.1
RE: [PATCH V2 3/3] pinctrl: imx: Support building i.MX pinctrl core driver as module
> From: Anson Huang > Sent: Monday, September 7, 2020 8:33 PM > > Change PINCTRL_IMX to tristate to support loadable module build. > > And i.MX common pinctrl driver should depend on CONFIG_OF to make sure no > build error when i.MX common pinctrl driver is enabled for different > architectures without CONFIG_OF. > > Also add module author, description and license. > > Signed-off-by: Anson Huang Reviewed-by: Dong Aisheng Regards Aisheng
[PATCH v3 2/2] f2fs: change return value of f2fs_disable_compressed_file to bool
From: Daeho Jeong The returned integer is not required anywhere. So we need to change the return value to bool type. Signed-off-by: Daeho Jeong --- fs/f2fs/data.c | 2 +- fs/f2fs/f2fs.h | 17 ++--- fs/f2fs/file.c | 4 ++-- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 48cab85205e2..f30348063017 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -3936,7 +3936,7 @@ static int f2fs_swap_activate(struct swap_info_struct *sis, struct file *file, if (ret) return ret; - if (f2fs_disable_compressed_file(inode)) + if (!f2fs_disable_compressed_file(inode)) return -EINVAL; ret = check_swap_activate(sis, file, span); diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index c615e75c82fd..a33c837e833a 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3930,26 +3930,21 @@ static inline void set_compress_context(struct inode *inode) f2fs_mark_inode_dirty_sync(inode, true); } -static inline u32 f2fs_disable_compressed_file(struct inode *inode) +static inline bool f2fs_disable_compressed_file(struct inode *inode) { struct f2fs_inode_info *fi = F2FS_I(inode); - u32 i_compr_blocks; if (!f2fs_compressed_file(inode)) - return 0; - if (S_ISREG(inode->i_mode)) { - if (get_dirty_pages(inode)) - return 1; - i_compr_blocks = atomic_read(>i_compr_blocks); - if (i_compr_blocks) - return i_compr_blocks; - } + return true; + if (S_ISREG(inode->i_mode) && + (get_dirty_pages(inode) || atomic_read(>i_compr_blocks))) + return false; fi->i_flags &= ~F2FS_COMPR_FL; stat_dec_compr_inode(inode); clear_inode_flag(inode, FI_COMPRESSED_FILE); f2fs_mark_inode_dirty_sync(inode, true); - return 0; + return true; } #define F2FS_FEATURE_FUNCS(name, flagname) \ diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index adc4acad488a..d69def08e25e 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -1828,7 +1828,7 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask) if ((iflags ^ masked_flags) & F2FS_COMPR_FL) { if (masked_flags & F2FS_COMPR_FL) { - if (f2fs_disable_compressed_file(inode)) + if (!f2fs_disable_compressed_file(inode)) return -EINVAL; } if (iflags & F2FS_NOCOMP_FL) @@ -3258,7 +3258,7 @@ static int f2fs_ioc_set_pin_file(struct file *filp, unsigned long arg) if (ret) goto out; - if (f2fs_disable_compressed_file(inode)) { + if (!f2fs_disable_compressed_file(inode)) { ret = -EOPNOTSUPP; goto out; } -- 2.28.0.526.ge36021eeef-goog
[PATCH v3 1/2] f2fs: change i_compr_blocks of inode to atomic value
From: Daeho Jeong writepages() can be concurrently invoked for the same file by different threads such as a thread fsyncing the file and a kworker kernel thread. So, changing i_compr_blocks without protection is racy and we need to protect it by changing it with atomic type value. Plus, we don't need a 64bit value for i_compr_blocks, so just we will use a atomic value, not atomic64. Signed-off-by: Daeho Jeong --- Changes in v3: - Roll back to the original flow except changing atomic64 to atomic Changes in v2: - Change atomic64 to atomic and remove unnecessary part Signed-off-by: Daeho Jeong --- fs/f2fs/f2fs.h | 17 ++--- fs/f2fs/file.c | 22 -- fs/f2fs/inode.c | 11 +++ fs/f2fs/super.c | 1 + 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index f60414805e05..c615e75c82fd 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -801,7 +801,7 @@ struct f2fs_inode_info { struct timespec64 i_disk_time[4];/* inode disk times */ /* for file compress */ - u64 i_compr_blocks; /* # of compressed blocks */ + atomic_t i_compr_blocks;/* # of compressed blocks */ unsigned char i_compress_algorithm; /* algorithm type */ unsigned char i_log_cluster_size; /* log of cluster size */ unsigned int i_cluster_size;/* cluster size */ @@ -3930,17 +3930,19 @@ static inline void set_compress_context(struct inode *inode) f2fs_mark_inode_dirty_sync(inode, true); } -static inline u64 f2fs_disable_compressed_file(struct inode *inode) +static inline u32 f2fs_disable_compressed_file(struct inode *inode) { struct f2fs_inode_info *fi = F2FS_I(inode); + u32 i_compr_blocks; if (!f2fs_compressed_file(inode)) return 0; if (S_ISREG(inode->i_mode)) { if (get_dirty_pages(inode)) return 1; - if (fi->i_compr_blocks) - return fi->i_compr_blocks; + i_compr_blocks = atomic_read(>i_compr_blocks); + if (i_compr_blocks) + return i_compr_blocks; } fi->i_flags &= ~F2FS_COMPR_FL; @@ -4057,16 +4059,17 @@ static inline void f2fs_i_compr_blocks_update(struct inode *inode, u64 blocks, bool add) { int diff = F2FS_I(inode)->i_cluster_size - blocks; + struct f2fs_inode_info *fi = F2FS_I(inode); /* don't update i_compr_blocks if saved blocks were released */ - if (!add && !F2FS_I(inode)->i_compr_blocks) + if (!add && !atomic_read(>i_compr_blocks)) return; if (add) { - F2FS_I(inode)->i_compr_blocks += diff; + atomic_add(diff, >i_compr_blocks); stat_add_compr_blocks(inode, diff); } else { - F2FS_I(inode)->i_compr_blocks -= diff; + atomic_sub(diff, >i_compr_blocks); stat_sub_compr_blocks(inode, diff); } f2fs_mark_inode_dirty_sync(inode, true); diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index cc7f5670390f..adc4acad488a 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -564,7 +564,7 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count) bool compressed_cluster = false; int cluster_index = 0, valid_blocks = 0; int cluster_size = F2FS_I(dn->inode)->i_cluster_size; - bool released = !F2FS_I(dn->inode)->i_compr_blocks; + bool released = !atomic_read(_I(dn->inode)->i_compr_blocks); if (IS_INODE(dn->node_page) && f2fs_has_extra_attr(dn->inode)) base = get_extra_isize(dn->inode); @@ -3436,7 +3436,7 @@ static int f2fs_get_compress_blocks(struct file *filp, unsigned long arg) if (!f2fs_compressed_file(inode)) return -EINVAL; - blocks = F2FS_I(inode)->i_compr_blocks; + blocks = atomic_read(_I(inode)->i_compr_blocks); return put_user(blocks, (u64 __user *)arg); } @@ -3535,7 +3535,7 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) if (ret) goto out; - if (!F2FS_I(inode)->i_compr_blocks) + if (!atomic_read(_I(inode)->i_compr_blocks)) goto out; F2FS_I(inode)->i_flags |= F2FS_IMMUTABLE_FL; @@ -3588,14 +3588,15 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) if (ret >= 0) { ret = put_user(released_blocks, (u64 __user *)arg); - } else if (released_blocks && F2FS_I(inode)->i_compr_blocks) { + } else if (released_blocks && + atomic_read(_I(inode)->i_compr_blocks)) { set_sbi_flag(sbi, SBI_NEED_FSCK); f2fs_warn(sbi, "%s: partial blocks were released i_ino=%lx " - "iblocks=%llu,
RE: [PATCH V2 2/3] pinctrl: imx: Support building SCU pinctrl core driver as module
> From: Anson Huang > Sent: Monday, September 7, 2020 8:33 PM > > Change PINCTR_IMX_SCU to tristate, remove unnecessary #ifdef and add > module author, description and license to support building SCU pinctrl core > driver as module. > > Signed-off-by: Anson Huang > --- > Changes since V1: > - split V1 [1/2] patch to 2 patches, this patch supports building SCU > pinctrl > core > driver as module; > - remove unnecessary #ifdef check and #else block. > --- > drivers/pinctrl/freescale/Kconfig | 2 +- > drivers/pinctrl/freescale/pinctrl-imx.h | 20 > drivers/pinctrl/freescale/pinctrl-scu.c | 5 + > 3 files changed, 6 insertions(+), 21 deletions(-) > > diff --git a/drivers/pinctrl/freescale/Kconfig > b/drivers/pinctrl/freescale/Kconfig > index 08fcf5c..452c499 100644 > --- a/drivers/pinctrl/freescale/Kconfig > +++ b/drivers/pinctrl/freescale/Kconfig > @@ -7,7 +7,7 @@ config PINCTRL_IMX > select REGMAP > > config PINCTRL_IMX_SCU > - bool > + tristate "IMX SCU pinctrl core driver" > depends on IMX_SCU > select PINCTRL_IMX > [...] > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h > b/drivers/pinctrl/freescale/pinctrl-imx.h > index 40927ca..fd8c4b6 100644 > --- a/drivers/pinctrl/freescale/pinctrl-imx.h > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h > @@ -144,7 +144,6 @@ struct imx_pinctrl_soc_info { int > imx_pinctrl_probe(struct platform_device *pdev, > const struct imx_pinctrl_soc_info *info); > > -#ifdef CONFIG_PINCTRL_IMX_SCU > #define BM_PAD_CTL_GP_ENABLE BIT(30) > #define BM_PAD_CTL_IFMUX_ENABLE BIT(31) > #define BP_PAD_CTL_IFMUX 27 > @@ -157,23 +156,4 @@ int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, > unsigned pin_id, void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl, > unsigned int *pin_id, struct imx_pin *pin, > const __be32 **list_p); > -#else > -static inline int imx_pinconf_get_scu(struct pinctrl_dev *pctldev, > - unsigned pin_id, unsigned long *config) > -{ > - return -EINVAL; > -} > -static inline int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, > - unsigned pin_id, unsigned long *configs, > - unsigned num_configs) > -{ > - return -EINVAL; > -} > -static inline void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl, > - unsigned int *pin_id, > - struct imx_pin *pin, > - const __be32 **list_p) > -{ > -} > -#endif > #endif /* __DRIVERS_PINCTRL_IMX_H */ Should this part of changes go to Patch 1? > diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c > b/drivers/pinctrl/freescale/pinctrl-scu.c > index 9df45d3..59b5f8a 100644 > --- a/drivers/pinctrl/freescale/pinctrl-scu.c > +++ b/drivers/pinctrl/freescale/pinctrl-scu.c > @@ -7,6 +7,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -123,3 +124,7 @@ void imx_pinctrl_parse_pin_scu(struct imx_pinctrl > *ipctl, > pin_scu->mux_mode, pin_scu->config); > } > EXPORT_SYMBOL_GPL(imx_pinctrl_parse_pin_scu); > + > +MODULE_AUTHOR("Dong Aisheng "); > +MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.7.4
Re: [PATCH v2 12/12] xhci: tegra: enable ELPG for runtime/system PM
Thanks Dmitry. I will remove this. On 9/2/20 4:33 AM, Dmitry Osipenko wrote: > 31.08.2020 07:40, JC Kuo пишет: >> +err = devm_request_threaded_irq(>dev, tegra->padctl_irq, >> +NULL, >> +tegra_xusb_padctl_irq, >> +IRQF_ONESHOT | > >> +IRQF_TRIGGER_HIGH, > > Specifying trigger levels is meaningless for interrupts coming from a > device-tree because DT levels always take precedence. >
Re: [PATCH 1/1] watchdog: remove unneeded inclusion of
On 9/7/20 12:50 AM, Leizhen (ThunderTown) wrote: > Hi, Wim Van Sebroeck, Guenter Roeck: > What's your opinion? Guenter Roeck given "Reviewed-by" two weeks ago. > The patch is in my watchdog-next branch, and Wim usually picks it up from there. Guenter > > On 2020/8/27 21:40, Guenter Roeck wrote: >> On 8/26/20 11:21 PM, Zhen Lei wrote: >>> There has been no reference to "struct sched_param" since >>> commit 94beddacb53c ("sched,watchdog: Convert to sched_set_fifo()"), so >>> there's no need to include any more, delete >>> it. >>> >>> Signed-off-by: Zhen Lei >> >> Reviewed-by: Guenter Roeck >> >>> --- >>> drivers/watchdog/watchdog_dev.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/drivers/watchdog/watchdog_dev.c >>> b/drivers/watchdog/watchdog_dev.c >>> index 6798addabd5a067..0f18fa2433310b0 100644 >>> --- a/drivers/watchdog/watchdog_dev.c >>> +++ b/drivers/watchdog/watchdog_dev.c >>> @@ -43,8 +43,6 @@ >>> #include /* For watchdog specific items */ >>> #include /* For copy_to_user/put_user/... */ >>> >>> -#include /* For struct sched_param */ >>> - >>> #include "watchdog_core.h" >>> #include "watchdog_pretimeout.h" >>> >>> >> >> >> >
RE: [PATCH V2 1/3] pinctrl: imx: Use function callbacks for SCU related functions
> From: Anson Huang > Sent: Monday, September 7, 2020 8:33 PM > > Use function callbacks for SCU related functions in pinctrl-imx.c in order to > support the scenario of PINCTRL_IMX is built in while PINCTRL_IMX_SCU is built > as module, all drivers using SCU pinctrl driver need to initialize the SCU > related > function callback. > > Signed-off-by: Anson Huang > --- > Changes since V1: > - split V1 [1/2] patch to 2 patches, this patch does the change of using > function > callbacks for SCU related functions. > --- > drivers/pinctrl/freescale/pinctrl-imx.c | 8 +++ > drivers/pinctrl/freescale/pinctrl-imx.h | 37 > + > drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 3 +++ > drivers/pinctrl/freescale/pinctrl-imx8qm.c | 3 +++ > drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 3 +++ > 5 files changed, 35 insertions(+), 19 deletions(-) > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c > b/drivers/pinctrl/freescale/pinctrl-imx.c > index 507e4af..b80c450 100644 > --- a/drivers/pinctrl/freescale/pinctrl-imx.c > +++ b/drivers/pinctrl/freescale/pinctrl-imx.c > @@ -373,7 +373,7 @@ static int imx_pinconf_get(struct pinctrl_dev *pctldev, > const struct imx_pinctrl_soc_info *info = ipctl->info; > > if (info->flags & IMX_USE_SCU) > - return imx_pinconf_get_scu(pctldev, pin_id, config); > + return info->imx_pinconf_get(pctldev, pin_id, config); > else > return imx_pinconf_get_mmio(pctldev, pin_id, config); } @@ > -423,7 > +423,7 @@ static int imx_pinconf_set(struct pinctrl_dev *pctldev, > const struct imx_pinctrl_soc_info *info = ipctl->info; > > if (info->flags & IMX_USE_SCU) > - return imx_pinconf_set_scu(pctldev, pin_id, > + return info->imx_pinconf_set(pctldev, pin_id, > configs, num_configs); > else > return imx_pinconf_set_mmio(pctldev, pin_id, @@ -440,7 +440,7 > @@ static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev, > int ret; > > if (info->flags & IMX_USE_SCU) { > - ret = imx_pinconf_get_scu(pctldev, pin_id, ); > + ret = info->imx_pinconf_get(pctldev, pin_id, ); > if (ret) { > dev_err(ipctl->dev, "failed to get %s pinconf\n", > pin_get_name(pctldev, pin_id)); > @@ -629,7 +629,7 @@ static int imx_pinctrl_parse_groups(struct device_node > *np, > for (i = 0; i < grp->num_pins; i++) { > pin = &((struct imx_pin *)(grp->data))[i]; > if (info->flags & IMX_USE_SCU) > - imx_pinctrl_parse_pin_scu(ipctl, >pins[i], > + info->imx_pinctrl_parse_pin(ipctl, >pins[i], > pin, ); > else > imx_pinctrl_parse_pin_mmio(ipctl, >pins[i], diff > --git > a/drivers/pinctrl/freescale/pinctrl-imx.h > b/drivers/pinctrl/freescale/pinctrl-imx.h > index 333d32b..40927ca 100644 > --- a/drivers/pinctrl/freescale/pinctrl-imx.h > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h > @@ -75,6 +75,21 @@ struct imx_cfg_params_decode { > bool invert; > }; > > +/** > + * @dev: a pointer back to containing device > + * @base: the offset to the controller in virtual memory */ struct > +imx_pinctrl { > + struct device *dev; > + struct pinctrl_dev *pctl; > + void __iomem *base; > + void __iomem *input_sel_base; > + const struct imx_pinctrl_soc_info *info; > + struct imx_pin_reg *pin_regs; > + unsigned int group_index; > + struct mutex mutex; > +}; > + You seems missed my question in the former patch review. Could you clarify a bit why need move this part code? Regards Aisheng > struct imx_pinctrl_soc_info { > const struct pinctrl_pin_desc *pins; > unsigned int npins; > @@ -98,21 +113,13 @@ struct imx_pinctrl_soc_info { > struct pinctrl_gpio_range *range, > unsigned offset, > bool input); > -}; > - > -/** > - * @dev: a pointer back to containing device > - * @base: the offset to the controller in virtual memory > - */ > -struct imx_pinctrl { > - struct device *dev; > - struct pinctrl_dev *pctl; > - void __iomem *base; > - void __iomem *input_sel_base; > - const struct imx_pinctrl_soc_info *info; > - struct imx_pin_reg *pin_regs; > - unsigned int group_index; > - struct mutex mutex; > + int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned int pin_id, > +unsigned long *config); > + int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned int pin_id, > +unsigned long *configs, unsigned int > num_configs); > + void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl, > + unsigned int
Re: [PATCH net-next 0/2] net: two updates related to UDP GSO
On 2020/9/7 23:35, Willem de Bruijn wrote: On Mon, Sep 7, 2020 at 3:38 PM tanhuazhong wrote: On 2020/9/7 17:22, Willem de Bruijn wrote: On Sun, Sep 6, 2020 at 8:42 PM Jakub Kicinski wrote: On Sat, 5 Sep 2020 14:11:11 +0800 Huazhong Tan wrote: There are two updates relates to UDP GSO. #1 adds a new GSO type for UDPv6 #2 adds check for UDP GSO when csum is disable in netdev_fix_features(). Changes since RFC V2: - modifies the timing of setting UDP GSO type when doing UDP GRO in #1. Changes since RFC V1: - updates NETIF_F_GSO_LAST suggested by Willem de Bruijn. and add NETIF_F_GSO_UDPV6_L4 feature for each driver who support UDP GSO in #1. - add #2 who needs #1. Please CC people who gave you feedback (Willem). I don't feel good about this series. IPv6 is not optional any more. AFAIU you have some issues with csum support in your device? Can you use .ndo_features_check() to handle this? The change in semantics of NETIF_F_GSO_UDP_L4 from "v4 and v6" to "just v4" can trip people over; this is not a new feature people may be depending on the current semantics. Willem, what are your thoughts on this? If that is the only reason, +1 on fixing it up in the driver's ndo_features_check. Hi, Willem & Jakub. This series mainly fixes the feature dependency between hardware checksum and UDP GSO. When turn off hardware checksum offload, run 'ethtool -k [devname]' we can see TSO is off as well, but udp gso still is on. I see. That does not entirely require separate IPv4 and IPv6 flags. It can be disabled if either checksum offload is disabled. I'm not aware of any hardware that only supports checksum offload for one of the two network protocols. below patch is acceptable? i have sent this patch before (https://patchwork.ozlabs.org/project/netdev/patch/1594180136-15912-3-git-send-email-tanhuazh...@huawei.com/) diff --git a/net/core/dev.c b/net/core/dev.c index c02bae9..dcb6b35 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -9095,6 +9095,12 @@ static netdev_features_t netdev_fix_features(struct net_device *dev, features &= ~NETIF_F_TSO6; } + if ((features & NETIF_F_GSO_UDP_L4) && !(features & NETIF_F_HW_CSUM) && + (!(features & NETIF_F_IP_CSUM) || !(features & NETIF_F_IPV6_CSUM))) { + netdev_dbg(dev, "Dropping UDP GSO features since no CSUM feature.\n"); + features &= ~NETIF_F_GSO_UDP_L4; + } + /* TSO with IPv4 ID mangling requires IPv4 TSO be enabled */ if ((features & NETIF_F_TSO_MANGLEID) && !(features & NETIF_F_TSO)) features &= ~NETIF_F_TSO_MANGLEID; As Eric Dumazet commented "This would prevent a device providing IPv4 checksum only (no IPv6 csum support) from sending IPv4 UDP GSO packets ?", so i send this series to decouple them. Is there any good ways to shuttle this issue? Or as you said there is not device only support checksum offload for one of the two network protocols. Alternatively, the real value of splitting the type is in advertising the features separately through ethtool. That requires additional changes. .
Re: [PATCH v2 12/12] xhci: tegra: enable ELPG for runtime/system PM
Hi Thierry, Thanks for review. I will amend accordingly and submit a new revision. JC On 8/31/20 8:50 PM, Thierry Reding wrote: > On Mon, Aug 31, 2020 at 12:40:43PM +0800, JC Kuo wrote: >> This commit implements the complete programming sequence for ELPG >> entry and exit. >> >> 1. At ELPG entry, invokes tegra_xusb_padctl_enable_phy_sleepwalk() >> and tegra_xusb_padctl_enable_phy_wake() to configure XUSB PADCTL >> sleepwalk and wake detection circuits to maintain USB lines level >> and respond to wake events (wake-on-connect, wake-on-disconnect, >> device-initiated-wake). >> >> 2. At ELPG exit, invokes tegra_xusb_padctl_disable_phy_sleepwalk() >> and tegra_xusb_padctl_disable_phy_wake() to disarm sleepwalk and >> wake detection circuits. >> >> At runtime suspend, XUSB host controller can enter ELPG to reduce >> power consumption. When XUSB PADCTL wake detection circuit detects >> a wake event, an interrupt will be raised. xhci-tegra driver then >> will invoke pm_runtime_resume() for xhci-tegra. >> >> Runtime resume could also be triggered by protocol drivers, this is >> the host-initiated-wake event. At runtime resume, xhci-tegra driver >> brings XUSB host controller out of ELPG to handle the wake events. >> >> The same ELPG enter/exit procedure will be performed for system >> suspend/resume path so USB devices can remain connected across SC7. >> >> Signed-off-by: JC Kuo >> --- >> drivers/usb/host/xhci-tegra.c | 391 +++--- >> 1 file changed, 361 insertions(+), 30 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c >> index ce6526c2caf6..9530cfc83f45 100644 >> --- a/drivers/usb/host/xhci-tegra.c >> +++ b/drivers/usb/host/xhci-tegra.c >> @@ -15,9 +15,11 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -224,6 +226,7 @@ struct tegra_xusb { >> >> int xhci_irq; >> int mbox_irq; >> +int padctl_irq; >> >> void __iomem *ipfs_base; >> void __iomem *fpci_base; >> @@ -268,10 +271,13 @@ struct tegra_xusb { >> dma_addr_t phys; >> } fw; >> >> +bool suspended; >> struct tegra_xusb_context context; >> }; >> >> static struct hc_driver __read_mostly tegra_xhci_hc_driver; >> +static int tegra_xusb_exit_elpg(struct tegra_xusb *tegra, bool runtime); >> +static int tegra_xusb_enter_elpg(struct tegra_xusb *tegra, bool runtime); >> >> static inline u32 fpci_readl(struct tegra_xusb *tegra, unsigned int offset) >> { >> @@ -657,6 +663,9 @@ static irqreturn_t tegra_xusb_mbox_thread(int irq, void >> *data) >> >> mutex_lock(>lock); >> >> +if (pm_runtime_suspended(tegra->dev) || tegra->suspended) >> +goto out; >> + >> value = fpci_readl(tegra, tegra->soc->mbox.data_out); >> tegra_xusb_mbox_unpack(, value); >> >> @@ -670,6 +679,7 @@ static irqreturn_t tegra_xusb_mbox_thread(int irq, void >> *data) >> >> tegra_xusb_mbox_handle(tegra, ); >> >> +out: >> mutex_unlock(>lock); >> return IRQ_HANDLED; >> } >> @@ -812,12 +822,27 @@ static void tegra_xusb_phy_disable(struct tegra_xusb >> *tegra) >> >> static int tegra_xusb_runtime_suspend(struct device *dev) >> { >> -return 0; >> +struct tegra_xusb *tegra = dev_get_drvdata(dev); >> +int ret; >> + >> +synchronize_irq(tegra->mbox_irq); >> +mutex_lock(>lock); >> +ret = tegra_xusb_enter_elpg(tegra, true); >> +mutex_unlock(>lock); >> + >> +return ret; >> } >> >> static int tegra_xusb_runtime_resume(struct device *dev) >> { >> -return 0; >> +struct tegra_xusb *tegra = dev_get_drvdata(dev); >> +int err; >> + >> +mutex_lock(>lock); >> +err = tegra_xusb_exit_elpg(tegra, true); >> +mutex_unlock(>lock); >> + >> +return err; >> } >> >> #ifdef CONFIG_PM_SLEEP >> @@ -1121,6 +1146,22 @@ static int >> __tegra_xusb_enable_firmware_messages(struct tegra_xusb *tegra) >> return err; >> } >> >> +static irqreturn_t tegra_xusb_padctl_irq(int irq, void *data) >> +{ >> +struct tegra_xusb *tegra = data; >> + >> +mutex_lock(>lock); >> +if (tegra->suspended) { >> +mutex_unlock(>lock); >> +return IRQ_HANDLED; >> +} >> +mutex_unlock(>lock); > > Blank lines before and after a block can help make this less cluttered. > >> + >> +pm_runtime_resume(tegra->dev); >> + >> +return IRQ_HANDLED; >> +} >> + >> static int tegra_xusb_enable_firmware_messages(struct tegra_xusb *tegra) >> { >> int err; >> @@ -1244,6 +1285,51 @@ static void tegra_xhci_id_work(struct work_struct >> *work) >> } >> } >> >> +static bool is_usb2_otg_phy(struct tegra_xusb *tegra, int index) > > unsigned int index? > >> +{ >> +return (tegra->usbphy[index] != NULL); >> +} >> + >> +static bool is_usb3_otg_phy(struct tegra_xusb *tegra, int index) > > Here too. > >> +{ >> +
Re: [PATCH v2 11/12] usb: host: xhci-tegra: unlink power domain devices
On 8/31/20 8:42 PM, Thierry Reding wrote: > On Mon, Aug 31, 2020 at 12:40:42PM +0800, JC Kuo wrote: >> This commit unlinks xhci-tegra platform device with ss/host power >> domain devices. Reasons for this change is - at elpg entry, phy >> sleepwalk and wake configuration need to be done before powering >> down ss/host partitions, and phy need be powered off after powering >> down ss/host partitions. Sequence looks like roughly below: >> >> tegra_xusb_enter_elpg() -> xhci_suspend() >> -> enable phy sleepwalk and wake if needed >> -> power down ss/host partitions >> -> power down phy >> >> If ss/host power domains are linked to xhci-tegra platform device, we >> are not able to perform the sequence like above. >> >> This commit introduces: >> 1. tegra_xusb_unpowergate_partitions() to power up ss and host >> partitions together. If ss/host power domain devices are >> available, it invokes pm_runtime_get_sync() to request power >> driver to power up partitions; If power domain devices are not >> available, tegra_powergate_sequence_power_up() will be used to >> power up partitions. >> >> 2. tegra_xusb_powergate_partitions() to power down ss and host >> partitions together. If ss/host power domain devices are >> available, it invokes pm_runtime_put_sync() to request power >> driver to power down partitions; If power domain devices are not >> available, tegra_powergate_power_off() will be used to power down >> partitions. >> >> Signed-off-by: JC Kuo >> --- >> drivers/usb/host/xhci-tegra.c | 202 +++--- >> 1 file changed, 111 insertions(+), 91 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c >> index 934be1686352..ce6526c2caf6 100644 >> --- a/drivers/usb/host/xhci-tegra.c >> +++ b/drivers/usb/host/xhci-tegra.c >> @@ -249,8 +249,6 @@ struct tegra_xusb { >> >> struct device *genpd_dev_host; >> struct device *genpd_dev_ss; >> -struct device_link *genpd_dl_host; >> -struct device_link *genpd_dl_ss; >> >> struct phy **phys; >> unsigned int num_phys; >> @@ -814,36 +812,12 @@ static void tegra_xusb_phy_disable(struct tegra_xusb >> *tegra) >> >> static int tegra_xusb_runtime_suspend(struct device *dev) >> { >> -struct tegra_xusb *tegra = dev_get_drvdata(dev); >> - >> -regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); >> -tegra_xusb_clk_disable(tegra); >> - >> return 0; >> } >> >> static int tegra_xusb_runtime_resume(struct device *dev) >> { >> -struct tegra_xusb *tegra = dev_get_drvdata(dev); >> -int err; >> - >> -err = tegra_xusb_clk_enable(tegra); >> -if (err) { >> -dev_err(dev, "failed to enable clocks: %d\n", err); >> -return err; >> -} >> - >> -err = regulator_bulk_enable(tegra->soc->num_supplies, tegra->supplies); >> -if (err) { >> -dev_err(dev, "failed to enable regulators: %d\n", err); >> -goto disable_clk; >> -} >> - >> return 0; >> - >> -disable_clk: >> -tegra_xusb_clk_disable(tegra); >> -return err; >> } >> >> #ifdef CONFIG_PM_SLEEP >> @@ -1019,10 +993,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb >> *tegra) >> static void tegra_xusb_powerdomain_remove(struct device *dev, >>struct tegra_xusb *tegra) >> { >> -if (tegra->genpd_dl_ss) >> -device_link_del(tegra->genpd_dl_ss); >> -if (tegra->genpd_dl_host) >> -device_link_del(tegra->genpd_dl_host); >> if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) >> dev_pm_domain_detach(tegra->genpd_dev_ss, true); >> if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) >> @@ -1048,20 +1018,88 @@ static int tegra_xusb_powerdomain_init(struct device >> *dev, >> return err; >> } >> >> -tegra->genpd_dl_host = device_link_add(dev, tegra->genpd_dev_host, >> - DL_FLAG_PM_RUNTIME | >> - DL_FLAG_STATELESS); >> -if (!tegra->genpd_dl_host) { >> -dev_err(dev, "adding host device link failed!\n"); >> -return -ENODEV; >> +return 0; >> +} >> + >> +static int tegra_xusb_unpowergate_partitions(struct tegra_xusb *tegra) >> +{ >> +struct device *dev = tegra->dev; >> +bool use_genpd; >> +int rc; >> + >> +use_genpd = of_property_read_bool(dev->of_node, "power-domains"); > > I don't think that's technically correct. Just because a "power-domains" > property exists in DT doesn't mean any power domains are necessarily > attached to the device. I think you'll need to check for something like > > if (dev->pm_domain) > > here. > Thanks Thierry. I will do so in the next revision. > Thierry >
[PATCH] pinctrl: rockchip: populate platform device for rockchip gpio
Register both gpio driver and device as part of driver model, so that the '-gpio'/'-gpios' dependency in dts can be correctly handled by of_devlink/of_fwlink. Signed-off-by: Jianqun Xu --- drivers/pinctrl/pinctrl-rockchip.c | 305 + 1 file changed, 175 insertions(+), 130 deletions(-) diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c index c98bd352f831..2e4fc711d0d1 100644 --- a/drivers/pinctrl/pinctrl-rockchip.c +++ b/drivers/pinctrl/pinctrl-rockchip.c @@ -3370,139 +3370,121 @@ static void rockchip_irq_disable(struct irq_data *d) } static int rockchip_interrupts_register(struct platform_device *pdev, - struct rockchip_pinctrl *info) + struct rockchip_pin_bank *bank) { - struct rockchip_pin_ctrl *ctrl = info->ctrl; - struct rockchip_pin_bank *bank = ctrl->pin_banks; unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN; struct irq_chip_generic *gc; int ret; - int i; - for (i = 0; i < ctrl->nr_banks; ++i, ++bank) { - if (!bank->valid) { - dev_warn(>dev, "bank %s is not valid\n", -bank->name); - continue; - } + if (!bank->valid) { + dev_warn(>dev, "bank %s is not valid\n", +bank->name); + return -EINVAL; + } - ret = clk_enable(bank->clk); - if (ret) { - dev_err(>dev, "failed to enable clock for bank %s\n", - bank->name); - continue; - } + ret = clk_enable(bank->clk); + if (ret) { + dev_err(>dev, "failed to enable clock for bank %s\n", + bank->name); + return ret; + } - bank->domain = irq_domain_add_linear(bank->of_node, 32, - _generic_chip_ops, NULL); - if (!bank->domain) { - dev_warn(>dev, "could not initialize irq domain for bank %s\n", -bank->name); - clk_disable(bank->clk); - continue; - } + bank->domain = irq_domain_add_linear(bank->of_node, 32, + _generic_chip_ops, NULL); + if (!bank->domain) { + dev_warn(>dev, "could not initialize irq domain for bank %s\n", +bank->name); + clk_disable(bank->clk); + return -EINVAL; + } - ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1, -"rockchip_gpio_irq", handle_level_irq, -clr, 0, 0); - if (ret) { - dev_err(>dev, "could not alloc generic chips for bank %s\n", - bank->name); - irq_domain_remove(bank->domain); - clk_disable(bank->clk); - continue; - } + ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1, +"rockchip_gpio_irq", handle_level_irq, +clr, 0, 0); + if (ret) { + dev_err(>dev, "could not alloc generic chips for bank %s\n", + bank->name); + irq_domain_remove(bank->domain); + clk_disable(bank->clk); + return ret; + } - gc = irq_get_domain_generic_chip(bank->domain, 0); - gc->reg_base = bank->reg_base; - gc->private = bank; - gc->chip_types[0].regs.mask = GPIO_INTMASK; - gc->chip_types[0].regs.ack = GPIO_PORTS_EOI; - gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit; - gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit; - gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit; - gc->chip_types[0].chip.irq_enable = rockchip_irq_enable; - gc->chip_types[0].chip.irq_disable = rockchip_irq_disable; - gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake; - gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend; - gc->chip_types[0].chip.irq_resume = rockchip_irq_resume; - gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type; - gc->wake_enabled = IRQ_MSK(bank->nr_pins); + gc = irq_get_domain_generic_chip(bank->domain, 0); + gc->reg_base = bank->reg_base; + gc->private = bank; + gc->chip_types[0].regs.mask = GPIO_INTMASK; + gc->chip_types[0].regs.ack = GPIO_PORTS_EOI; + gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit; +
[PATCH v6 00/35] Improvements for Tegra I2C driver
Hello! This series performs refactoring of the Tegra I2C driver code and hardens the atomic-transfer mode. Changelog: v6: - Added new patch that adds missing RPM puts, thanks to Andy Shevchenko for the suggestion. - Improved commit messages by extending them with more a more detailed explanation of the changes. - Added clarifying comment to the "Use reset_control_reset()" change, which was asked by Andy Shevchenko. - Refactored the "Clean up probe function" patch by moving the dev_err_probe() change into the "Use clk-bulk helpers" patch, which was suggested by Andy Shevchenko. - Improved ordering of the patches like it was suggested by Andy Shevchenko. - Added Andy Shevchenko to suggested-by of the "Use clk-bulk helpers" patch. - Improved "Remove i2c_dev.clk_divisor_non_hs_mode member" patch by making the case-switch to use "fast plus mode" timing if clock rate is out-of-range. Just to make it more consistent. - The "Improve tegra_i2c_dev structure" patch is squashed into "Improve formatting of variables" and "Clean up types/names" patches. - All variable-renaming changes are squashed into a single "Clean up variable names" patch. - Made extra minor improvement to various patches, like more comments and indentations improved. v5: - Dropped the "Factor out runtime PM and hardware initialization" patch, like it was suggested by Michał Mirosław. Instead a less invasive "Factor out hardware initialization into separate function" patch added, it doesn't touch the RPM initialization. - The "Remove outdated barrier()" patch now removes outdated comments. - Updated commit description of the "Remove "dma" variable" patch, saying that the transfer mode may be changed by a callee. This was suggested by Michał Mirosław. - Reworked the "Clean up and improve comments" patch. Couple more comments are corrected and reworded now. - Added r-b's from Michał Mirosław. - New patches: i2c: tegra: Mask interrupt in tegra_i2c_issue_bus_clear() i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear() i2c: tegra: Don't fall back to PIO mode if DMA configuration fails i2c: tegra: Clean up variable types i2c: tegra: Improve tegra_i2c_dev structure v4: - Reordered patches in the fixes/features/cleanups order like it was suggested by Andy Shevchenko. - Now using clk-bulk API, which was suggested by Andy Shevchenko. - Reworked "Make tegra_i2c_flush_fifos() usable in atomic transfer" patch to use iopoll API, which was suggested by Andy Shevchenko. - Separated "Clean up probe function" into several smaller patches. - Squashed "Add missing newline before returns" patch into "Clean up whitespaces, newlines and indentation". - The "Drop '_timeout' from wait/poll function names" is renamed to "Rename wait/poll functions". - The "Use reset_control_reset()" is changed to not fail tegra_i2c_init(), but only emit warning. This should be more friendly behaviour in oppose to having a non-bootable machine if reset-control fails. - New patches: i2c: tegra: Remove error message used for devm_request_irq() failure i2c: tegra: Use devm_platform_get_and_ioremap_resource() i2c: tegra: Use platform_get_irq() i2c: tegra: Use clk-bulk helpers i2c: tegra: Remove bogus barrier() i2c: tegra: Factor out register polling into separate function i2c: tegra: Consolidate error handling in tegra_i2c_xfer_msg() i2c: tegra: Clean up and improve comments i2c: tegra: Rename couple "ret" variables to "err" v3: - Optimized "Make tegra_i2c_flush_fifos() usable in atomic transfer" patch by pre-checking FIFO state before starting to poll using ktime API, which may be expensive under some circumstances. - The "Clean up messages in the code" patch now makes all messages to use proper capitalization of abbreviations. Thanks to Andy Shevchenko and Michał Mirosław for the suggestion. - The "Remove unnecessary whitespaces and newlines" patch is transformed into "Clean up whitespaces and newlines", it now also adds missing newlines and spaces. - Reworked the "Clean up probe function" patch in accordance to suggestion from Michał Mirosław by factoring out only parts of the code that make error unwinding cleaner. - Added r-b from Michał Mirosław. - Added more patches: i2c: tegra: Reorder location of functions in the code i2c: tegra: Factor out packet header setup from tegra_i2c_xfer_msg() i2c: tegra: Remove "dma" variable i2c: tegra: Initialization div-clk rate unconditionally i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member v2: - Cleaned more messages in the "Clean up messages in the code"
[PATCH v6 05/35] i2c: tegra: Initialize div-clk rate unconditionally
It doesn't make sense to conditionalize the div-clk rate changes because rate is fixed and it won't ever change once it's set at the driver's probe time. All further changes are NO-OPs because CCF caches rate and skips rate-change if rate is unchanged. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 34 -- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 1d1ce266255a..720a75439e91 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -293,7 +293,7 @@ struct tegra_i2c_dev { bool is_curr_atomic_xfer; }; -static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool clk_reinit); +static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev); static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long reg) @@ -691,7 +691,7 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev) * domain ON. */ if (i2c_dev->is_vi) { - ret = tegra_i2c_init(i2c_dev, true); + ret = tegra_i2c_init(i2c_dev); if (ret) goto disable_div_clk; } @@ -778,7 +778,7 @@ static void tegra_i2c_vi_init(struct tegra_i2c_dev *i2c_dev) i2c_writel(i2c_dev, 0x0, I2C_TLOW_SEXT); } -static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool clk_reinit) +static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) { u32 val; int err; @@ -836,16 +836,14 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool clk_reinit) if (i2c_dev->hw->has_interface_timing_reg && tsu_thd) i2c_writel(i2c_dev, tsu_thd, I2C_INTERFACE_TIMING_1); - if (!clk_reinit) { - clk_multiplier = (tlow + thigh + 2); - clk_multiplier *= (i2c_dev->clk_divisor_non_hs_mode + 1); - err = clk_set_rate(i2c_dev->div_clk, - i2c_dev->bus_clk_rate * clk_multiplier); - if (err) { - dev_err(i2c_dev->dev, - "failed changing clock rate: %d\n", err); - return err; - } + clk_multiplier = tlow + thigh + 2; + clk_multiplier *= i2c_dev->clk_divisor_non_hs_mode + 1; + + err = clk_set_rate(i2c_dev->div_clk, + i2c_dev->bus_clk_rate * clk_multiplier); + if (err) { + dev_err(i2c_dev->dev, "failed to set div-clk rate: %d\n", err); + return err; } if (!i2c_dev->is_dvc && !i2c_dev->is_vi) { @@ -1319,7 +1317,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, if (!time_left && !completion_done(_dev->dma_complete)) { dev_err(i2c_dev->dev, "DMA transfer timeout\n"); - tegra_i2c_init(i2c_dev, true); + tegra_i2c_init(i2c_dev); return -ETIMEDOUT; } @@ -1340,7 +1338,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, if (time_left == 0) { dev_err(i2c_dev->dev, "i2c transfer timed out\n"); - tegra_i2c_init(i2c_dev, true); + tegra_i2c_init(i2c_dev); return -ETIMEDOUT; } @@ -1352,7 +1350,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, if (likely(i2c_dev->msg_err == I2C_ERR_NONE)) return 0; - tegra_i2c_init(i2c_dev, true); + tegra_i2c_init(i2c_dev); /* start recovery upon arbitration loss in single master mode */ if (i2c_dev->msg_err == I2C_ERR_ARBITRATION_LOST) { if (!i2c_dev->is_multimaster_mode) @@ -1811,7 +1809,7 @@ static int tegra_i2c_probe(struct platform_device *pdev) if (ret < 0) goto disable_div_clk; - ret = tegra_i2c_init(i2c_dev, false); + ret = tegra_i2c_init(i2c_dev); if (ret) { dev_err(>dev, "Failed to initialize i2c controller\n"); goto release_dma; @@ -1918,7 +1916,7 @@ static int __maybe_unused tegra_i2c_resume(struct device *dev) if (err) return err; - err = tegra_i2c_init(i2c_dev, false); + err = tegra_i2c_init(i2c_dev); if (err) return err; -- 2.27.0
[PATCH v6 12/35] i2c: tegra: Use clk-bulk helpers
Use clk-bulk helpers and factor out clocks initialization into separate function in order to make code cleaner. The clocks initialization now performed after reset-control initialization in order to avoid a noisy -PROBE_DEFER errors on T186+ from the clk-bulk helper which doesn't silence this error code. Hence reset_control_get() now may return -EPROBE_DEFER on newer Tegra SoCs because they use BPMP driver that provides reset controls and BPMP doesn't come up early during boot. Previously rst was protected by the clocks retrieval and now this patch makes dev_err_probe() to be used for the rst error handling. Suggested-by: Andy Shevchenko Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 187 - 1 file changed, 67 insertions(+), 120 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 505b5d37077d..d2fe0f1704a5 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -165,9 +165,6 @@ enum msg_end_type { * @has_continue_xfer_support: Continue transfer supports. * @has_per_pkt_xfer_complete_irq: Has enable/disable capability for transfer * complete interrupt per packet basis. - * @has_single_clk_source: The I2C controller has single clock source. Tegra30 - * and earlier SoCs have two clock sources i.e. div-clk and - * fast-clk. * @has_config_load_reg: Has the config load register to load the new * configuration. * @clk_divisor_hs_mode: Clock divisor in HS mode. @@ -208,7 +205,6 @@ enum msg_end_type { struct tegra_i2c_hw_feature { bool has_continue_xfer_support; bool has_per_pkt_xfer_complete_irq; - bool has_single_clk_source; bool has_config_load_reg; int clk_divisor_hs_mode; int clk_divisor_std_mode; @@ -236,7 +232,8 @@ struct tegra_i2c_hw_feature { * @hw: Tegra I2C HW feature * @adapter: core I2C layer adapter information * @div_clk: clock reference for div clock of I2C controller - * @fast_clk: clock reference for fast clock of I2C controller + * @clocks: array of I2C controller clocks + * @nclocks: number of clocks in the array * @rst: reset control for the I2C controller * @base: ioremapped registers cookie * @base_phys: physical base address of the I2C controller @@ -265,8 +262,8 @@ struct tegra_i2c_dev { const struct tegra_i2c_hw_feature *hw; struct i2c_adapter adapter; struct clk *div_clk; - struct clk *fast_clk; - struct clk *slow_clk; + struct clk_bulk_data *clocks; + unsigned int nclocks; struct reset_control *rst; void __iomem *base; phys_addr_t base_phys; @@ -662,25 +659,9 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev) if (ret) return ret; - ret = clk_enable(i2c_dev->fast_clk); - if (ret < 0) { - dev_err(i2c_dev->dev, - "Enabling fast clk failed, err %d\n", ret); + ret = clk_bulk_enable(i2c_dev->nclocks, i2c_dev->clocks); + if (ret) return ret; - } - - ret = clk_enable(i2c_dev->slow_clk); - if (ret < 0) { - dev_err(dev, "failed to enable slow clock: %d\n", ret); - goto disable_fast_clk; - } - - ret = clk_enable(i2c_dev->div_clk); - if (ret < 0) { - dev_err(i2c_dev->dev, - "Enabling div clk failed, err %d\n", ret); - goto disable_slow_clk; - } /* * VI I2C device is attached to VE power domain which goes through @@ -691,17 +672,14 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev) if (i2c_dev->is_vi) { ret = tegra_i2c_init(i2c_dev); if (ret) - goto disable_div_clk; + goto disable_clocks; } return 0; -disable_div_clk: - clk_disable(i2c_dev->div_clk); -disable_slow_clk: - clk_disable(i2c_dev->slow_clk); -disable_fast_clk: - clk_disable(i2c_dev->fast_clk); +disable_clocks: + clk_bulk_disable(i2c_dev->nclocks, i2c_dev->clocks); + return ret; } @@ -709,9 +687,7 @@ static int __maybe_unused tegra_i2c_runtime_suspend(struct device *dev) { struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev); - clk_disable(i2c_dev->div_clk); - clk_disable(i2c_dev->slow_clk); - clk_disable(i2c_dev->fast_clk); + clk_bulk_disable(i2c_dev->nclocks, i2c_dev->clocks); return pinctrl_pm_select_idle_state(i2c_dev->dev); } @@ -1479,7 +1455,6 @@ static struct i2c_bus_recovery_info tegra_i2c_recovery_info = { static const struct tegra_i2c_hw_feature tegra20_i2c_hw = { .has_continue_xfer_support = false, .has_per_pkt_xfer_complete_irq = false, - .has_single_clk_source = false, .clk_divisor_hs_mode = 3,
[PATCH v6 06/35] i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member
The "non_hs_mode" divisor value is fixed, thus there is no need to have the variable i2c_dev.clk_divisor_non_hs_mode struct member. Let's remove it and move the mode selection into tegra_i2c_init() where it can be united with the timing selection. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 46 -- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 720a75439e91..85ed0e02d48c 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -250,7 +250,6 @@ struct tegra_i2c_hw_feature { * @msg_buf_remaining: size of unsent data in the message buffer * @msg_read: identifies read transfers * @bus_clk_rate: current I2C bus clock rate - * @clk_divisor_non_hs_mode: clock divider for non-high-speed modes * @is_multimaster_mode: track if I2C controller is in multi-master mode * @tx_dma_chan: DMA transmit channel * @rx_dma_chan: DMA receive channel @@ -281,7 +280,6 @@ struct tegra_i2c_dev { size_t msg_buf_remaining; int msg_read; u32 bus_clk_rate; - u16 clk_divisor_non_hs_mode; bool is_multimaster_mode; struct dma_chan *tx_dma_chan; struct dma_chan *rx_dma_chan; @@ -783,6 +781,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) u32 val; int err; u32 clk_divisor, clk_multiplier; + u32 non_hs_mode; u32 tsu_thd; u8 tlow, thigh; @@ -805,24 +804,33 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) if (i2c_dev->is_vi) tegra_i2c_vi_init(i2c_dev); - /* Make sure clock divisor programmed correctly */ - clk_divisor = FIELD_PREP(I2C_CLK_DIVISOR_HSMODE, -i2c_dev->hw->clk_divisor_hs_mode) | - FIELD_PREP(I2C_CLK_DIVISOR_STD_FAST_MODE, -i2c_dev->clk_divisor_non_hs_mode); - i2c_writel(i2c_dev, clk_divisor, I2C_CLK_DIVISOR); - - if (i2c_dev->bus_clk_rate > I2C_MAX_STANDARD_MODE_FREQ && - i2c_dev->bus_clk_rate <= I2C_MAX_FAST_MODE_PLUS_FREQ) { + switch (i2c_dev->bus_clk_rate) { + case I2C_MAX_STANDARD_MODE_FREQ + 1 ... I2C_MAX_FAST_MODE_PLUS_FREQ: + default: tlow = i2c_dev->hw->tlow_fast_fastplus_mode; thigh = i2c_dev->hw->thigh_fast_fastplus_mode; tsu_thd = i2c_dev->hw->setup_hold_time_fast_fast_plus_mode; - } else { + + if (i2c_dev->bus_clk_rate > I2C_MAX_FAST_MODE_FREQ) + non_hs_mode = i2c_dev->hw->clk_divisor_fast_plus_mode; + else + non_hs_mode = i2c_dev->hw->clk_divisor_fast_mode; + break; + + case 0 ... I2C_MAX_STANDARD_MODE_FREQ: tlow = i2c_dev->hw->tlow_std_mode; thigh = i2c_dev->hw->thigh_std_mode; tsu_thd = i2c_dev->hw->setup_hold_time_std_mode; + non_hs_mode = i2c_dev->hw->clk_divisor_std_mode; + break; } + /* Make sure clock divisor programmed correctly */ + clk_divisor = FIELD_PREP(I2C_CLK_DIVISOR_HSMODE, +i2c_dev->hw->clk_divisor_hs_mode) | + FIELD_PREP(I2C_CLK_DIVISOR_STD_FAST_MODE, non_hs_mode); + i2c_writel(i2c_dev, clk_divisor, I2C_CLK_DIVISOR); + if (i2c_dev->hw->has_interface_timing_reg) { val = FIELD_PREP(I2C_INTERFACE_TIMING_THIGH, thigh) | FIELD_PREP(I2C_INTERFACE_TIMING_TLOW, tlow); @@ -837,7 +845,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) i2c_writel(i2c_dev, tsu_thd, I2C_INTERFACE_TIMING_1); clk_multiplier = tlow + thigh + 2; - clk_multiplier *= i2c_dev->clk_divisor_non_hs_mode + 1; + clk_multiplier *= non_hs_mode + 1; err = clk_set_rate(i2c_dev->div_clk, i2c_dev->bus_clk_rate * clk_multiplier); @@ -1751,18 +1759,6 @@ static int tegra_i2c_probe(struct platform_device *pdev) goto unprepare_fast_clk; } - if (i2c_dev->bus_clk_rate > I2C_MAX_FAST_MODE_FREQ && - i2c_dev->bus_clk_rate <= I2C_MAX_FAST_MODE_PLUS_FREQ) - i2c_dev->clk_divisor_non_hs_mode = - i2c_dev->hw->clk_divisor_fast_plus_mode; - else if (i2c_dev->bus_clk_rate > I2C_MAX_STANDARD_MODE_FREQ && -i2c_dev->bus_clk_rate <= I2C_MAX_FAST_MODE_FREQ) - i2c_dev->clk_divisor_non_hs_mode = - i2c_dev->hw->clk_divisor_fast_mode; - else - i2c_dev->clk_divisor_non_hs_mode = - i2c_dev->hw->clk_divisor_std_mode; - ret = clk_prepare(i2c_dev->div_clk); if (ret < 0) { dev_err(i2c_dev->dev, "Clock prepare failed %d\n", ret); --
[PATCH v6 07/35] i2c: tegra: Runtime PM always available on Tegra
The runtime PM is guaranteed to be always available on Tegra after commit 40b2bb1b132a ("ARM: tegra: enforce PM requirement"). Hence let's remove all the RPM-availability checking and handling from the code. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 29 ++--- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 85ed0e02d48c..a52c72135390 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -1775,18 +1775,10 @@ static int tegra_i2c_probe(struct platform_device *pdev) if (!i2c_dev->is_vi) pm_runtime_irq_safe(>dev); pm_runtime_enable(>dev); - if (!pm_runtime_enabled(>dev)) { - ret = tegra_i2c_runtime_resume(>dev); - if (ret < 0) { - dev_err(>dev, "runtime resume failed\n"); - goto unprepare_div_clk; - } - } else { - ret = pm_runtime_get_sync(i2c_dev->dev); - if (ret < 0) { - dev_err(>dev, "runtime resume failed\n"); - goto put_rpm; - } + ret = pm_runtime_get_sync(i2c_dev->dev); + if (ret < 0) { + dev_err(dev, "runtime resume failed\n"); + goto put_rpm; } if (i2c_dev->is_multimaster_mode) { @@ -1845,15 +1837,8 @@ static int tegra_i2c_probe(struct platform_device *pdev) clk_disable(i2c_dev->div_clk); put_rpm: - if (pm_runtime_enabled(>dev)) - pm_runtime_put_sync(>dev); - else - tegra_i2c_runtime_suspend(>dev); - - if (pm_runtime_enabled(>dev)) - pm_runtime_disable(>dev); - -unprepare_div_clk: + pm_runtime_put_sync(>dev); + pm_runtime_disable(>dev); clk_unprepare(i2c_dev->div_clk); unprepare_slow_clk: @@ -1875,8 +1860,6 @@ static int tegra_i2c_remove(struct platform_device *pdev) clk_disable(i2c_dev->div_clk); pm_runtime_disable(>dev); - if (!pm_runtime_status_suspended(>dev)) - tegra_i2c_runtime_suspend(>dev); clk_unprepare(i2c_dev->div_clk); clk_unprepare(i2c_dev->slow_clk); -- 2.27.0
[PATCH v6 08/35] i2c: tegra: Remove error message used for devm_request_irq() failure
The error message prints number of vIRQ, which isn't a useful information. In practice devm_request_irq() never fails, hence let's remove the bogus message in order to make code cleaner. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index a52c72135390..b813c0976c10 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -1807,10 +1807,8 @@ static int tegra_i2c_probe(struct platform_device *pdev) ret = devm_request_irq(>dev, i2c_dev->irq, tegra_i2c_isr, IRQF_NO_SUSPEND, dev_name(>dev), i2c_dev); - if (ret) { - dev_err(>dev, "Failed to request irq %i\n", i2c_dev->irq); + if (ret) goto release_dma; - } i2c_set_adapdata(_dev->adapter, i2c_dev); i2c_dev->adapter.owner = THIS_MODULE; -- 2.27.0
[PATCH v6 10/35] i2c: tegra: Use devm_platform_get_and_ioremap_resource()
Driver now uses devm_platform_get_and_ioremap_resource() which replaces the typical boilerplate code and makes code cleaner. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 90ba2f5327c5..c2bbdf92b11f 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -1678,12 +1678,12 @@ static int tegra_i2c_probe(struct platform_device *pdev) int irq; int ret; - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - base_phys = res->start; - base = devm_ioremap_resource(>dev, res); + base = devm_platform_get_and_ioremap_resource(pdev, 0, ); if (IS_ERR(base)) return PTR_ERR(base); + base_phys = res->start; + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { dev_err(>dev, "no irq resource\n"); -- 2.27.0
[PATCH v6 19/35] i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear()
The tegra_i2c_wait_for_config_load() checks for 'has_config_load_reg' by itself, hence there is no need to duplicate the check. Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 2393f52fc584..4e8514696b0c 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -1082,11 +1082,10 @@ static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap) reg = FIELD_PREP(I2C_BC_SCLK_THRESHOLD, 9) | I2C_BC_STOP_COND | I2C_BC_TERMINATE; i2c_writel(i2c_dev, reg, I2C_BUS_CLEAR_CNFG); - if (i2c_dev->hw->has_config_load_reg) { - err = tegra_i2c_wait_for_config_load(i2c_dev); - if (err) - return err; - } + + err = tegra_i2c_wait_for_config_load(i2c_dev); + if (err) + return err; reg |= I2C_BC_ENABLE; i2c_writel(i2c_dev, reg, I2C_BUS_CLEAR_CNFG); -- 2.27.0
[PATCH v6 14/35] i2c: tegra: Clean up probe function
The driver's probe function code is a bit difficult to read. This patch reorders code of the probe function, forming groups of code that are easy to work with. The probe tear-down order now matches the driver-removal order. All dev/>dev are replaced with i2c_dev->dev in order to have uniform code style across the driver. The "ret" variable renamed to "err" since it only carries error code and the new name clearly shows that. Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 141 + 1 file changed, 71 insertions(+), 70 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index e20937041504..01637e1fccde 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -440,6 +440,9 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev) i2c_dev->tx_dma_chan = chan; + i2c_dev->dma_buf_size = i2c_dev->hw->quirks->max_write_len + + I2C_PACKET_HEADER_SIZE; + dma_buf = dma_alloc_coherent(i2c_dev->dev, i2c_dev->dma_buf_size, _phys, GFP_KERNEL | __GFP_NOWARN); if (!dma_buf) { @@ -1690,38 +1693,45 @@ static void tegra_i2c_release_clocks(struct tegra_i2c_dev *i2c_dev) static int tegra_i2c_probe(struct platform_device *pdev) { - struct device *dev = >dev; struct tegra_i2c_dev *i2c_dev; struct resource *res; - void __iomem *base; - phys_addr_t base_phys; - int irq; - int ret; - - base = devm_platform_get_and_ioremap_resource(pdev, 0, ); - if (IS_ERR(base)) - return PTR_ERR(base); - - base_phys = res->start; - - irq = platform_get_irq(pdev, 0); - if (irq < 0) - return irq; + int err; i2c_dev = devm_kzalloc(>dev, sizeof(*i2c_dev), GFP_KERNEL); if (!i2c_dev) return -ENOMEM; - i2c_dev->base = base; - i2c_dev->base_phys = base_phys; - i2c_dev->adapter.algo = _i2c_algo; - i2c_dev->adapter.retries = 1; - i2c_dev->adapter.timeout = 6 * HZ; - i2c_dev->irq = irq; + platform_set_drvdata(pdev, i2c_dev); + + init_completion(_dev->msg_complete); + init_completion(_dev->dma_complete); + + i2c_dev->hw = of_device_get_match_data(>dev); i2c_dev->cont_id = pdev->id; i2c_dev->dev = >dev; - i2c_dev->rst = devm_reset_control_get_exclusive(>dev, "i2c"); + i2c_dev->base = devm_platform_get_and_ioremap_resource(pdev, 0, ); + if (IS_ERR(i2c_dev->base)) + return PTR_ERR(i2c_dev->base); + + i2c_dev->base_phys = res->start; + + err = platform_get_irq(pdev, 0); + if (err < 0) + return err; + + i2c_dev->irq = err; + + /* interrupt will be enabled during of transfer time */ + irq_set_status_flags(i2c_dev->irq, IRQ_NOAUTOEN); + + err = devm_request_irq(i2c_dev->dev, i2c_dev->irq, tegra_i2c_isr, + IRQF_NO_SUSPEND, dev_name(i2c_dev->dev), + i2c_dev); + if (err) + return err; + + i2c_dev->rst = devm_reset_control_get_exclusive(i2c_dev->dev, "i2c"); if (IS_ERR(i2c_dev->rst)) { dev_err_probe(i2c_dev->dev, PTR_ERR(i2c_dev->rst), "failed to get reset control\n"); @@ -1730,18 +1740,13 @@ static int tegra_i2c_probe(struct platform_device *pdev) tegra_i2c_parse_dt(i2c_dev); - ret = tegra_i2c_init_clocks(i2c_dev); - if (ret) - return ret; - - i2c_dev->hw = of_device_get_match_data(>dev); - i2c_dev->adapter.quirks = i2c_dev->hw->quirks; - i2c_dev->dma_buf_size = i2c_dev->adapter.quirks->max_write_len + - I2C_PACKET_HEADER_SIZE; - init_completion(_dev->msg_complete); - init_completion(_dev->dma_complete); + err = tegra_i2c_init_clocks(i2c_dev); + if (err) + return err; - platform_set_drvdata(pdev, i2c_dev); + err = tegra_i2c_init_dma(i2c_dev); + if (err) + goto release_clocks; /* * VI I2C is in VE power domain which is not always on and not @@ -1751,60 +1756,56 @@ static int tegra_i2c_probe(struct platform_device *pdev) * not be used for atomic transfers. */ if (!i2c_dev->is_vi) - pm_runtime_irq_safe(>dev); - pm_runtime_enable(>dev); - ret = pm_runtime_get_sync(i2c_dev->dev); - if (ret < 0) { - dev_err(dev, "runtime resume failed\n"); - goto put_rpm; - } + pm_runtime_irq_safe(i2c_dev->dev); - if (i2c_dev->hw->supports_bus_clear) - i2c_dev->adapter.bus_recovery_info = _i2c_recovery_info; + pm_runtime_enable(i2c_dev->dev); - ret = tegra_i2c_init_dma(i2c_dev); - if (ret < 0) +
[PATCH v6 18/35] i2c: tegra: Remove outdated barrier()
The barrier() was intended to reduce possibility of racing with the interrupt handler, but driver's code evolved significantly and today's driver enables interrupt only when it waits for completion notification. Hence barrier() has no good use anymore, let's remove it. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 29e096422b95..2393f52fc584 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -795,18 +795,17 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev *i2c_dev) words_to_transfer = tx_fifo_avail; /* -* Update state before writing to FIFO. If this casues us -* to finish writing all bytes (AKA buf_remaining goes to 0) we -* have a potential for an interrupt (PACKET_XFER_COMPLETE is -* not maskable). We need to make sure that the isr sees -* buf_remaining as 0 and doesn't call us back re-entrantly. +* Update state before writing to FIFO. Note that this may +* cause us to finish writing all bytes (AKA buf_remaining +* goes to 0), hence we have a potential for an interrupt +* (PACKET_XFER_COMPLETE is not maskable), but GIC interrupt +* is disabled at this point. */ buf_remaining -= words_to_transfer * BYTES_PER_FIFO_WORD; tx_fifo_avail -= words_to_transfer; i2c_dev->msg_buf_remaining = buf_remaining; i2c_dev->msg_buf = buf + words_to_transfer * BYTES_PER_FIFO_WORD; - barrier(); i2c_writesl(i2c_dev, buf, I2C_TX_FIFO, words_to_transfer); @@ -827,10 +826,8 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev *i2c_dev) memcpy(, buf, buf_remaining); val = le32_to_cpu(val); - /* Again update before writing to FIFO to make sure isr sees. */ i2c_dev->msg_buf_remaining = 0; i2c_dev->msg_buf = NULL; - barrier(); i2c_writel(i2c_dev, val, I2C_TX_FIFO); } -- 2.27.0
[PATCH v6 11/35] i2c: tegra: Use platform_get_irq()
Use common helper for retrieval of the interrupt number in order to make code cleaner. Note that platform_get_irq() prints error message by itself. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index c2bbdf92b11f..505b5d37077d 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -1684,12 +1684,9 @@ static int tegra_i2c_probe(struct platform_device *pdev) base_phys = res->start; - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); - if (!res) { - dev_err(>dev, "no irq resource\n"); - return -EINVAL; - } - irq = res->start; + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; div_clk = devm_clk_get(>dev, "div-clk"); if (IS_ERR(div_clk)) { -- 2.27.0
[PATCH v6 09/35] i2c: tegra: Use reset_control_reset()
Use a single reset_control_reset() instead of assert/deasset couple in order to make code cleaner a tad. Note that the reset_control_reset() uses 1 microsecond delay instead of 2 that was used previously, but this shouldn't matter because one microsecond is a default reset time for most of Tegra peripherals and TRM doesn't mention anything special in regards to I2C controller's reset propagation time. In addition don't ignore potential error of the reset control by emitting a noisy warning if it fails, which will indicate an existence of a severe problem, while still allow machine to boot up. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index b813c0976c10..90ba2f5327c5 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -785,9 +785,16 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) u32 tsu_thd; u8 tlow, thigh; - reset_control_assert(i2c_dev->rst); - udelay(2); - reset_control_deassert(i2c_dev->rst); + /* +* The reset shouldn't ever fail in practice. The failure will be a +* sign of a severe problem that needs to be resolved. Still we don't +* want to fail the initialization completely because this may break +* kernel boot up since voltage regulators use I2C. Hence, we will +* emit a noisy warning on error, which won't stay unnoticed and +* won't hose machine entirely. +*/ + err = reset_control_reset(i2c_dev->rst); + WARN_ON_ONCE(err); if (i2c_dev->is_dvc) tegra_dvc_init(i2c_dev); -- 2.27.0
[PATCH v6 13/35] i2c: tegra: Move out all device-tree parsing into tegra_i2c_parse_dt()
Move out code related to device-tree parsing from the probe function into tegra_i2c_parse_dt() in order to make code more consistent. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index d2fe0f1704a5..e20937041504 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -1428,6 +1428,12 @@ static void tegra_i2c_parse_dt(struct tegra_i2c_dev *i2c_dev) multi_mode = of_property_read_bool(np, "multi-master"); i2c_dev->is_multimaster_mode = multi_mode; + + if (of_device_is_compatible(np, "nvidia,tegra20-i2c-dvc")) + i2c_dev->is_dvc = true; + + if (of_device_is_compatible(np, "nvidia,tegra210-i2c-vi")) + i2c_dev->is_vi = true; } static const struct i2c_algorithm tegra_i2c_algo = { @@ -1729,10 +1735,6 @@ static int tegra_i2c_probe(struct platform_device *pdev) return ret; i2c_dev->hw = of_device_get_match_data(>dev); - i2c_dev->is_dvc = of_device_is_compatible(pdev->dev.of_node, - "nvidia,tegra20-i2c-dvc"); - i2c_dev->is_vi = of_device_is_compatible(dev->of_node, -"nvidia,tegra210-i2c-vi"); i2c_dev->adapter.quirks = i2c_dev->hw->quirks; i2c_dev->dma_buf_size = i2c_dev->adapter.quirks->max_write_len + I2C_PACKET_HEADER_SIZE; -- 2.27.0
[PATCH v6 25/35] i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg()
Factor out error recovery code from tegra_i2c_xfer_msg() in order to make this function easier to read and follow. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 46 ++ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 0d823aae9eaf..e53334bc3de3 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -1098,6 +1098,32 @@ static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap) return -EAGAIN; } +static int tegra_i2c_error_recover(struct tegra_i2c_dev *i2c_dev, + struct i2c_msg *msg) +{ + if (i2c_dev->msg_err == I2C_ERR_NONE) + return 0; + + tegra_i2c_init(i2c_dev); + + /* start recovery upon arbitration loss in single master mode */ + if (i2c_dev->msg_err == I2C_ERR_ARBITRATION_LOST) { + if (!i2c_dev->is_multimaster_mode) + return i2c_recover_bus(_dev->adapter); + + return -EAGAIN; + } + + if (i2c_dev->msg_err == I2C_ERR_NO_ACK) { + if (msg->flags & I2C_M_IGNORE_NAK) + return 0; + + return -EREMOTEIO; + } + + return -EIO; +} + static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, struct i2c_msg *msg, enum msg_end_type end_state) @@ -1282,24 +1308,12 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, i2c_dev->msg_err); i2c_dev->is_curr_dma_xfer = false; - if (i2c_dev->msg_err == I2C_ERR_NONE) - return 0; - tegra_i2c_init(i2c_dev); - /* start recovery upon arbitration loss in single master mode */ - if (i2c_dev->msg_err == I2C_ERR_ARBITRATION_LOST) { - if (!i2c_dev->is_multimaster_mode) - return i2c_recover_bus(_dev->adapter); - return -EAGAIN; - } - - if (i2c_dev->msg_err == I2C_ERR_NO_ACK) { - if (msg->flags & I2C_M_IGNORE_NAK) - return 0; - return -EREMOTEIO; - } + err = tegra_i2c_error_recover(i2c_dev, msg); + if (err) + return err; - return -EIO; + return 0; } static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], -- 2.27.0
[PATCH v6 20/35] i2c: tegra: Remove "dma" variable from tegra_i2c_xfer_msg()
The "dma" variable of tegra_i2c_xfer_msg() function doesn't bring much in regards to readability and generation of the code. Besides readability, it's also not very nice that the is_curr_dma_xfer is initialized in tegra_i2c_xfer_msg() and then could be overridden by tegra_i2c_config_fifo_trig(). In a result, the "dma" variable creates slight confusion since it's not instantly obvious why it's set after tegra_i2c_config_fifo_trig(). Hence should be better to have the variable removed. This makes code more consistent. Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 4e8514696b0c..e94da14d36e2 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -1120,7 +1120,6 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, size_t xfer_size; u32 *buffer = NULL; int err = 0; - bool dma; u16 xfer_time = 100; err = tegra_i2c_flush_fifos(i2c_dev); @@ -1143,7 +1142,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, i2c_dev->dma_buf && !i2c_dev->is_curr_atomic_xfer; tegra_i2c_config_fifo_trig(i2c_dev, xfer_size); - dma = i2c_dev->is_curr_dma_xfer; + /* * Transfer time in mSec = Total bits / transfer rate * Total bits = 9 bits per byte (including ACK bit) + Start & stop bits @@ -1153,7 +1152,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, int_mask = I2C_INT_NO_ACK | I2C_INT_ARBITRATION_LOST; tegra_i2c_unmask_irq(i2c_dev, int_mask); - if (dma) { + if (i2c_dev->is_curr_dma_xfer) { if (i2c_dev->msg_read) { dma_sync_single_for_device(i2c_dev->dev, i2c_dev->dma_phys, @@ -1181,13 +1180,13 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, PACKET_HEADER0_PROTOCOL_I2C) | FIELD_PREP(PACKET_HEADER0_CONT_ID, i2c_dev->cont_id) | FIELD_PREP(PACKET_HEADER0_PACKET_ID, 1); - if (dma && !i2c_dev->msg_read) + if (i2c_dev->is_curr_dma_xfer && !i2c_dev->msg_read) *buffer++ = packet_header; else i2c_writel(i2c_dev, packet_header, I2C_TX_FIFO); packet_header = msg->len - 1; - if (dma && !i2c_dev->msg_read) + if (i2c_dev->is_curr_dma_xfer && !i2c_dev->msg_read) *buffer++ = packet_header; else i2c_writel(i2c_dev, packet_header, I2C_TX_FIFO); @@ -1207,13 +1206,13 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, packet_header |= I2C_HEADER_CONT_ON_NAK; if (msg->flags & I2C_M_RD) packet_header |= I2C_HEADER_READ; - if (dma && !i2c_dev->msg_read) + if (i2c_dev->is_curr_dma_xfer && !i2c_dev->msg_read) *buffer++ = packet_header; else i2c_writel(i2c_dev, packet_header, I2C_TX_FIFO); if (!i2c_dev->msg_read) { - if (dma) { + if (i2c_dev->is_curr_dma_xfer) { memcpy(buffer, msg->buf, msg->len); dma_sync_single_for_device(i2c_dev->dev, i2c_dev->dma_phys, @@ -1233,7 +1232,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, if (i2c_dev->hw->has_per_pkt_xfer_complete_irq) int_mask |= I2C_INT_PACKET_XFER_COMPLETE; - if (!dma) { + if (!i2c_dev->is_curr_dma_xfer) { if (msg->flags & I2C_M_RD) int_mask |= I2C_INT_RX_FIFO_DATA_REQ; else if (i2c_dev->msg_buf_remaining) @@ -1244,7 +1243,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, dev_dbg(i2c_dev->dev, "unmasked irq: %02x\n", i2c_readl(i2c_dev, I2C_INT_MASK)); - if (dma) { + if (i2c_dev->is_curr_dma_xfer) { time_left = tegra_i2c_wait_completion_timeout( i2c_dev, _dev->dma_complete, xfer_time); -- 2.27.0
[PATCH v6 15/35] i2c: tegra: Clean up variable types
Don't use signed types for unsigned values and use consistent types for sibling variables. Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 38 +- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 01637e1fccde..8ab31f8100a8 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -206,20 +206,20 @@ struct tegra_i2c_hw_feature { bool has_continue_xfer_support; bool has_per_pkt_xfer_complete_irq; bool has_config_load_reg; - int clk_divisor_hs_mode; - int clk_divisor_std_mode; - int clk_divisor_fast_mode; - u16 clk_divisor_fast_plus_mode; + u32 clk_divisor_hs_mode; + u32 clk_divisor_std_mode; + u32 clk_divisor_fast_mode; + u32 clk_divisor_fast_plus_mode; bool has_multi_master_mode; bool has_slcg_override_reg; bool has_mst_fifo; const struct i2c_adapter_quirks *quirks; bool supports_bus_clear; bool has_apb_dma; - u8 tlow_std_mode; - u8 thigh_std_mode; - u8 tlow_fast_fastplus_mode; - u8 thigh_fast_fastplus_mode; + u32 tlow_std_mode; + u32 thigh_std_mode; + u32 tlow_fast_fastplus_mode; + u32 thigh_fast_fastplus_mode; u32 setup_hold_time_std_mode; u32 setup_hold_time_fast_fast_plus_mode; u32 setup_hold_time_hs_mode; @@ -267,15 +267,15 @@ struct tegra_i2c_dev { struct reset_control *rst; void __iomem *base; phys_addr_t base_phys; - int cont_id; - int irq; - int is_dvc; + unsigned int cont_id; + unsigned int irq; + bool is_dvc; bool is_vi; struct completion msg_complete; int msg_err; u8 *msg_buf; size_t msg_buf_remaining; - int msg_read; + bool msg_read; u32 bus_clk_rate; bool is_multimaster_mode; struct dma_chan *tx_dma_chan; @@ -331,13 +331,13 @@ static u32 i2c_readl(struct tegra_i2c_dev *i2c_dev, unsigned long reg) } static void i2c_writesl(struct tegra_i2c_dev *i2c_dev, void *data, - unsigned long reg, int len) + unsigned long reg, unsigned int len) { writesl(i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg), data, len); } static void i2c_readsl(struct tegra_i2c_dev *i2c_dev, void *data, - unsigned long reg, int len) + unsigned long reg, unsigned int len) { readsl(i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg), data, len); } @@ -506,10 +506,10 @@ static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev) static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev *i2c_dev) { u32 val; - int rx_fifo_avail; + unsigned int rx_fifo_avail; u8 *buf = i2c_dev->msg_buf; size_t buf_remaining = i2c_dev->msg_buf_remaining; - int words_to_transfer; + unsigned int words_to_transfer; /* * Catch overflow due to message fully sent @@ -567,10 +567,10 @@ static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev *i2c_dev) static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev *i2c_dev) { u32 val; - int tx_fifo_avail; + unsigned int tx_fifo_avail; u8 *buf = i2c_dev->msg_buf; size_t buf_remaining = i2c_dev->msg_buf_remaining; - int words_to_transfer; + unsigned int words_to_transfer; if (i2c_dev->hw->has_mst_fifo) { val = i2c_readl(i2c_dev, I2C_MST_FIFO_STATUS); @@ -1178,7 +1178,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, i2c_dev->msg_buf = msg->buf; i2c_dev->msg_buf_remaining = msg->len; i2c_dev->msg_err = I2C_ERR_NONE; - i2c_dev->msg_read = (msg->flags & I2C_M_RD); + i2c_dev->msg_read = !!(msg->flags & I2C_M_RD); reinit_completion(_dev->msg_complete); if (i2c_dev->msg_read) -- 2.27.0
[PATCH v6 21/35] i2c: tegra: Don't fall back to PIO mode if DMA configuration fails
The DMA code path has been tested well enough and the DMA configuration performed by tegra_i2c_config_fifo_trig() shouldn't ever fail in practice. Hence let's remove the obscure transfer-mode switching in order to have a cleaner and simpler code. Now I2C transfer will be failed if DMA configuration fails. Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index e94da14d36e2..ba71b64e5e64 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -940,8 +940,7 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id) return IRQ_HANDLED; } -static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev, - size_t len) +static int tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev, size_t len) { u32 val, reg; u8 dma_burst; @@ -992,12 +991,10 @@ static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev, if (ret < 0) { dev_err(i2c_dev->dev, "DMA slave config failed: %d\n", ret); - dev_err(i2c_dev->dev, "falling back to PIO\n"); - tegra_i2c_release_dma(i2c_dev); - i2c_dev->is_curr_dma_xfer = false; - } else { - goto out; + return ret; } + + goto out; } if (i2c_dev->hw->has_mst_fifo) @@ -1008,6 +1005,8 @@ static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev, I2C_FIFO_CONTROL_RX_TRIG(1); out: i2c_writel(i2c_dev, val, reg); + + return 0; } static unsigned long @@ -1141,7 +1140,10 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, i2c_dev->is_curr_dma_xfer = (xfer_size > I2C_PIO_MODE_PREFERRED_LEN) && i2c_dev->dma_buf && !i2c_dev->is_curr_atomic_xfer; - tegra_i2c_config_fifo_trig(i2c_dev, xfer_size); + + err = tegra_i2c_config_fifo_trig(i2c_dev, xfer_size); + if (err) + return err; /* * Transfer time in mSec = Total bits / transfer rate -- 2.27.0
[PATCH v6 17/35] i2c: tegra: Remove likely/unlikely from the code
The likely/unlikely annotations should be used only in a hot paths of performance-critical code. The I2C driver doesn't have such paths, and thus, there is no justification for usage of likely/unlikely annotations in the code. Hence remove them. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 542436cb7823..29e096422b95 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -855,7 +855,7 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id) goto err; } - if (unlikely(status & status_err)) { + if (status & status_err) { tegra_i2c_disable_packet_mode(i2c_dev); if (status & I2C_INT_NO_ACK) i2c_dev->msg_err |= I2C_ERR_NO_ACK; @@ -1297,7 +1297,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, i2c_dev->msg_err); i2c_dev->is_curr_dma_xfer = false; - if (likely(i2c_dev->msg_err == I2C_ERR_NONE)) + if (i2c_dev->msg_err == I2C_ERR_NONE) return 0; tegra_i2c_init(i2c_dev); -- 2.27.0
[PATCH v6 27/35] i2c: tegra: Factor out register polling into separate function
Factor out register polling into a separate function in order to remove boilerplate code and make code cleaner. Reviewed-by: Michał Mirosław Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 43 +++--- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 7d54b8b3ed9a..2e7beadd381f 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -518,10 +518,24 @@ static void tegra_i2c_vi_init(struct tegra_i2c_dev *i2c_dev) i2c_writel(i2c_dev, 0x0, I2C_TLOW_SEXT); } +static int tegra_i2c_poll_register(struct tegra_i2c_dev *i2c_dev, + u32 reg, u32 mask, u32 delay_us, + u32 timeout_us) +{ + void __iomem *addr = i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg); + u32 val; + + if (!i2c_dev->is_curr_atomic_xfer) + return readl_relaxed_poll_timeout(addr, val, !(val & mask), + delay_us, timeout_us); + + return readl_relaxed_poll_timeout_atomic(addr, val, !(val & mask), +delay_us, timeout_us); +} + static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev) { - u32 mask, val, offset, reg_offset; - void __iomem *addr; + u32 mask, val, offset; int err; if (i2c_dev->hw->has_mst_fifo) { @@ -538,16 +552,7 @@ static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev) val |= mask; i2c_writel(i2c_dev, val, offset); - reg_offset = tegra_i2c_reg_addr(i2c_dev, offset); - addr = i2c_dev->base + reg_offset; - - if (i2c_dev->is_curr_atomic_xfer) - err = readl_relaxed_poll_timeout_atomic(addr, val, !(val & mask), - 1000, 100); - else - err = readl_relaxed_poll_timeout(addr, val, !(val & mask), -1000, 100); - + err = tegra_i2c_poll_register(i2c_dev, offset, mask, 1000, 100); if (err) { dev_err(i2c_dev->dev, "failed to flush FIFO\n"); return err; @@ -557,25 +562,15 @@ static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev) static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev) { - unsigned long reg_offset; - void __iomem *addr; - u32 val; int err; if (!i2c_dev->hw->has_config_load_reg) return 0; - reg_offset = tegra_i2c_reg_addr(i2c_dev, I2C_CONFIG_LOAD); - addr = i2c_dev->base + reg_offset; i2c_writel(i2c_dev, I2C_MSTR_CONFIG_LOAD, I2C_CONFIG_LOAD); - if (i2c_dev->is_curr_atomic_xfer) - err = readl_relaxed_poll_timeout_atomic(addr, val, val == 0, 1000, - I2C_CONFIG_LOAD_TIMEOUT); - else - err = readl_relaxed_poll_timeout(addr, val, val == 0, 1000, -I2C_CONFIG_LOAD_TIMEOUT); - + err = tegra_i2c_poll_register(i2c_dev, I2C_CONFIG_LOAD, 0x, + 1000, I2C_CONFIG_LOAD_TIMEOUT); if (err) { dev_warn(i2c_dev->dev, "timeout waiting for config load\n"); return err; -- 2.27.0