[PATCH v5 1/3] venus: core: handle race condititon for core ops
For core ops we are having only write protect but there
is no read protect, because of this in multithreading
and concurrency, one CPU core is reading without wait
which is causing the NULL pointer dereferece crash.
one such scenario is as show below, where in one CPU
core, core->ops becoming NULL and in another CPU core
calling core->ops->session_init().
CPU: core-7:
Call trace:
hfi_session_init+0x180/0x1dc [venus_core]
vdec_queue_setup+0x9c/0x364 [venus_dec]
vb2_core_reqbufs+0x1e4/0x368 [videobuf2_common]
vb2_reqbufs+0x4c/0x64 [videobuf2_v4l2]
v4l2_m2m_reqbufs+0x50/0x84 [v4l2_mem2mem]
v4l2_m2m_ioctl_reqbufs+0x2c/0x38 [v4l2_mem2mem]
v4l_reqbufs+0x4c/0x5c
__video_do_ioctl+0x2b0/0x39c
CPU: core-0:
Call trace:
venus_shutdown+0x98/0xfc [venus_core]
venus_sys_error_handler+0x64/0x148 [venus_core]
process_one_work+0x210/0x3d0
worker_thread+0x248/0x3f4
kthread+0x11c/0x12c
Signed-off-by: Mansur Alisha Shaik
Acked-by: Stanimir Varbanov
---
Changes in v5:
- Addressed review comments by stan in v4 version
drivers/media/platform/qcom/venus/hfi.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/hfi.c
b/drivers/media/platform/qcom/venus/hfi.c
index a59022a..638ed5c 100644
--- a/drivers/media/platform/qcom/venus/hfi.c
+++ b/drivers/media/platform/qcom/venus/hfi.c
@@ -198,6 +198,18 @@ int hfi_session_init(struct venus_inst *inst, u32 pixfmt)
const struct hfi_ops *ops = core->ops;
int ret;
+ /*
+* If core shutdown is in progress or if we are in system
+* recovery, return an error as during system error recovery
+* session_init() can't pass successfully
+*/
+ mutex_lock(&core->lock);
+ if (!core->ops || core->sys_error) {
+ mutex_unlock(&core->lock);
+ return -EIO;
+ }
+ mutex_unlock(&core->lock);
+
if (inst->state != INST_UNINIT)
return -EINVAL;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH v5 2/3] venus: handle use after free for iommu_map/iommu_unmap
In concurrency usecase and reboot scenario we are seeing muliple
crashes related to iommu_map/iommu_unamp of core->fw.iommu_domain.
In one case we are seeing "Unable to handle kernel NULL pointer
dereference at virtual address 0008" crash, this is
because of core->fw.iommu_domain in venus_firmware_deinit() and
trying to map in venus_boot() during venus_sys_error_handler()
Call trace:
__iommu_map+0x4c/0x348
iommu_map+0x5c/0x70
venus_boot+0x184/0x230 [venus_core]
venus_sys_error_handler+0xa0/0x14c [venus_core]
process_one_work+0x210/0x3d0
worker_thread+0x248/0x3f4
kthread+0x11c/0x12c
ret_from_fork+0x10/0x18
In second case we are seeing "Unable to handle kernel paging request
at virtual address 006b6b6b6b6b6b9b" crash, this is because of
unmapping iommu domain which is already unmapped.
Call trace:
venus_remove+0xf8/0x108 [venus_core]
venus_core_shutdown+0x1c/0x34 [venus_core]
platform_drv_shutdown+0x28/0x34
device_shutdown+0x154/0x1fc
kernel_restart_prepare+0x40/0x4c
kernel_restart+0x1c/0x64
__arm64_sys_reboot+0x190/0x238
el0_svc_common+0xa4/0x154
el0_svc_compat_handler+0x2c/0x38
el0_svc_compat+0x8/0x10
Signed-off-by: Mansur Alisha Shaik
---
drivers/media/platform/qcom/venus/firmware.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/firmware.c
b/drivers/media/platform/qcom/venus/firmware.c
index 1db64a8..d03e2dd 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -171,9 +171,14 @@ static int venus_shutdown_no_tz(struct venus_core *core)
iommu = core->fw.iommu_domain;
- unmapped = iommu_unmap(iommu, VENUS_FW_START_ADDR, mapped);
- if (unmapped != mapped)
- dev_err(dev, "failed to unmap firmware\n");
+ if (core->fw.mapped_mem_size && iommu) {
+ unmapped = iommu_unmap(iommu, VENUS_FW_START_ADDR, mapped);
+
+ if (unmapped != mapped)
+ dev_err(dev, "failed to unmap firmware\n");
+ else
+ core->fw.mapped_mem_size = 0;
+ }
return 0;
}
@@ -305,7 +310,11 @@ void venus_firmware_deinit(struct venus_core *core)
iommu = core->fw.iommu_domain;
iommu_detach_device(iommu, core->fw.dev);
- iommu_domain_free(iommu);
+
+ if (core->fw.iommu_domain) {
+ iommu_domain_free(iommu);
+ core->fw.iommu_domain = NULL;
+ }
platform_device_unregister(to_platform_device(core->fw.dev));
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH v5 3/3] venus: core: add shutdown callback for venus
After the SMMU translation is disabled in the
arm-smmu shutdown callback during reboot, if
any subsystem are still alive then IOVAs they
are using will become PAs on bus, which may
lead to crash.
So implemented shutdown callback, which detach iommu maps.
Signed-off-by: Mansur Alisha Shaik
Acked-by: Stanimir Varbanov
---
drivers/media/platform/qcom/venus/core.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/core.c
b/drivers/media/platform/qcom/venus/core.c
index 6103aaf..65b71ac 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -345,6 +345,14 @@ static int venus_remove(struct platform_device *pdev)
return ret;
}
+static void venus_core_shutdown(struct platform_device *pdev)
+{
+ struct venus_core *core = platform_get_drvdata(pdev);
+
+ venus_shutdown(core);
+ venus_firmware_deinit(core);
+}
+
static __maybe_unused int venus_runtime_suspend(struct device *dev)
{
struct venus_core *core = dev_get_drvdata(dev);
@@ -602,6 +610,7 @@ static struct platform_driver qcom_venus_driver = {
.of_match_table = venus_dt_match,
.pm = &venus_pm_ops,
},
+ .shutdown = venus_core_shutdown,
};
module_platform_driver(qcom_venus_driver);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH v5 0/3] venus: core: add shutdown callback for venus
Add shutdown callback for venus driver. Handle race conditions in concurrency usecases like multiple browser YouTube browser tabs(approx 50 tabs) graphics_Stress, WiFi ON/OFF, Bluetooth ON/OF, and reboot in parallel. Mansur Alisha Shaik (3): venus: core: handle race condititon for core ops venus: handle use after free for iommu_map/iommu_unmap venus: core: add shutdown callback for venus drivers/media/platform/qcom/venus/core.c | 9 + drivers/media/platform/qcom/venus/firmware.c | 17 + drivers/media/platform/qcom/venus/hfi.c | 12 3 files changed, 34 insertions(+), 4 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] omap3: enable off mode automatically
On Fri, 11 Sep 2020 18:12:09 +0200 Andreas Kemnade wrote: > Enabling off mode was only reachable deeply hidden > in the debugfs. As powersaving is an important feature, > move the option out of its shady place. > The debugfs file can still be used to override the default. > > Use the presence of a device compatible to ti,twl4030-idle or > ti,twl4030-idle-osc-off as an indicator that the board is wired correctly > for off mode. > > Signed-off-by: Andreas Kemnade > --- > An earlier version of this patch was here: > https://patchwork.kernel.org/patch/10794121/ > > A config option was used instead of the suggested devicetree check. > > Changes in v2: > - fix compile without CONFIG_ARCH_OMAP3 > The variable enable_off_mode is now always a real one and not > a preprocessor constant to avoid trouble with unusual configurations. > Anything I still missed here? Regards, Andreas
[PATCH 3/4] habanalabs: add debug messages for opening/closing context
During debugging of error we sometimes need to know whether the error
happened when a user context was open. Add debug prints when opening and
closing user contexts.
Signed-off-by: Oded Gabbay
---
drivers/misc/habanalabs/common/context.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/misc/habanalabs/common/context.c
b/drivers/misc/habanalabs/common/context.c
index df8171a2226c..bd03ef074eed 100644
--- a/drivers/misc/habanalabs/common/context.c
+++ b/drivers/misc/habanalabs/common/context.c
@@ -28,6 +28,8 @@ static void hl_ctx_fini(struct hl_ctx *ctx)
kfree(ctx->cs_pending);
if (ctx->asid != HL_KERNEL_ASID_ID) {
+ dev_dbg(hdev->dev, "closing user context %d\n", ctx->asid);
+
/* The engines are stopped as there is no executing CS, but the
* Coresight might be still working by accessing addresses
* related to the stopped engines. Hence stop it explicitly.
@@ -41,6 +43,7 @@ static void hl_ctx_fini(struct hl_ctx *ctx)
hl_vm_ctx_fini(ctx);
hl_asid_free(hdev, ctx->asid);
} else {
+ dev_dbg(hdev->dev, "closing kernel context\n");
hl_mmu_ctx_fini(ctx);
}
}
@@ -168,6 +171,8 @@ int hl_ctx_init(struct hl_device *hdev, struct hl_ctx *ctx,
bool is_kernel_ctx)
dev_err(hdev->dev, "ctx_init failed\n");
goto err_cb_va_pool_fini;
}
+
+ dev_dbg(hdev->dev, "create user context %d\n", ctx->asid);
}
return 0;
--
2.17.1
[PATCH 4/4] habanalabs: add notice of device not idle
The device should be idle after a context is closed. If not, print a
notice.
Signed-off-by: Oded Gabbay
---
drivers/misc/habanalabs/common/context.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/misc/habanalabs/common/context.c
b/drivers/misc/habanalabs/common/context.c
index bd03ef074eed..7a59dd7c6450 100644
--- a/drivers/misc/habanalabs/common/context.c
+++ b/drivers/misc/habanalabs/common/context.c
@@ -12,6 +12,7 @@
static void hl_ctx_fini(struct hl_ctx *ctx)
{
struct hl_device *hdev = ctx->hdev;
+ u64 idle_mask = 0;
int i;
/*
@@ -42,6 +43,13 @@ static void hl_ctx_fini(struct hl_ctx *ctx)
hl_cb_va_pool_fini(ctx);
hl_vm_ctx_fini(ctx);
hl_asid_free(hdev, ctx->asid);
+
+ if ((!hdev->pldm) && (hdev->pdev) &&
+ (!hdev->asic_funcs->is_device_idle(hdev,
+ &idle_mask, NULL)))
+ dev_notice(hdev->dev,
+ "device not idle after user context is closed
(0x%llx)\n",
+ idle_mask);
} else {
dev_dbg(hdev->dev, "closing kernel context\n");
hl_mmu_ctx_fini(ctx);
--
2.17.1
[PATCH 2/4] habanalabs: release kernel context after hw_fini
Some engines use resources that belong to the kernel context (e.g. MMU
mappings). In case the halt-engines doesn't work properly due to H/W
restriction, we need to make sure the kernel context lives on until after
the hw_fini. The hw_fini resets the ASIC after that no engine is alive and
we can safely close the kernel context.
Signed-off-by: Oded Gabbay
---
drivers/misc/habanalabs/common/device.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/habanalabs/common/device.c
b/drivers/misc/habanalabs/common/device.c
index 196e35d71118..20572224099a 100644
--- a/drivers/misc/habanalabs/common/device.c
+++ b/drivers/misc/habanalabs/common/device.c
@@ -967,14 +967,13 @@ int hl_device_reset(struct hl_device *hdev, bool
hard_reset,
flush_workqueue(hdev->eq_wq);
}
- /* Release kernel context */
- if ((hard_reset) && (hl_ctx_put(hdev->kernel_ctx) == 1))
- hdev->kernel_ctx = NULL;
-
/* Reset the H/W. It will be in idle state after this returns */
hdev->asic_funcs->hw_fini(hdev, hard_reset);
if (hard_reset) {
+ /* Release kernel context */
+ if (hl_ctx_put(hdev->kernel_ctx) == 1)
+ hdev->kernel_ctx = NULL;
hl_vm_fini(hdev);
hl_mmu_fini(hdev);
hl_eq_reset(hdev, &hdev->event_queue);
@@ -1465,13 +1464,13 @@ void hl_device_fini(struct hl_device *hdev)
hl_cb_pool_fini(hdev);
+ /* Reset the H/W. It will be in idle state after this returns */
+ hdev->asic_funcs->hw_fini(hdev, true);
+
/* Release kernel context */
if ((hdev->kernel_ctx) && (hl_ctx_put(hdev->kernel_ctx) != 1))
dev_err(hdev->dev, "kernel ctx is still alive\n");
- /* Reset the H/W. It will be in idle state after this returns */
- hdev->asic_funcs->hw_fini(hdev, true);
-
hl_vm_fini(hdev);
hl_mmu_fini(hdev);
--
2.17.1
[PATCH 1/4] habanalabs: correct an error message
We don't try to allocate huge pages here so remove the huge word.
Signed-off-by: Oded Gabbay
---
drivers/misc/habanalabs/common/memory.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/habanalabs/common/memory.c
b/drivers/misc/habanalabs/common/memory.c
index 3324332811bc..84227819e4d1 100644
--- a/drivers/misc/habanalabs/common/memory.c
+++ b/drivers/misc/habanalabs/common/memory.c
@@ -77,8 +77,8 @@ static int alloc_device_memory(struct hl_ctx *ctx, struct
hl_mem_in *args,
paddr = (u64) gen_pool_alloc(vm->dram_pg_pool, total_size);
if (!paddr) {
dev_err(hdev->dev,
- "failed to allocate %llu huge contiguous
pages\n",
- num_pgs);
+ "failed to allocate %llu contiguous pages with
total size of %llu\n",
+ num_pgs, total_size);
return -ENOMEM;
}
}
--
2.17.1
Re: [PATCH v1 3/4] mm: introduce page memcg flags
On Tue, Sep 22, 2020 at 1:38 PM Roman Gushchin wrote:
>
> The lowest bit in page->memcg_data is used to distinguish between
> struct memory_cgroup pointer and a pointer to a objcgs array.
> All checks and modifications of this bit are open-coded.
>
> Let's formalize it using page memcg flags, defined in page_memcg_flags
> enum and replace all open-coded accesses with test_bit()/__set_bit().
>
> Few additional flags might be added later. Flags are intended to be
> mutually exclusive.
Why mutually exclusive? I understand mutual exclusion between non-slab
kernel memory and objcgs vector but future feature might not need to
be mutually exclusive.
One use-case I am thinking of is actually using a couple of bits here
to store more idle (or hot) age by future extension of DAMON. That
would be for user memory (anon or file and not slab or kmem) but
multiple bits can set.
>
> Signed-off-by: Roman Gushchin
> ---
> include/linux/memcontrol.h | 29 +++--
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index ab3ea3e90583..9a49f1e1c0c7 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -343,6 +343,11 @@ struct mem_cgroup {
>
> extern struct mem_cgroup *root_mem_cgroup;
>
> +enum page_memcg_flags {
> + /* page->memcg_data is a pointer to an objcgs vector */
> + PG_MEMCG_OBJ_CGROUPS,
> +};
If you agree with my next comment then I think PG_MEMCG_LAST_FLAG and
MEMCG_FLAGS_MASK should be introduced in this patch instead of the
next one.
> +
> /*
> * page_mem_cgroup - get the memory cgroup associated with a page
> * @page: a pointer to the page struct
> @@ -371,13 +376,7 @@ static inline struct mem_cgroup
> *page_mem_cgroup_check(struct page *page)
> {
> unsigned long memcg_data = page->memcg_data;
>
> - /*
> -* The lowest bit set means that memcg isn't a valid
> -* memcg pointer, but a obj_cgroups pointer.
> -* In this case the page is shared and doesn't belong
> -* to any specific memory cgroup.
> -*/
> - if (memcg_data & 0x1UL)
> + if (test_bit(PG_MEMCG_OBJ_CGROUPS, &memcg_data))
> return NULL;
>
> return (struct mem_cgroup *)memcg_data;
> @@ -422,7 +421,13 @@ static inline void clear_page_mem_cgroup(struct page
> *page)
> */
> static inline struct obj_cgroup **page_obj_cgroups(struct page *page)
> {
> - return (struct obj_cgroup **)(page->memcg_data & ~0x1UL);
> + unsigned long memcg_data = page->memcg_data;
> +
> + VM_BUG_ON_PAGE(memcg_data && !test_bit(PG_MEMCG_OBJ_CGROUPS,
> + &memcg_data), page);
> + __clear_bit(PG_MEMCG_OBJ_CGROUPS, &memcg_data);
> +
> + return (struct obj_cgroup **)memcg_data;
Wouldn't the following be more future proof?
return (struct obj_cgroup **)(memcg_data & ~MEMCG_FLAGS_MASK);
> }
>
> /*
> @@ -437,7 +442,7 @@ static inline struct obj_cgroup
> **page_obj_cgroups_check(struct page *page)
> {
> unsigned long memcg_data = page->memcg_data;
>
> - if (memcg_data && (memcg_data & 0x1UL))
> + if (memcg_data && test_bit(PG_MEMCG_OBJ_CGROUPS, &memcg_data))
> return (struct obj_cgroup **)memcg_data;
>
> return NULL;
> @@ -453,7 +458,11 @@ static inline struct obj_cgroup
> **page_obj_cgroups_check(struct page *page)
> static inline bool set_page_obj_cgroups(struct page *page,
> struct obj_cgroup **objcgs)
> {
> - return !cmpxchg(&page->memcg_data, 0, (unsigned long)objcgs | 0x1UL);
> + unsigned long memcg_data = (unsigned long)objcgs;
> +
> + __set_bit(PG_MEMCG_OBJ_CGROUPS, &memcg_data);
> +
> + return !cmpxchg(&page->memcg_data, 0, memcg_data);
> }
>
> /*
> --
> 2.26.2
>
Re: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings for nxp,pcf2127
Hello, On Thu, Sep 24, 2020 at 03:20:33AM +, Qiang Zhao wrote: > On 21/09/2020 13:48:19+0800, Qiang Zhao wrote: > > > -Original Message- > > From: Alexandre Belloni > > Sent: 2020年9月23日 17:45 > > To: Qiang Zhao > > Cc: Wim Van Sebroeck ; Guenter Roeck > > ; linux-watch...@vger.kernel.org; > > a.zu...@towertech.it; robh...@kernel.org; linux-...@vger.kernel.org; > > devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Uwe Kleine-König > > > > Subject: Re: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings for > > nxp,pcf2127 > > > > Hi, > > > > You forgot to copy the watchdog maintainers, I think such a property should > > be > > discussed with them. > > > > Note that I'm still convinced this is not a complete solution, see: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern > > el.org%2Flinux-rtc%2F20200716181816.GF3428%40piout.net%2F&data= > > 02%7C01%7Cqiang.zhao%40nxp.com%7Cb71f79a044b0493d6d4f08d85fa551c > > b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637364510931174 > > 355&sdata=%2BOxrzB8RIuxM9Let5slhfCVmMm6PMNoERDeHC9%2Fdxng > > %3D&reserved=0 haha > Yes, you are right, There is not a fundamental solution. > However it somewhat avoid this situation at least. > > And if without this issue, > is it correct to register a rtc device as watchdog no matter it is used as > watchdog on the board? > Every time Linux are booted up, watchdog device should be configured to the > right one manually. > So the patch are useful, even though it is not for the issue. > > What should we do to really resolve this issue? I still think we need a kernel solution here. I would expect that most assembled pcf2127 chips are unable to act as a watchdog (i.e. don't have the RST output connected to something that resets the machine). So my favoured solution would be a positive property like: has-watchdog; or something similar. In my eyes this is definitely something we want to specify in the device tree because it is a relevant hardware property. I consider it a bug to give a watchdog device to userspace that isn't functional. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH] ARM: dts: document pinctrl-single,pins when #pinctrl-cells = 2
* Trent Piepho [200924 06:31]: > On Wed, Sep 23, 2020 at 11:06 PM Tony Lindgren wrote: > > > > * Trent Piepho [200924 05:49]: > > > On Wed, Sep 23, 2020 at 10:43 PM Tony Lindgren wrote: > > > > > > > > * Trent Piepho [200924 01:34]: > > > > > On Tue, Sep 22, 2020 at 11:57 PM Tony Lindgren > > > > > wrote: > > > > > > > > > > > > Also FYI, folks have also complained for a long time that the > > > > > > pinctrl-single > > > > > > binding mixes mux and conf values while they should be handled > > > > > > separately. > > > > > > > > > > > > > > > > Instead of combining two fields when the dts is generated they are now > > > > > combined when the pinctrl-single driver reads the dts. Other than > > > > > this detail, the result is the same. The board dts source is the > > > > > same. The value programmed into the pinctrl register is the same. > > > > > There is no mechanism currently that can alter that value in any way. > > > > > > > > > > What does combining them later allow that is not possible now? > > > > > > > > It now allows further driver changes to manage conf and mux separately > > > > :) > > > > > > The pinctrl-single driver? How will that work with boards that are > > > not am335x and don't use conf and mux fields in the same manner as > > > am335x? > > > > For those cases we still have #pinctrl-cells = <1>. > > If pincntrl-single is going to be am335x specific, then shouldn't it > be a different compatible string? Certainly different compatible strings can be used as needed. But pinctrl-single is not going to be am335x specific though :) We have quite a few SoCs using it: $ git grep pinctrl-single,function-mask arch/*/boot/dts/ | wc -l 41 > Are the driver changes something that can be not be done with the > pinconf-single properties? They all include a mask. Sure but in the long term we're better off with using #pinctrl-cells along the lines what we have for example for #interrupt-cells and #gpio-cells. Regards, Tony
Re: [PATCH] PCI: dwc: Move allocate and map page for msi out of dw_pcie_msi_init()
Hi Rob,
On Wed, 23 Sep 2020 10:41:45 -0600 Rob Herring wrote:
>
> On Wed, Sep 23, 2020 at 12:27 AM Jisheng Zhang
> wrote:
> >
> > Currently, dw_pcie_msi_init() allocates and maps page for msi, then
> > program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex
> > may lose power during suspend-to-RAM, so when we resume, we want to
> > redo the latter but not the former. If designware based driver (for
> > example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the
> > previous msi page will be leaked.
>
> It's worse than this. I think there's also error paths too leaking the
I think you mean the leaking in pcie-tegra194.c's error path. Synaptics
SoC pcie driver(not mainlined) needs to call dw_pcie_msi_init() in resume
path, pcie-tegra194.c shares the same problem, so I mentioned it in the commit
msg, but the patch isn't targeting to fix all the leaking issues in
pcie-tegra194.c. This patch at least fix one of the issue.
> page. Also, there's never a dma_unmap_page call which should happen
> before freeing.
Thanks for pointing it out. I will add it in v2
>
> > Move the allocate and map msi page from dw_pcie_msi_init() to
> > dw_pcie_host_init() to fix this problem.
> >
> > Fixes: 56e15a238d92 ("PCI: tegra: Add Tegra194 PCIe support")
> > Signed-off-by: Jisheng Zhang
> > ---
> > drivers/pci/controller/dwc/pci-dra7xx.c | 18 -
> > .../pci/controller/dwc/pcie-designware-host.c | 27 +--
> > 2 files changed, 30 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c
> > b/drivers/pci/controller/dwc/pci-dra7xx.c
> > index dc387724cf08..4301cf844a4c 100644
> > --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> > +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> > @@ -490,7 +490,9 @@ static struct irq_chip dra7xx_pci_msi_bottom_irq_chip =
> > {
> > static int dra7xx_pcie_msi_host_init(struct pcie_port *pp)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > + struct device *dev = pci->dev;
> > u32 ctrl, num_ctrls;
> > + int ret;
> >
> > pp->msi_irq_chip = &dra7xx_pci_msi_bottom_irq_chip;
> >
> > @@ -506,7 +508,21 @@ static int dra7xx_pcie_msi_host_init(struct pcie_port
> > *pp)
> > ~0);
> > }
> >
> > - return dw_pcie_allocate_domains(pp);
> > + ret = dw_pcie_allocate_domains(pp);
> > + if (ret)
> > + return ret;
> > +
> > + pp->msi_page = alloc_page(GFP_KERNEL);
> > + pp->msi_data = dma_map_page(dev, pp->msi_page, 0, PAGE_SIZE,
> > + DMA_FROM_DEVICE);
> > + ret = dma_mapping_error(dev, pp->msi_data);
> > + if (ret) {
> > + dev_err(dev, "Failed to map MSI data\n");
> > + __free_page(pp->msi_page);
> > + pp->msi_page = NULL;
> > + dw_pcie_free_msi(pp);
> > + }
>
> I don't like having 2 copies of the same thing. Also, doesn't keystone
> need this too?
what about introduce dw_pcie_msi_alloc() to do this?
IIUC, keystone doesn't need this.
>
> The other thing is .msi_host_init() is abused by having an empty
> function to disable MSI support. We should have a flag instead to
> enable/disable MSI support and then we can key off of that in the
> common code.
FWICT, the .msi_host_init() is to init soc's own msi support rather
than init the DWC's integrated MSI module. So the usage is correct.
>
> > + return ret;
> > }
> >
> > static const struct dw_pcie_host_ops dra7xx_pcie_host_ops = {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
> > b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 9dafecba347f..c23ba64f64fe 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -294,20 +294,7 @@ void dw_pcie_free_msi(struct pcie_port *pp)
> >
> > void dw_pcie_msi_init(struct pcie_port *pp)
>
> Might be good to rename this function with exactly what it does.
> There's too many 'init' and 'setup' functions...
If we move the msi page allocation out of dw_pcie_msi_init(), then
it only initializes the integrated MSI.
Thanks
>
> > {
> > - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > - struct device *dev = pci->dev;
> > - u64 msi_target;
> > -
> > - pp->msi_page = alloc_page(GFP_KERNEL);
> > - pp->msi_data = dma_map_page(dev, pp->msi_page, 0, PAGE_SIZE,
> > - DMA_FROM_DEVICE);
> > - if (dma_mapping_error(dev, pp->msi_data)) {
> > - dev_err(dev, "Failed to map MSI data\n");
> > - __free_page(pp->msi_page);
> > - pp->msi_page = NULL;
> > - return;
> > - }
> > - msi_target = (u64)pp->msi_data;
> > + u64 msi_target = (u64)pp->msi_data;
> >
> > /* Program the msi_data */
> > dw_pcie_wr_own_conf(pp, PCIE_MSI_ADD
Re: [PATCH v1 4/4] mm: convert page kmemcg type to a page memcg flag
On Tue, Sep 22, 2020 at 1:37 PM Roman Gushchin wrote: > > PageKmemcg flag is currently defined as a page type (like buddy, > offline, table and guard). Semantically it means that the page > was accounted as a kernel memory by the page allocator and has > to be uncharged on the release. > > As a side effect of defining the flag as a page type, the accounted > page can't be mapped to userspace (look at page_has_type() and > comments above). In particular, this blocks the accounting of > vmalloc-backed memory used by some bpf maps, because these maps > do map the memory to userspace. > > One option is to fix it by complicating the access to page->mapcount, > which provides some free bits for page->page_type. > > But it's way better to move this flag into page->memcg_data flags. > Indeed, the flag makes no sense without enabled memory cgroups > and memory cgroup pointer set in particular. > > This commit replaces PageKmemcg() and __SetPageKmemcg() with > PageMemcgKmem() and SetPageMemcgKmem(). __ClearPageKmemcg() > can be simple deleted because clear_page_mem_cgroup() already > does the job. > > As a bonus, on !CONFIG_MEMCG build the PageMemcgKmem() check will > be compiled out. > > Signed-off-by: Roman Gushchin Reviewed-by: Shakeel Butt
Re: [PATCH v2] omap3: enable off mode automatically
* Andreas Kemnade [200924 07:00]: > On Fri, 11 Sep 2020 18:12:09 +0200 > Andreas Kemnade wrote: > > > Enabling off mode was only reachable deeply hidden > > in the debugfs. As powersaving is an important feature, > > move the option out of its shady place. > > The debugfs file can still be used to override the default. > > > > Use the presence of a device compatible to ti,twl4030-idle or > > ti,twl4030-idle-osc-off as an indicator that the board is wired correctly > > for off mode. > > > > Signed-off-by: Andreas Kemnade > > --- > > An earlier version of this patch was here: > > https://patchwork.kernel.org/patch/10794121/ > > > > A config option was used instead of the suggested devicetree check. > > > > Changes in v2: > > - fix compile without CONFIG_ARCH_OMAP3 > > The variable enable_off_mode is now always a real one and not > > a preprocessor constant to avoid trouble with unusual configurations. > > > Anything I still missed here? No the missing part is just me picking up the remaining patches for v5.10 that I'll hopefully manage to do today :) Regards, Tony
Re: [net] net: mscc: ocelot: fix fields offset in SG_CONFIG_REG_3
Hi, On 24/09/2020 10:11:13+0800, Xiaoliang Yang wrote: > INIT_IPS and GATE_ENABLE fields have a wrong offset in SG_CONFIG_REG_3. You are changing GATE_STATE, not GATE_ENABLE > This register is used by stream gate control of PSFP, and it has not > been used before, because PSFP is not implemented in ocelot driver. > > Signed-off-by: Xiaoliang Yang > --- > include/soc/mscc/ocelot_ana.h | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/soc/mscc/ocelot_ana.h b/include/soc/mscc/ocelot_ana.h > index 841c6ec22b64..1669481d9779 100644 > --- a/include/soc/mscc/ocelot_ana.h > +++ b/include/soc/mscc/ocelot_ana.h > @@ -252,10 +252,10 @@ > #define ANA_SG_CONFIG_REG_3_LIST_LENGTH_M GENMASK(18, 16) > #define ANA_SG_CONFIG_REG_3_LIST_LENGTH_X(x) (((x) & > GENMASK(18, 16)) >> 16) > #define ANA_SG_CONFIG_REG_3_GATE_ENABLE BIT(20) > -#define ANA_SG_CONFIG_REG_3_INIT_IPS(x) (((x) << 24) & > GENMASK(27, 24)) > -#define ANA_SG_CONFIG_REG_3_INIT_IPS_MGENMASK(27, 24) > -#define ANA_SG_CONFIG_REG_3_INIT_IPS_X(x) (((x) & > GENMASK(27, 24)) >> 24) > -#define ANA_SG_CONFIG_REG_3_INIT_GATE_STATE BIT(28) > +#define ANA_SG_CONFIG_REG_3_INIT_IPS(x) (((x) << 21) & > GENMASK(24, 21)) > +#define ANA_SG_CONFIG_REG_3_INIT_IPS_MGENMASK(24, 21) > +#define ANA_SG_CONFIG_REG_3_INIT_IPS_X(x) (((x) & > GENMASK(24, 21)) >> 21) > +#define ANA_SG_CONFIG_REG_3_INIT_GATE_STATE BIT(25) > VSC7514 doesn't have the stream gate registers ans this was generated automatically from the cml file for felix. Did that change? Seeing that bits in this register are not packed, I would believe your change is correct. > #define ANA_SG_GCL_GS_CONFIG_RSZ 0x4 > > -- > 2.17.1 > -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH] Bluetooth: btusb: Avoid unnecessary reset upon system resume
[+Cc linux-usb]
Hi Abhishek,
> On Sep 24, 2020, at 04:41, Abhishek Pandit-Subedi
> wrote:
>
> Hi Kai-Heng,
>
> Which Realtek controller is this on?'
The issue happens on 8821CE.
>
> Specifically for RTL8822CE, we tested without reset_resume being set
> and that was causing the controller being reset without bluez ever
> learning about it (resulting in devices being unusable without
> toggling the BT power).
The reset is done by the kernel, so how does that affect bluez?
>From what you described, it sounds more like runtime resume since bluez is
>already running.
If we need reset resume for runtime resume, maybe it's another bug which needs
to be addressed?
> If the firmware doesn't cut off power during suspend, maybe you
> shouldn't set the BTUSB_WAKEUP_DISABLE flag for that controller.
We don't know beforehand if the platform firmware (BIOS for my case) will cut
power off or not.
In general, laptops will cut off the USB power during S3.
When AC is plugged, some laptops cuts USB power off and some don't. This also
applies to many desktops. Not to mention there can be BIOS options to control
USB power under S3/S4/S5...
So we don't know beforehand.
>
> I would prefer this doesn't get accepted in its current state.
Of course.
I think we need to find the root cause for your case before applying this one.
Kai-Heng
>
> Abhishek
>
> On Wed, Sep 23, 2020 at 10:56 AM Kai-Heng Feng
> wrote:
>>
>> Realtek bluetooth controller may fail to work after system sleep:
>> [ 1272.707670] Bluetooth: hci0: command 0x1001 tx timeout
>> [ 1280.835712] Bluetooth: hci0: RTL: HCI_OP_READ_LOCAL_VERSION failed (-110)
>>
>> If platform firmware doesn't cut power off during suspend, the firmware
>> is considered retained in controller but the driver is still asking USB
>> core to perform a reset-resume. This can make bluetooth controller
>> unusable.
>>
>> So avoid unnecessary reset to resolve the issue.
>>
>> For devices that really lose power during suspend, USB core will detect
>> and handle reset-resume correctly.
>>
>> Signed-off-by: Kai-Heng Feng
>> ---
>> drivers/bluetooth/btusb.c | 8 +++-
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index 8d2608ddfd08..de86ef4388f9 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -4255,17 +4255,15 @@ static int btusb_suspend(struct usb_interface *intf,
>> pm_message_t message)
>>enable_irq(data->oob_wake_irq);
>>}
>>
>> - /* For global suspend, Realtek devices lose the loaded fw
>> -* in them. But for autosuspend, firmware should remain.
>> -* Actually, it depends on whether the usb host sends
>> + /* For global suspend, Realtek devices lose the loaded fw in them if
>> +* platform firmware cut power off. But for autosuspend, firmware
>> +* should remain. Actually, it depends on whether the usb host sends
>> * set feature (enable wakeup) or not.
>> */
>>if (test_bit(BTUSB_WAKEUP_DISABLE, &data->flags)) {
>>if (PMSG_IS_AUTO(message) &&
>>device_can_wakeup(&data->udev->dev))
>>data->udev->do_remote_wakeup = 1;
>> - else if (!PMSG_IS_AUTO(message))
>> - data->udev->reset_resume = 1;
>>}
>>
>>return 0;
>> --
>> 2.17.1
>>
[RESEND v2 PATCH 1/3] spi: spi-zynqmp-gqspi: Fix kernel-doc warnings
From: Amit Kumar Mahapatra
Fix kernel-doc warnings in ZynqMP qspi driver file.
Signed-off-by: Amit Kumar Mahapatra
Signed-off-by: Michal Simek
---
drivers/spi/spi-zynqmp-gqspi.c | 45 +-
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index e17a20125255..b479b9c3d1e6 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -173,9 +173,10 @@ struct zynqmp_qspi {
};
/**
- * zynqmp_gqspi_read: For GQSPI controller read operation
+ * zynqmp_gqspi_read - For GQSPI controller read operation
* @xqspi: Pointer to the zynqmp_qspi structure
* @offset:Offset from where to read
+ * Return: Value at the offset
*/
static u32 zynqmp_gqspi_read(struct zynqmp_qspi *xqspi, u32 offset)
{
@@ -183,7 +184,7 @@ static u32 zynqmp_gqspi_read(struct zynqmp_qspi *xqspi, u32
offset)
}
/**
- * zynqmp_gqspi_write: For GQSPI controller write operation
+ * zynqmp_gqspi_write - For GQSPI controller write operation
* @xqspi: Pointer to the zynqmp_qspi structure
* @offset:Offset where to write
* @val: Value to be written
@@ -195,7 +196,7 @@ static inline void zynqmp_gqspi_write(struct zynqmp_qspi
*xqspi, u32 offset,
}
/**
- * zynqmp_gqspi_selectslave: For selection of slave device
+ * zynqmp_gqspi_selectslave - For selection of slave device
* @instanceptr: Pointer to the zynqmp_qspi structure
* @slavecs: For chip select
* @slavebus: To check which bus is selected- upper or lower
@@ -242,7 +243,7 @@ static void zynqmp_gqspi_selectslave(struct zynqmp_qspi
*instanceptr,
}
/**
- * zynqmp_qspi_init_hw:Initialize the hardware
+ * zynqmp_qspi_init_hw - Initialize the hardware
* @xqspi: Pointer to the zynqmp_qspi structure
*
* The default settings of the QSPI controller's configurable parameters on
@@ -330,7 +331,7 @@ static void zynqmp_qspi_init_hw(struct zynqmp_qspi *xqspi)
}
/**
- * zynqmp_qspi_copy_read_data: Copy data to RX buffer
+ * zynqmp_qspi_copy_read_data - Copy data to RX buffer
* @xqspi: Pointer to the zynqmp_qspi structure
* @data: The variable where data is stored
* @size: Number of bytes to be copied from data to RX buffer
@@ -344,7 +345,7 @@ static void zynqmp_qspi_copy_read_data(struct zynqmp_qspi
*xqspi,
}
/**
- * zynqmp_prepare_transfer_hardware: Prepares hardware for transfer.
+ * zynqmp_prepare_transfer_hardware - Prepares hardware for transfer.
* @master:Pointer to the spi_master structure which provides
* information about the controller.
*
@@ -361,7 +362,7 @@ static int zynqmp_prepare_transfer_hardware(struct
spi_master *master)
}
/**
- * zynqmp_unprepare_transfer_hardware: Relaxes hardware after transfer
+ * zynqmp_unprepare_transfer_hardware - Relaxes hardware after transfer
* @master:Pointer to the spi_master structure which provides
* information about the controller.
*
@@ -378,7 +379,7 @@ static int zynqmp_unprepare_transfer_hardware(struct
spi_master *master)
}
/**
- * zynqmp_qspi_chipselect: Select or deselect the chip select line
+ * zynqmp_qspi_chipselect - Select or deselect the chip select line
* @qspi: Pointer to the spi_device structure
* @is_high: Select(0) or deselect (1) the chip select line
*/
@@ -423,7 +424,7 @@ static void zynqmp_qspi_chipselect(struct spi_device *qspi,
bool is_high)
}
/**
- * zynqmp_qspi_setup_transfer: Configure QSPI controller for specified
+ * zynqmp_qspi_setup_transfer - Configure QSPI controller for specified
* transfer
* @qspi: Pointer to the spi_device structure
* @transfer: Pointer to the spi_transfer structure which provides
@@ -482,7 +483,7 @@ static int zynqmp_qspi_setup_transfer(struct spi_device
*qspi,
}
/**
- * zynqmp_qspi_setup: Configure the QSPI controller
+ * zynqmp_qspi_setup - Configure the QSPI controller
* @qspi: Pointer to the spi_device structure
*
* Sets the operational mode of QSPI controller for the next QSPI transfer,
@@ -498,7 +499,7 @@ static int zynqmp_qspi_setup(struct spi_device *qspi)
}
/**
- * zynqmp_qspi_filltxfifo: Fills the TX FIFO as long as there is room in
+ * zynqmp_qspi_filltxfifo - Fills the TX FIFO as long as there is room in
* the FIFO or the bytes required to be
* transmitted.
* @xqspi: Pointer to the zynqmp_qspi structure
@@ -524,7 +525,7 @@ static void zynqmp_qspi_filltxfifo(struct zynqmp_qspi
*xqspi, int size)
}
/**
- * zynqmp_qspi_readrxfifo: Fills the RX FIFO as long as there is room in
+ * zynqmp_qspi_readrxfifo - Fills the RX FIFO as long as there is room in
* the FIFO.
* @xqspi: Pointer to the zynqmp_qspi structure
* @size: Number of bytes to be copied from RX buffer to RX FIFO
@@ -552,7 +553,7 @@ static void zy
RE: [PATCH 4/4] habanalabs: add notice of device not idle
On Thu, Sep 24, 2020 at 10:03 AM Oded Gabbay wrote: > The device should be idle after a context is closed. If not, print a > notice. > > Signed-off-by: Oded Gabbay This patch-set is: Reviewed-by: Tomer Tayar
Re: [PATCH 1/6] seccomp: Introduce SECCOMP_PIN_ARCHITECTURE
On Thu, Sep 24, 2020 at 02:41:36AM +0200, Jann Horn wrote:
> On Thu, Sep 24, 2020 at 1:29 AM Kees Cook wrote:
> > For systems that provide multiple syscall maps based on audit
> > architectures (e.g. AUDIT_ARCH_X86_64 and AUDIT_ARCH_I386 via
> > CONFIG_COMPAT) or via syscall masks (e.g. x86_x32), allow a fast way
> > to pin the process to a specific syscall table, instead of needing
> > to generate all filters with an architecture check as the first filter
> > action.
> >
> > This creates the internal representation that seccomp itself can use
> > (which is separate from the filters, which need to stay runtime
> > agnostic). Additionally paves the way for constant-action bitmaps.
>
> I don't really see the point in providing this UAPI - the syscall
> number checking will probably have much more performance cost than the
> architecture number check, and it's not like this lets us avoid the
> check, we're just moving it over into C code.
It's desirable for libseccomp and is a request from systemd (which is,
at this point, the largest seccomp user I know of), as they have no way
to force an arch without doing it in filters, which doesn't help much
with reducing filter runtime.
>
> > Signed-off-by: Kees Cook
> > ---
> > include/linux/seccomp.h | 9 +++
> > include/uapi/linux/seccomp.h | 1 +
> > kernel/seccomp.c | 79 ++-
> > tools/testing/selftests/seccomp/seccomp_bpf.c | 33
> > 4 files changed, 120 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> > index 02aef2844c38..0be20bc81ea9 100644
> > --- a/include/linux/seccomp.h
> > +++ b/include/linux/seccomp.h
> > @@ -20,12 +20,18 @@
> > #include
> > #include
> >
> > +#define SECCOMP_ARCH_IS_NATIVE 1
> > +#define SECCOMP_ARCH_IS_COMPAT 2
>
> FYI, mips has three different possible "arch" values (per kernel build
> config; the __AUDIT_ARCH_LE flag can also be set, but that's fixed
> based on the config):
>
> - AUDIT_ARCH_MIPS
> - AUDIT_ARCH_MIPS | __AUDIT_ARCH_64BIT
> - AUDIT_ARCH_MIPS | __AUDIT_ARCH_64BIT | __AUDIT_ARCH_CONVENTION_MIPS64_N32
>
> But I guess we can deal with that once someone wants to actually add
> support for this on mips.
Yup!
>
> > +#define SECCOMP_ARCH_IS_MULTIPLEX 3
>
> Why should X32 be handled specially? If the seccomp filter allows
Because it's a masked lookup into a separate table; the syscalls don't
map to x86_64's table; so for seccomp to correctly figure out which
bitmap to use, it has to do this decoding.
> specific syscalls (as it should), we don't have to care about X32.
> Only in weird cases where the seccomp filter wants to deny specific
> syscalls (a horrible idea), X32 is a concern, and in such cases, the
> userspace code can generate a single conditional jump to deal with it.
I feel like I must not understand what you mean. The x32-aware seccomp
filters are using syscall tests with 0x4000 included in the values.
So seccomp's bitmap cannot handle this because it must know how many
syscalls to include in a linearly-allocated bitmap.
> And when seccomp is used properly to allow specific syscalls, the
> kernel will just waste time uselessly checking this X32 stuff.
It not measurable in my tests -- seccomp_data::nr is rather hot in the
cache. ;) That said, if it's unwanted, then CONFIG_X86_X32=n is the way
to go.
> [...]
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> [...]
> > +static long seccomp_pin_architecture(void)
> > +{
> > +#ifdef SECCOMP_ARCH
> > + struct task_struct *task = current;
> > +
> > + u8 arch = seccomp_get_arch(syscall_get_arch(task),
> > + syscall_get_nr(task,
> > task_pt_regs(task)));
> > +
> > + /* How did you even get here? */
>
> Via a racing TSYNC, that's how.
Yes; thanks. This will need to take ¤t->sighand->siglock.
>
> > + if (task->seccomp.arch && task->seccomp.arch != arch)
> > + return -EBUSY;
> > +
> > + task->seccomp.arch = arch;
> > +#endif
> > + return 0;
> > +}
>
> Why does this return 0 if SECCOMP_ARCH is not defined? That suggests
> to userspace that we have successfully pinned the ABI, even though
> we're actually unable to do so.
Yup; thanks for the catch. This is a logical leftover from the RFC. This
should be, I think:
+ task->seccomp.arch = arch;
+ return 0;
+#else
+ return -EINVAL;
+#endif
--
Kees Cook
Re: [EXT] Re: [PATCH] net: fec: Keep device numbering consistent with datasheet
Hi Andy, David and Andrew, first of all, thanks for your review. I really appreciate it! On 24.09.20 08:36, Andy Duan wrote: From: David Miller Sent: Thursday, September 24, 2020 4:32 AM From: Stefan Riedmueller Date: Wed, 23 Sep 2020 16:25:28 +0200 From: Christian Hemp Make use of device tree alias for device enumeration to keep the device order consistent with the naming in the datasheet. Otherwise for the i.MX 6UL/ULL the ENET1 interface is enumerated as eth1 and ENET2 as eth0. Signed-off-by: Christian Hemp Signed-off-by: Stefan Riedmueller Device naming and ordering for networking devices was never, ever, guaranteed. Use udev or similar. @@ -3691,6 +3692,10 @@ fec_probe(struct platform_device *pdev) ndev->max_mtu = PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN; + eth_id = of_alias_get_id(pdev->dev.of_node, "ethernet"); + if (eth_id >= 0) + sprintf(ndev->name, "eth%d", eth_id); You can't ever just write into ndev->name, what if another networking device is already using that name? This change is incorrect on many levels. David is correct. For example, imx8DXL has ethernet0 is EQOS TSN, ethernet1 is FEC. EQOS TSN is andother driver and is registered early, the dev->name is eth0. So the patch will bring conflict in such case. I was not aware of that conflict, but now that you mention it it makes total sense. I wanted to make life a little easier for myself but underestimated the global context. I will try to find a solution with udev or something similar. So please drop this patch and sorry for the noise. Stefan Andy
Re: [PATCH v2] MAINTAINERS: add Dan Murphy as TP LP8xxx drivers maintainer
On Wed, 23 Sep 2020 at 22:01, Jonathan Cameron wrote: > > On Wed, 23 Sep 2020 11:53:33 -0500 > Dan Murphy wrote: > > > Hello > > > > On 9/22/20 10:28 AM, Krzysztof Kozlowski wrote: > > > Milo Kim's email in TI bounces with permanent error (550: Invalid > > > recipient). Last email from him on LKML was in 2017. Move Milo Kim to > > > credits and add Dan Murphy from TI to look after: > > > - TI LP855x backlight driver, > > > - TI LP8727 charger driver, > > > - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers. > > > > > > Cc: Dan Murphy > > > Signed-off-by: Krzysztof Kozlowski > > > > Acked-by: Dan Murphy > > > Not sure who will pick this one up, but > Acked-by: Jonathan Cameron I guess whoever is first. :) This spans across systems but the common part is MFD, so maybe Lee - could you pick it up? Best regards, Krzysztof
[RESEND v2 PATCH 0/3] spi: spi-zynqmp-gqspi: Update driver to use spi-mem framework
Hi, I am resending this series instead of Amit because his patches are broken in lore. Mark has reported the issue with it and I clearly see that patch is broken in lore. For example: https://lore.kernel.org/linux-spi/20200922164016.30979-2-amit.kumar-mahapa...@xilinx.com/raw There is additional = which shouldn't be there. @@ -183,7 +184,7 @@ static u32 zynqmp_gqspi_read(struct zynqmp_qspi *xqspi,= u32 offset) We will investigate why this is happening but it is related to any his setting. Thanks, Michal This patch series: - Fixes kernel-doc warnings in ZynqMP qspi controller driver file. - Updates the ZynqMP qspi controller driver to use spi-mem framework. - Fixes incorrect indentation in ZynqMP qspi controller driver file. Tested: flashcp and mtd_utils Branch: for-5.10 HEAD commit: bf253e6bf6b8 (spi/for-5.10) spi: spi-imx: spi_imx_transfer(): add support for effective_speed_hz Amit Kumar Mahapatra (3): spi: spi-zynqmp-gqspi: Fix kernel-doc warnings spi: spi-zynqmp-gqspi: Update driver to use spi-mem framework spi: spi-zynqmp-gqspi: Fix incorrect indentation drivers/spi/spi-zynqmp-gqspi.c | 720 +++-- 1 file changed, 407 insertions(+), 313 deletions(-) -- 2.28.0
[RESEND v2 PATCH 3/3] spi: spi-zynqmp-gqspi: Fix incorrect indentation
From: Amit Kumar Mahapatra
Fixed incorrect indentation in ZynqMP qspi controller driver.
Addresses-checkpatch: "Alignment should match open parenthesis"
Signed-off-by: Amit Kumar Mahapatra
Signed-off-by: Michal Simek
---
drivers/spi/spi-zynqmp-gqspi.c | 46 +-
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index 7f57923f76ea..c8fa6ee18ae7 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -326,8 +326,8 @@ static void zynqmp_qspi_init_hw(struct zynqmp_qspi *xqspi)
GQSPI_SELECT_FLASH_BUS_LOWER);
/* Initialize DMA */
zynqmp_gqspi_write(xqspi,
- GQSPI_QSPIDMA_DST_CTRL_OFST,
- GQSPI_QSPIDMA_DST_CTRL_RESET_VAL);
+ GQSPI_QSPIDMA_DST_CTRL_OFST,
+ GQSPI_QSPIDMA_DST_CTRL_RESET_VAL);
/* Enable the GQSPI */
zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
@@ -374,8 +374,8 @@ static void zynqmp_qspi_chipselect(struct spi_device *qspi,
bool is_high)
/* Manually start the generic FIFO command */
zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
- zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
- GQSPI_CFG_START_GEN_FIFO_MASK);
+ zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
+ GQSPI_CFG_START_GEN_FIFO_MASK);
timeout = jiffies + msecs_to_jiffies(1000);
@@ -384,10 +384,9 @@ static void zynqmp_qspi_chipselect(struct spi_device
*qspi, bool is_high)
statusreg = zynqmp_gqspi_read(xqspi, GQSPI_ISR_OFST);
if ((statusreg & GQSPI_ISR_GENFIFOEMPTY_MASK) &&
- (statusreg & GQSPI_ISR_TXEMPTY_MASK))
+ (statusreg & GQSPI_ISR_TXEMPTY_MASK))
break;
- else
- cpu_relax();
+ cpu_relax();
} while (!time_after_eq(jiffies, timeout));
if (time_after_eq(jiffies, timeout))
@@ -549,7 +548,7 @@ static void zynqmp_qspi_readrxfifo(struct zynqmp_qspi
*xqspi, u32 size)
while ((count < size) && (xqspi->bytes_to_receive > 0)) {
if (xqspi->bytes_to_receive >= 4) {
- (*(u32 *) xqspi->rxbuf) =
+ (*(u32 *)xqspi->rxbuf) =
zynqmp_gqspi_read(xqspi, GQSPI_RXD_OFST);
xqspi->rxbuf += 4;
xqspi->bytes_to_receive -= 4;
@@ -645,14 +644,14 @@ static void zynqmp_process_dma_irq(struct zynqmp_qspi
*xqspi)
u32 config_reg, genfifoentry;
dma_unmap_single(xqspi->dev, xqspi->dma_addr,
- xqspi->dma_rx_bytes, DMA_FROM_DEVICE);
+xqspi->dma_rx_bytes, DMA_FROM_DEVICE);
xqspi->rxbuf += xqspi->dma_rx_bytes;
xqspi->bytes_to_receive -= xqspi->dma_rx_bytes;
xqspi->dma_rx_bytes = 0;
/* Disabling the DMA interrupts */
zynqmp_gqspi_write(xqspi, GQSPI_QSPIDMA_DST_I_DIS_OFST,
- GQSPI_QSPIDMA_DST_I_EN_DONE_MASK);
+ GQSPI_QSPIDMA_DST_I_EN_DONE_MASK);
if (xqspi->bytes_to_receive > 0) {
/* Switch to IO mode,for remaining bytes to receive */
@@ -670,14 +669,15 @@ static void zynqmp_process_dma_irq(struct zynqmp_qspi
*xqspi)
/* Manual start */
zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
- (zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
- GQSPI_CFG_START_GEN_FIFO_MASK));
+ (zynqmp_gqspi_read(xqspi,
+ GQSPI_CONFIG_OFST) |
+ GQSPI_CFG_START_GEN_FIFO_MASK));
/* Enable the RX interrupts for IO mode */
zynqmp_gqspi_write(xqspi, GQSPI_IER_OFST,
- GQSPI_IER_GENFIFOEMPTY_MASK |
- GQSPI_IER_RXNEMPTY_MASK |
- GQSPI_IER_RXEMPTY_MASK);
+ GQSPI_IER_GENFIFOEMPTY_MASK |
+ GQSPI_IER_RXNEMPTY_MASK |
+ GQSPI_IER_RXEMPTY_MASK);
}
}
@@ -708,7 +708,7 @@ static irqreturn_t zynqmp_qspi_irq(int irq, void *dev_id)
dma_status =
zynqmp_gqspi_read(xqspi, GQSPI_QSPIDMA_DST_I_STS_OFST);
zynqmp_gqspi_write(xqspi, GQSPI_QSPIDMA_DST_I_STS_OFST,
- dma_status);
+ dma_status);
}
if (mask & GQSPI_ISR_TXNOT_FULL_MASK) {
@@ -725,8 +725,8 @@ static irqreturn_t zynqmp_qspi_irq(int irq, void *dev_id)
[RESEND v2 PATCH 2/3] spi: spi-zynqmp-gqspi: Update driver to use spi-mem framework
From: Amit Kumar Mahapatra
Updated Zynqmp qspi controller driver to use spi-mem framework.
Signed-off-by: Amit Kumar Mahapatra
Signed-off-by: Michal Simek
---
drivers/spi/spi-zynqmp-gqspi.c | 645 +++--
1 file changed, 369 insertions(+), 276 deletions(-)
diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index b479b9c3d1e6..7f57923f76ea 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
/* Generic QSPI register offsets */
#define GQSPI_CONFIG_OFST 0x0100
@@ -153,6 +154,7 @@ enum mode_type {GQSPI_MODE_IO, GQSPI_MODE_DMA};
* @dma_addr: DMA address after mapping the kernel buffer
* @genfifoentry: Used for storing the genfifoentry instruction.
* @mode: Defines the mode in which QSPI is operating
+ * @data_completion: completion structure
*/
struct zynqmp_qspi {
void __iomem *regs;
@@ -170,6 +172,7 @@ struct zynqmp_qspi {
dma_addr_t dma_addr;
u32 genfifoentry;
enum mode_type mode;
+ struct completion data_completion;
};
/**
@@ -344,40 +347,6 @@ static void zynqmp_qspi_copy_read_data(struct zynqmp_qspi
*xqspi,
xqspi->bytes_to_receive -= size;
}
-/**
- * zynqmp_prepare_transfer_hardware - Prepares hardware for transfer.
- * @master:Pointer to the spi_master structure which provides
- * information about the controller.
- *
- * This function enables SPI master controller.
- *
- * Return: 0 on success; error value otherwise
- */
-static int zynqmp_prepare_transfer_hardware(struct spi_master *master)
-{
- struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
-
- zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
- return 0;
-}
-
-/**
- * zynqmp_unprepare_transfer_hardware - Relaxes hardware after transfer
- * @master:Pointer to the spi_master structure which provides
- * information about the controller.
- *
- * This function disables the SPI master controller.
- *
- * Return: Always 0
- */
-static int zynqmp_unprepare_transfer_hardware(struct spi_master *master)
-{
- struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
-
- zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
- return 0;
-}
-
/**
* zynqmp_qspi_chipselect - Select or deselect the chip select line
* @qspi: Pointer to the spi_device structure
@@ -387,12 +356,14 @@ static void zynqmp_qspi_chipselect(struct spi_device
*qspi, bool is_high)
{
struct zynqmp_qspi *xqspi = spi_master_get_devdata(qspi->master);
ulong timeout;
- u32 genfifoentry = 0x0, statusreg;
+ u32 genfifoentry = 0, statusreg;
genfifoentry |= GQSPI_GENFIFO_MODE_SPI;
- genfifoentry |= xqspi->genfifobus;
if (!is_high) {
+ xqspi->genfifobus = GQSPI_GENFIFO_BUS_LOWER;
+ xqspi->genfifocs = GQSPI_GENFIFO_CS_LOWER;
+ genfifoentry |= xqspi->genfifobus;
genfifoentry |= xqspi->genfifocs;
genfifoentry |= GQSPI_GENFIFO_CS_SETUP;
} else {
@@ -424,11 +395,38 @@ static void zynqmp_qspi_chipselect(struct spi_device
*qspi, bool is_high)
}
/**
- * zynqmp_qspi_setup_transfer - Configure QSPI controller for specified
+ * zynqmp_qspi_selectspimode - Selects SPI mode - x1 or x2 or x4.
+ * @xqspi: xqspi is a pointer to the GQSPI instance
+ * @spimode: spimode - SPI or DUAL or QUAD.
+ * Return: Mask to set desired SPI mode in GENFIFO entry.
+ */
+static inline u32 zynqmp_qspi_selectspimode(struct zynqmp_qspi *xqspi,
+ u8 spimode)
+{
+ u32 mask = 0;
+
+ switch (spimode) {
+ case GQSPI_SELECT_MODE_DUALSPI:
+ mask = GQSPI_GENFIFO_MODE_DUALSPI;
+ break;
+ case GQSPI_SELECT_MODE_QUADSPI:
+ mask = GQSPI_GENFIFO_MODE_QUADSPI;
+ break;
+ case GQSPI_SELECT_MODE_SPI:
+ mask = GQSPI_GENFIFO_MODE_SPI;
+ break;
+ default:
+ dev_warn(xqspi->dev, "Invalid SPI mode\n");
+ }
+
+ return mask;
+}
+
+/**
+ * zynqmp_qspi_config_op - Configure QSPI controller for specified
* transfer
+ * @xqspi: Pointer to the zynqmp_qspi structure
* @qspi: Pointer to the spi_device structure
- * @transfer: Pointer to the spi_transfer structure which provides
- * information about next transfer setup parameters
*
* Sets the operational mode of QSPI controller for the next QSPI transfer and
* sets the requested clock frequency.
@@ -445,17 +443,11 @@ static void zynqmp_qspi_chipselect(struct spi_device
*qspi, bool is_high)
* by the QSPI controller the driver will set the highest or lowest
* frequency supported by controller.
*/
-static int zynqmp_qspi_setup_transfer(struct spi_device *qs
Re: [RFC PATCH 6/9] surface_aggregator: Add dedicated bus and device type
On Wed, Sep 23, 2020 at 11:12:49PM +0200, Maximilian Luz wrote:
> On 9/23/20 7:33 PM, Greg Kroah-Hartman wrote:
> > On Wed, Sep 23, 2020 at 05:15:08PM +0200, Maximilian Luz wrote:
> [...]
>
> > Overall, nice work on this patch, the integration to the driver core
> > looks totally correct. Great job.
>
> Thanks!
>
> > A few minor nits below:
> >
> > > --- /dev/null
> > > +++ b/drivers/misc/surface_aggregator/bus.c
> > > @@ -0,0 +1,419 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +
> >
> > No copyright?
>
> As with the other files, I forgot to add that.
>
> [...]
>
> > > +int ssam_device_add(struct ssam_device *sdev)
> > > +{
> > > + int status;
> > > +
> > > + /*
> > > + * Ensure that we can only add new devices to a controller if it has
> > > + * been started and is not going away soon. This works in combination
> > > + * with ssam_controller_remove_clients to ensure driver presence for the
> > > + * controller device, i.e. it ensures that the controller (sdev->ctrl)
> > > + * is always valid and can be used for requests as long as the client
> > > + * device we add here is registered as child under it. This essentially
> > > + * guarantees that the client driver can always expect the preconditions
> > > + * for functions like ssam_request_sync (controller has to be started
> > > + * and is not suspended) to hold and thus does not have to check for
> > > + * them.
> > > + *
> > > + * Note that for this to work, the controller has to be a parent device.
> > > + * If it is not a direct parent, care has to be taken that the device is
> > > + * removed via ssam_device_remove(), as device_unregister does not
> > > + * remove child devices recursively.
> > > + */
> > > + ssam_controller_statelock(sdev->ctrl);
> > > +
> > > + if (READ_ONCE(sdev->ctrl->state) != SSAM_CONTROLLER_STARTED) {
> >
> > You locked the state, why the READ_ONCE()? Is taht needed?
>
> At this point, no. I have, at some point, decided that, since I do
> access the state outside of that lock at some point (specifically when
> submitting the request in ssam_request_sync_submit() to detect mis-use
> of the AP), that I'm going to mark them all as READ_ONCE. Mostly
> because, due to that one check, I have to set the state via WRITE_ONCE.
> Note that that check accessing it outside of the lock is a very basic
> validity check and actually doesn't guarantee _anything_. Again, it's
> just there to try and spot bad API usage. Every actually valid access to
> the state should be locked, so the rest doesn't need the READ_ONCE. I
> can remove those if you want me to.
I would remove the ones you don't really need, but as you are doing this
also to show intent, that should be fine.
> > > + ssam_controller_stateunlock(sdev->ctrl);
> > > + return -ENXIO;
> >
> > odd error value, why this one?
>
> I generally use -ENXIO to indicate that the controller device is not
> present, has not been initialized yet, or is being/has been shut down.
> The error here will be caused by the controller going away (or having
> been suspended) after the device has been created and befor the device
> is added. I guess in case of shutdown, -ESHUTDOWN may be better, but
> then I'm not sure what to return when the controller is suspended.
Do you really need different error values?
Anyway, it's fine, that just seemed like an odd error for that case, but
any error is ok.
> > > +/**
> > > + * struct ssam_device_uid - Unique identifier for SSAM device.
> > > + * @domain: Domain of the device.
> > > + * @category: Target category of the device.
> > > + * @target: Target ID of the device.
> > > + * @instance: Instance ID of the device.
> > > + * @function: Sub-function of the device. This field can be used to
> > > split a
> > > + *single SAM device into multiple virtual subdevices to
> > > separate
> > > + *different functionality of that device and allow one
> > > driver per
> > > + *such functionality.
> > > + */
> > > +struct ssam_device_uid {
> > > + u8 domain;
> > > + u8 category;
> > > + u8 target;
> > > + u8 instance;
> > > + u8 function;
> > > +};
> > > +
> > > +/*
> > > + * Special values for device matching.
> > > + */
> > > +#define SSAM_ANY_TID 0x
> > > +#define SSAM_ANY_IID 0x
> > > +#define SSAM_ANY_FUN 0x
> >
> > These are 16 bits, but the uid values above are 8 bits. How does that
> > match up?
>
> Those values are only intended for use with the SSAM_DEVICE() macro,
> where they are used to set the match flags. They're u16 so that they
> don't interfere with any potentially valid ID value (0x00 to 0xff). The
> lowest byte is specifically 0xff to make it easier to spot potential
> mis-use in the struct above, as that's an ID that, as far as I know,
> doesn't have any valid use (at least yet). They should never be used
> directly with the struct above, something I should probably clarify in
> the documentation.
Re: [PATCH 2/6] x86: Enable seccomp architecture tracking
On Thu, Sep 24, 2020 at 02:45:45AM +0200, Jann Horn wrote: > On Thu, Sep 24, 2020 at 1:29 AM Kees Cook wrote: > > Provide seccomp internals with the details to calculate which syscall > > table the running kernel is expecting to deal with. This allows for > > efficient architecture pinning and paves the way for constant-action > > bitmaps. > [...] > > diff --git a/arch/x86/include/asm/seccomp.h b/arch/x86/include/asm/seccomp.h > [...] > > +#ifdef CONFIG_X86_64 > [...] > > +#else /* !CONFIG_X86_64 */ > > +# define SECCOMP_ARCH AUDIT_ARCH_I386 > > +#endif > > If we are on a 32-bit kernel, performing architecture number checks in > the kernel is completely pointless, because we know that there is only > a single architecture identifier under which syscalls can happen. > > While this patch is useful for enabling the bitmap logic in the > following patches, I think it adds unnecessary overhead in the context > of the previous patch. That's what the RFC was trying to do (avoid the logic if there is only a single arch known to the kernel). I will rework this a bit harder. :) -- Kees Cook
Re: [PATCH v13 2/2] Add PWM fan controller driver for LGM SoC
On Thu, Sep 24, 2020 at 08:55:34AM +0200, Uwe Kleine-König wrote: > Hello, > > (hhm Thierry already announced to have taken this patch, so my review is > late.) There's also a build warning in linux-next caused by this patch, so I'm going to back it out. Rahul, please address Uwe's comments and make sure to fix the build warning as well. Thierry signature.asc Description: PGP signature
Re: [PATCH net] Revert "ravb: Fixed to be able to unload modules"
Hi David,
On Thu, Sep 24, 2020 at 2:40 AM David Miller wrote:
> From: Geert Uytterhoeven
> Date: Tue, 22 Sep 2020 09:29:31 +0200
>
> > This reverts commit 1838d6c62f57836639bd3d83e7855e0ee4f6defc.
> >
> > This commit moved the ravb_mdio_init() call (and thus the
> > of_mdiobus_register() call) from the ravb_probe() to the ravb_open()
> > call. This causes a regression during system resume (s2idle/s2ram), as
> > new PHY devices cannot be bound while suspended.
> ...
> >
> > Signed-off-by: Geert Uytterhoeven
> > Cc: sta...@vger.kernel.org
>
> I noticed this too late, but please don't CC: stable on networking
> patches. We have our own workflow as per the netdev FAQ.
OK, will try to remember.
I wanted to give a heads-up to stable that they've backported early a
patch which turned out to have issues.
> I've applied this but the inability to remove a module is an
> extremely serious bug and should be fixed properly.
Sure. As you stated in
https://lore.kernel.org/linux-renesas-soc/20200820.165244.540878641387937530.da...@davemloft.net/
that will need some rework in the MDIO subsystem...
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND PATCH 1/3] spi: spi-zynqmp-gqspi: Fix kernel-doc warnings
Hi, On 23. 09. 20 20:34, Mark Brown wrote: > On Tue, Sep 22, 2020 at 10:40:14AM -0600, Amit Kumar Mahapatra > wrote: >> Fix kernel-doc warnings in ZynqMP qspi driver file. > > This *still* doesn't apply against current code: > > $ git am -3 --signoff > ~/queue/spi/next/20200922_amit_kumar_mahapatra_spi_spi_zynqmp_gqspi_update_driver_to_use_spi_mem_framework.mbx > > Applying: spi: spi-zynqmp-gqspi: Fix kernel-doc warnings > Using index info to reconstruct a base tree... error: patch failed: > drivers/spi/spi-zynqmp-gqspi.c:173 error: > drivers/spi/spi-zynqmp-gqspi.c: patch does not apply error: Did you > hand edit your patch? It does not apply to blobs recorded in its > index. > > Are you positive that you are using my for-5.10 branch and that > your mail server is not mangling the patches you are sending? I > also tried with plain patch and had similar results with almost all > hunks in this series failing to apply. > I have looked at it. The first issue is that there is still footer and and raw patch is broken. Direct patches I got are fine but not that one which are recorded in lore. Anyway I have resend the whole series myself and will work with Amit to fix his setup. Thanks for your paitience, Michal
Re: [RFC PATCH 01/24] vhost-vdpa: fix backend feature ioctls
On Thu, Sep 24, 2020 at 11:21:02AM +0800, Jason Wang wrote:
> Commit 653055b9acd4 ("vhost-vdpa: support get/set backend features")
> introduces two malfunction backend features ioctls:
>
> 1) the ioctls was blindly added to vring ioctl instead of vdpa device
>ioctl
> 2) vhost_set_backend_features() was called when dev mutex has already
>been held which will lead a deadlock
>
I assume this patch requires some patch in qemu as well. Do you have
such patch?
> This patch fixes the above issues.
>
> Cc: Eli Cohen
> Reported-by: Zhu Lingshan
> Fixes: 653055b9acd4 ("vhost-vdpa: support get/set backend features")
> Signed-off-by: Jason Wang
> ---
> drivers/vhost/vdpa.c | 30 --
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 3fab94f88894..796fe979f997 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -353,8 +353,6 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
> unsigned int cmd,
> struct vdpa_callback cb;
> struct vhost_virtqueue *vq;
> struct vhost_vring_state s;
> - u64 __user *featurep = argp;
> - u64 features;
> u32 idx;
> long r;
>
> @@ -381,18 +379,6 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
> unsigned int cmd,
>
> vq->last_avail_idx = vq_state.avail_index;
> break;
> - case VHOST_GET_BACKEND_FEATURES:
> - features = VHOST_VDPA_BACKEND_FEATURES;
> - if (copy_to_user(featurep, &features, sizeof(features)))
> - return -EFAULT;
> - return 0;
> - case VHOST_SET_BACKEND_FEATURES:
> - if (copy_from_user(&features, featurep, sizeof(features)))
> - return -EFAULT;
> - if (features & ~VHOST_VDPA_BACKEND_FEATURES)
> - return -EOPNOTSUPP;
> - vhost_set_backend_features(&v->vdev, features);
> - return 0;
> }
>
> r = vhost_vring_ioctl(&v->vdev, cmd, argp);
> @@ -440,8 +426,20 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> struct vhost_vdpa *v = filep->private_data;
> struct vhost_dev *d = &v->vdev;
> void __user *argp = (void __user *)arg;
> + u64 __user *featurep = argp;
> + u64 features;
> long r;
>
> + if (cmd == VHOST_SET_BACKEND_FEATURES) {
> + r = copy_from_user(&features, featurep, sizeof(features));
> + if (r)
> + return r;
> + if (features & ~VHOST_VDPA_BACKEND_FEATURES)
> + return -EOPNOTSUPP;
> + vhost_set_backend_features(&v->vdev, features);
> + return 0;
> + }
> +
> mutex_lock(&d->mutex);
>
> switch (cmd) {
> @@ -476,6 +474,10 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> case VHOST_VDPA_SET_CONFIG_CALL:
> r = vhost_vdpa_set_config_call(v, argp);
> break;
> + case VHOST_GET_BACKEND_FEATURES:
> + features = VHOST_VDPA_BACKEND_FEATURES;
> + r = copy_to_user(featurep, &features, sizeof(features));
> + break;
> default:
> r = vhost_dev_ioctl(&v->vdev, cmd, argp);
> if (r == -ENOIOCTLCMD)
> --
> 2.20.1
>
Re: [PATCH 2/3] dm: add support for passing through inline crypto support
On Wed, Sep 23, 2020 at 09:14:39PM -0400, Mike Snitzer wrote: > On Mon, Sep 21 2020 at 8:32pm -0400, > Eric Biggers wrote: > > > On Wed, Sep 09, 2020 at 11:44:21PM +, Satya Tangirala wrote: > > > From: Eric Biggers > > > > > > Update the device-mapper core to support exposing the inline crypto > > > support of the underlying device(s) through the device-mapper device. > > > > > > This works by creating a "passthrough keyslot manager" for the dm > > > device, which declares support for encryption settings which all > > > underlying devices support. When a supported setting is used, the bio > > > cloning code handles cloning the crypto context to the bios for all the > > > underlying devices. When an unsupported setting is used, the blk-crypto > > > fallback is used as usual. > > > > > > Crypto support on each underlying device is ignored unless the > > > corresponding dm target opts into exposing it. This is needed because > > > for inline crypto to semantically operate on the original bio, the data > > > must not be transformed by the dm target. Thus, targets like dm-linear > > > can expose crypto support of the underlying device, but targets like > > > dm-crypt can't. (dm-crypt could use inline crypto itself, though.) > > > > > > When a key is evicted from the dm device, it is evicted from all > > > underlying devices. > > > > > > Signed-off-by: Eric Biggers > > > Co-developed-by: Satya Tangirala > > > Signed-off-by: Satya Tangirala > > > > Looks good as far as Satya's changes from my original patch are concerned. > > > > Can the device-mapper maintainers take a look at this? > > In general it looks like these changes were implemented very carefully > and are reasonable if we _really_ want to enable passing through inline > crypto. > > I do have concerns about the inability to handle changes at runtime (due > to a table reload that introduces new devices without the encryption > settings the existing devices in the table are using). But the fallback > mechanism saves it from being a complete non-starter. Unfortunately, the fallback doesn't completely handle that situation right now. The DM device could be suspended while an upper layer like fscrypt is doing something like "checking if encryption algorithm 'A' is supported by the DM device". It's possible that fscrypt thinks the DM device supports 'A' even though the DM device is suspended, and the table is about to be reloaded to introduce a new device that doesn't support 'A'. Before the DM device is resumed with the new table, fscrypt might send a bio that uses encryption algorithm 'A' without initializing the blk-crypto-fallback ciphers for 'A', because it believes that the DM device supports 'A'. When the bio gets processed by the DM (or when blk-crypto does its checks to decide whether to use the fallback on that bio), the bio will fail because the fallback ciphers aren't initialized. Off the top of my head, one thing we could do is to always allocate the fallback ciphers when the device mapper is the target device for the bio (by maybe adding a "encryption_capabilities_may_change_at_runtime" flag to struct blk_keyslot_manager that the DM will set to true, and that the block layer will check for and decide to appropriately allocate the fallback ciphers), although this does waste memory on systems where we know the DM device tables will never change This patch also doesn't handle the case when the encryption capabilities of the new table are a superset of the old capabilities. Currently, a DM device's capabilities can only shrink after the device is initially created. They can never "expand" to make use of capabilities that might be added due to introduction of new devices via table reloads. I might be forgetting something I thought of before, but looking at it again now, I don't immediately see anything wrong with expanding the advertised capabilities on table reloadI'll look carefully into that again. > > Can you help me better understand the expected consumer of this code? > If you have something _real_ please be explicit. It makes justifying > supporting niche code like this more tolerable. So the motivation for this code was that Android currently uses a device mapper target on top of a phone's disk for user data. On many phones, that disk has inline encryption support, and it'd be great to be able to make use of that. The DM device configuration isn't changed at runtime. > > Thanks, > Mike >
Re: [PATCH v2 1/2] dt-bindings: usb: dwc3-xilinx: Add documentation for Versal DWC3 Controller
Rob Herring writes: > On Thu, Sep 10, 2020 at 12:33:04AM +0530, Manish Narani wrote: >> Add documentation for Versal DWC3 controller. Add required property >> 'reg' for the same. Also add optional properties for snps,dwc3. >> >> Signed-off-by: Manish Narani >> --- >> .../devicetree/bindings/usb/dwc3-xilinx.txt | 20 +-- >> 1 file changed, 18 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> index 4aae5b2cef56..219b5780dbee 100644 >> --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> @@ -1,7 +1,8 @@ >> Xilinx SuperSpeed DWC3 USB SoC controller >> >> Required properties: >> -- compatible: Should contain "xlnx,zynqmp-dwc3" >> +- compatible: May contain "xlnx,zynqmp-dwc3" or "xlnx,versal-dwc3" >> +- reg: Base address and length of the register control block >> - clocks: A list of phandles for the clocks listed in clock-names >> - clock-names: Should contain the following: >>"bus_clk" Master/Core clock, have to be >= 125 MHz for SS >> @@ -13,12 +14,24 @@ Required child node: >> A child node must exist to represent the core DWC3 IP block. The name of >> the node is not important. The content of the node is defined in dwc3.txt. >> >> +Optional properties for snps,dwc3: >> +- dma-coherent: Enable this flag if CCI is enabled in design. Adding >> this >> +flag configures Global SoC bus Configuration Register and >> +Xilinx USB 3.0 IP - USB coherency register to enable CCI. >> +- snps,enable-hibernation: Add this flag to enable hibernation support for >> +peripheral mode. > > This belongs in the DWC3 binding. It also implies that hibernation is > not supported by any other DWC3 based platform. Can't this be implied by > the compatible string (in the parent)? hibernation support is detectable in runtime, and we've been using that. -- balbi signature.asc Description: PGP signature
[PATCH v6 1/6] genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER
To avoid compilation error if an irqchip driver references the function
set_handle_irq() but may not select GENERIC_IRQ_MULTI_HANDLER on some
systems.
For example, the Synopsys DesignWare APB interrupt controller
(dw_apb_ictl) is used as the secondary interrupt controller on arc, csky,
arm64, and most arm32 SoCs, and it's also used as the primary interrupt
controller on Hisilicon SD5203 (an arm32 SoC). The latter need to use
set_handle_irq() to register the top-level IRQ handler, but this multi
irq handler registration mechanism is not implemented on arc system.
The input parameter "handle_irq" maybe defined as static and only
set_handle_irq() references it. This will trigger "defined but not used"
warning. So add "(void)handle_irq" to suppress it.
Signed-off-by: Zhen Lei
---
include/linux/irq.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 1b7f4dfee35b397..b167baef88c0b43 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1252,6 +1252,12 @@ void irq_matrix_free(struct irq_matrix *m, unsigned int
cpu,
* top-level IRQ handler.
*/
extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+#else
+#define set_handle_irq(handle_irq) \
+ do {\
+ (void)handle_irq; \
+ WARN_ON(1); \
+ } while (0)
#endif
#endif /* _LINUX_IRQ_H */
--
1.8.3
Re: [PATCH RESEND] sched/fair: Fix wrong cpu selecting from isolated domain
On Thu, 24 Sep 2020 at 08:48, Xunlei Pang wrote:
>
> We've met problems that occasionally tasks with full cpumask
> (e.g. by putting it into a cpuset or setting to full affinity)
> were migrated to our isolated cpus in production environment.
>
> After some analysis, we found that it is due to the current
> select_idle_smt() not considering the sched_domain mask.
>
> Steps to reproduce on my 31-CPU hyperthreads machine:
> 1. with boot parameter: "isolcpus=domain,2-31"
>(thread lists: 0,16 and 1,17)
> 2. cgcreate -g cpu:test; cgexec -g cpu:test "test_threads"
> 3. some threads will be migrated to the isolated cpu16~17.
>
> Fix it by checking the valid domain mask in select_idle_smt().
>
> Fixes: 10e2f1acd010 ("sched/core: Rewrite and improve select_idle_siblings())
> Reported-by: Wetp Zhang
> Reviewed-by: Jiang Biao
> Signed-off-by: Xunlei Pang
Reviewed-by: Vincent Guittot
> ---
> kernel/sched/fair.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 1a68a05..fa942c4 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6075,7 +6075,7 @@ static int select_idle_core(struct task_struct *p,
> struct sched_domain *sd, int
> /*
> * Scan the local SMT mask for idle CPUs.
> */
> -static int select_idle_smt(struct task_struct *p, int target)
> +static int select_idle_smt(struct task_struct *p, struct sched_domain *sd,
> int target)
> {
> int cpu;
>
> @@ -6083,7 +6083,8 @@ static int select_idle_smt(struct task_struct *p, int
> target)
> return -1;
>
> for_each_cpu(cpu, cpu_smt_mask(target)) {
> - if (!cpumask_test_cpu(cpu, p->cpus_ptr))
> + if (!cpumask_test_cpu(cpu, p->cpus_ptr) ||
> + !cpumask_test_cpu(cpu, sched_domain_span(sd)))
> continue;
> if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
> return cpu;
> @@ -6099,7 +6100,7 @@ static inline int select_idle_core(struct task_struct
> *p, struct sched_domain *s
> return -1;
> }
>
> -static inline int select_idle_smt(struct task_struct *p, int target)
> +static inline int select_idle_smt(struct task_struct *p, struct sched_domain
> *sd, int target)
> {
> return -1;
> }
> @@ -6274,7 +6275,7 @@ static int select_idle_sibling(struct task_struct *p,
> int prev, int target)
> if ((unsigned)i < nr_cpumask_bits)
> return i;
>
> - i = select_idle_smt(p, target);
> + i = select_idle_smt(p, sd, target);
> if ((unsigned)i < nr_cpumask_bits)
> return i;
>
> --
> 1.8.3.1
>
[PATCH v6 0/6] irqchip: dw-apb-ictl: support hierarchy irq domain
v5 --> v6: 1. add Reviewed-by: Rob Herring for Patch 4. 2. Some modifications are made to Patch 5: 1) add " |" for each "description:" property if its content exceeds one line, to tell the yaml keep the "newline" character. 2) add "..." to mark the end of the yaml file. 3) Change the name list of maintainers to the author of "snps,dw-apb-ictl.txt" maintainers: - - Marc Zyngier + - Sebastian Hesselbarth 4) add "maxItems: 1" for property "reg". 5) for property "interrupts": interrupts: -minItems: 1 -maxItems: 65 +maxItems: 1 6) move below descriptions under the top level property "description:" description: | Synopsys DesignWare provides interrupt controller IP for APB known as dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt controller in some SoCs, e.g. Hisilicon SD5203. + The interrupt sources map to the corresponding bits in the interrupt + registers, i.e. + - 0 maps to bit 0 of low interrupts, + - 1 maps to bit 1 of low interrupts, + - 32 maps to bit 0 of high interrupts, + - 33 maps to bit 1 of high interrupts, + - (optional) fast interrupts start at 64. + For more details of 2-6), please refer https://lkml.org/lkml/2020/9/24/13 v4 --> v5: 1. Add WARN_ON(1) in set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER 2. Convert "snps,dw-apb-ictl.txt" to "snps,dw-apb-ictl.yaml" 3. Fix the errors detected by "snps,dw-apb-ictl.yaml" on arch/arc v3 --> v4: 1. remove "gc->chip_types[0].chip.irq_eoi = irq_gc_noop;", the "chip.irq_eoi" hook is not needed by handle_level_irq(). Thanks for Marc Zyngier's review. 2. Add a new patch: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER to avoid compilation error on arch/arc system. v2 --> v3: 1. change (1 << hwirq) to BIT(hwirq). 2. change __exception_irq_entry to __irq_entry, so we can "#include " instead of "#include ". Ohterwise, an compilation error will be reported on arch/csky. drivers/irqchip/irq-dw-apb-ictl.c:20:10: fatal error: asm/exception.h: No such file or directory 3. use "if (!parent || (np == parent))" to determine whether it is primary interrupt controller. 4. make the primary interrupt controller case also use function handle_level_irq(), I used handle_fasteoi_irq() as flow_handler before. 5. Other minor changes are not detailed. v1 --> v2: According to Marc Zyngier's suggestion, discard adding an independent SD5203-VIC driver, but make the dw-apb-ictl irqchip driver to support hierarchy irq domain. It was originally available only for secondary interrupt controller, now it can also be used as primary interrupt controller. The related dt-bindings is updated appropriately. Add "Suggested-by: Marc Zyngier ". Add "Tested-by: Haoyu Lv ". v1: The interrupt controller of SD5203 SoC is VIC(vector interrupt controller), it's based on Synopsys DesignWare APB interrupt controller (dw_apb_ictl) IP, but it can not directly use dw_apb_ictl driver. The main reason is that VIC is used as primary interrupt controller and dw_apb_ictl driver worked for secondary interrupt controller. So add a new driver: "hisilicon,sd5203-vic". Zhen Lei (6): genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER irqchip: dw-apb-ictl: prepare for support hierarchy irq domain irqchip: dw-apb-ictl: support hierarchy irq domain dt-bindings: dw-apb-ictl: support hierarchy irq domain dt-bindings: dw-apb-ictl: convert to json-schema ARC: [dts] fix the errors detected by dtbs_check .../interrupt-controller/snps,dw-apb-ictl.txt | 31 .../interrupt-controller/snps,dw-apb-ictl.yaml | 74 +++ arch/arc/boot/dts/axc001.dtsi | 2 +- arch/arc/boot/dts/axc003.dtsi | 2 +- arch/arc/boot/dts/axc003_idu.dtsi | 2 +- arch/arc/boot/dts/vdk_axc003.dtsi | 2 +- arch/arc/boot/dts/vdk_axc003_idu.dtsi | 2 +- drivers/irqchip/Kconfig| 2 +- drivers/irqchip/irq-dw-apb-ictl.c | 83 ++ include/linux/irq.h| 6 ++ 10 files changed, 157 insertions(+), 49 deletions(-) delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt create mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml -- 1.8.3
[PATCH v6 6/6] ARC: [dts] fix the errors detected by dtbs_check
xxx/arc/boot/dts/axs101.dt.yaml: dw-apb-ictl@e0012000: $nodename:0: \
'dw-apb-ictl@e0012000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
>From schema: xxx/interrupt-controller/snps,dw-apb-ictl.yaml
The node name of the interrupt controller must start with
"interrupt-controller" instead of "dw-apb-ictl".
Signed-off-by: Zhen Lei
---
arch/arc/boot/dts/axc001.dtsi | 2 +-
arch/arc/boot/dts/axc003.dtsi | 2 +-
arch/arc/boot/dts/axc003_idu.dtsi | 2 +-
arch/arc/boot/dts/vdk_axc003.dtsi | 2 +-
arch/arc/boot/dts/vdk_axc003_idu.dtsi | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arc/boot/dts/axc001.dtsi b/arch/arc/boot/dts/axc001.dtsi
index 79ec27c043c1da7..2a151607b08057c 100644
--- a/arch/arc/boot/dts/axc001.dtsi
+++ b/arch/arc/boot/dts/axc001.dtsi
@@ -91,7 +91,7 @@
* avoid duplicating the MB dtsi file given that IRQ from
* this intc to cpu intc are different for axs101 and axs103
*/
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0x0 0xe0012000 0x0 0x200 >;
diff --git a/arch/arc/boot/dts/axc003.dtsi b/arch/arc/boot/dts/axc003.dtsi
index ac8e1b463a70992..cd1edcf4f95efe6 100644
--- a/arch/arc/boot/dts/axc003.dtsi
+++ b/arch/arc/boot/dts/axc003.dtsi
@@ -129,7 +129,7 @@
* avoid duplicating the MB dtsi file given that IRQ from
* this intc to cpu intc are different for axs101 and axs103
*/
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0x0 0xe0012000 0x0 0x200 >;
diff --git a/arch/arc/boot/dts/axc003_idu.dtsi
b/arch/arc/boot/dts/axc003_idu.dtsi
index 9da21e7fd246f9f..70779386ca7963a 100644
--- a/arch/arc/boot/dts/axc003_idu.dtsi
+++ b/arch/arc/boot/dts/axc003_idu.dtsi
@@ -135,7 +135,7 @@
* avoid duplicating the MB dtsi file given that IRQ from
* this intc to cpu intc are different for axs101 and axs103
*/
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0x0 0xe0012000 0x0 0x200 >;
diff --git a/arch/arc/boot/dts/vdk_axc003.dtsi
b/arch/arc/boot/dts/vdk_axc003.dtsi
index f8be7ba8dad499c..c21d0eb07bf6737 100644
--- a/arch/arc/boot/dts/vdk_axc003.dtsi
+++ b/arch/arc/boot/dts/vdk_axc003.dtsi
@@ -46,7 +46,7 @@
};
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0xe0012000 0x200 >;
diff --git a/arch/arc/boot/dts/vdk_axc003_idu.dtsi
b/arch/arc/boot/dts/vdk_axc003_idu.dtsi
index 0afa3e53a4e3932..4d348853ac7c5dc 100644
--- a/arch/arc/boot/dts/vdk_axc003_idu.dtsi
+++ b/arch/arc/boot/dts/vdk_axc003_idu.dtsi
@@ -54,7 +54,7 @@
};
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0xe0012000 0x200 >;
--
1.8.3
[PATCH v6 4/6] dt-bindings: dw-apb-ictl: support hierarchy irq domain
Add support to use dw-apb-ictl as primary interrupt controller.
Signed-off-by: Zhen Lei
Reviewed-by: Rob Herring
---
.../bindings/interrupt-controller/snps,dw-apb-ictl.txt | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
index 086ff08322db94f..2db59df9408f4c6 100644
---
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
+++
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
@@ -2,7 +2,8 @@ Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
Synopsys DesignWare provides interrupt controller IP for APB known as
dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
-APB bus, e.g. Marvell Armada 1500.
+APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt
+controller in some SoCs, e.g. Hisilicon SD5203.
Required properties:
- compatible: shall be "snps,dw-apb-ictl"
@@ -10,6 +11,8 @@ Required properties:
region starting with ENABLE_LOW register
- interrupt-controller: identifies the node as an interrupt controller
- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be
1
+
+Additional required property when it's used as secondary interrupt controller:
- interrupts: interrupt reference to primary interrupt controller
The interrupt sources map to the corresponding bits in the interrupt
@@ -21,6 +24,7 @@ registers, i.e.
- (optional) fast interrupts start at 64.
Example:
+ /* dw_apb_ictl is used as secondary interrupt controller */
aic: interrupt-controller@3000 {
compatible = "snps,dw-apb-ictl";
reg = <0x3000 0xc00>;
@@ -29,3 +33,11 @@ Example:
interrupt-parent = <&gic>;
interrupts = ;
};
+
+ /* dw_apb_ictl is used as primary interrupt controller */
+ vic: interrupt-controller@1013 {
+ compatible = "snps,dw-apb-ictl";
+ reg = <0x1013 0x1000>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
--
1.8.3
[PATCH v6 2/6] irqchip: dw-apb-ictl: prepare for support hierarchy irq domain
Rename some functions and variables in advance, to make the next patch
looks more clear. The details are as follows:
1. rename dw_apb_ictl_handler() to dw_apb_ictl_handle_irq_cascaded().
2. change (1 << hwirq) to BIT(hwirq).
In function dw_apb_ictl_init():
1. rename local variable irq to parent_irq.
2. add "const struct irq_domain_ops *domain_ops = &irq_generic_chip_ops",
then replace &irq_generic_chip_ops in other places with domain_ops.
No functional change.
Signed-off-by: Zhen Lei
Tested-by: Haoyu Lv
---
drivers/irqchip/irq-dw-apb-ictl.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c
b/drivers/irqchip/irq-dw-apb-ictl.c
index e4550e9c810ba94..5458004242e9d20 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -26,7 +26,7 @@
#define APB_INT_FINALSTATUS_H 0x34
#define APB_INT_BASE_OFFSET0x04
-static void dw_apb_ictl_handler(struct irq_desc *desc)
+static void dw_apb_ictl_handle_irq_cascaded(struct irq_desc *desc)
{
struct irq_domain *d = irq_desc_get_handler_data(desc);
struct irq_chip *chip = irq_desc_get_chip(desc);
@@ -43,7 +43,7 @@ static void dw_apb_ictl_handler(struct irq_desc *desc)
u32 virq = irq_find_mapping(d, gc->irq_base + hwirq);
generic_handle_irq(virq);
- stat &= ~(1 << hwirq);
+ stat &= ~BIT(hwirq);
}
}
@@ -73,12 +73,13 @@ static int __init dw_apb_ictl_init(struct device_node *np,
struct irq_domain *domain;
struct irq_chip_generic *gc;
void __iomem *iobase;
- int ret, nrirqs, irq, i;
+ int ret, nrirqs, parent_irq, i;
u32 reg;
+ const struct irq_domain_ops *domain_ops = &irq_generic_chip_ops;
/* Map the parent interrupt for the chained handler */
- irq = irq_of_parse_and_map(np, 0);
- if (irq <= 0) {
+ parent_irq = irq_of_parse_and_map(np, 0);
+ if (parent_irq <= 0) {
pr_err("%pOF: unable to parse irq\n", np);
return -EINVAL;
}
@@ -120,8 +121,7 @@ static int __init dw_apb_ictl_init(struct device_node *np,
else
nrirqs = fls(readl_relaxed(iobase + APB_INT_ENABLE_L));
- domain = irq_domain_add_linear(np, nrirqs,
- &irq_generic_chip_ops, NULL);
+ domain = irq_domain_add_linear(np, nrirqs, domain_ops, NULL);
if (!domain) {
pr_err("%pOF: unable to add irq domain\n", np);
ret = -ENOMEM;
@@ -146,7 +146,8 @@ static int __init dw_apb_ictl_init(struct device_node *np,
gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
}
- irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain);
+ irq_set_chained_handler_and_data(parent_irq,
+ dw_apb_ictl_handle_irq_cascaded, domain);
return 0;
--
1.8.3
[PATCH v6 3/6] irqchip: dw-apb-ictl: support hierarchy irq domain
Add support to use dw-apb-ictl as primary interrupt controller.
Suggested-by: Marc Zyngier
Signed-off-by: Zhen Lei
Tested-by: Haoyu Lv
---
drivers/irqchip/Kconfig | 2 +-
drivers/irqchip/irq-dw-apb-ictl.c | 74 ++-
2 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index bfc9719dbcdc31c..7c2d1c8fa551a66 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -148,7 +148,7 @@ config DAVINCI_CP_INTC
config DW_APB_ICTL
bool
select GENERIC_IRQ_CHIP
- select IRQ_DOMAIN
+ select IRQ_DOMAIN_HIERARCHY
config FARADAY_FTINTC010
bool
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c
b/drivers/irqchip/irq-dw-apb-ictl.c
index 5458004242e9d20..418183b9983dfad 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#define APB_INT_ENABLE_L 0x00
#define APB_INT_ENABLE_H 0x04
@@ -26,6 +27,27 @@
#define APB_INT_FINALSTATUS_H 0x34
#define APB_INT_BASE_OFFSET0x04
+/* irq domain of the primary interrupt controller. */
+static struct irq_domain *dw_apb_ictl_irq_domain;
+
+static void __irq_entry dw_apb_ictl_handle_irq(struct pt_regs *regs)
+{
+ struct irq_domain *d = dw_apb_ictl_irq_domain;
+ int n;
+
+ for (n = 0; n < d->revmap_size; n += 32) {
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, n);
+ u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
+
+ while (stat) {
+ u32 hwirq = ffs(stat) - 1;
+
+ handle_domain_irq(d, hwirq, regs);
+ stat &= ~BIT(hwirq);
+ }
+ }
+}
+
static void dw_apb_ictl_handle_irq_cascaded(struct irq_desc *desc)
{
struct irq_domain *d = irq_desc_get_handler_data(desc);
@@ -50,6 +72,30 @@ static void dw_apb_ictl_handle_irq_cascaded(struct irq_desc
*desc)
chained_irq_exit(chip, desc);
}
+static int dw_apb_ictl_irq_domain_alloc(struct irq_domain *domain, unsigned
int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ int i, ret;
+ irq_hw_number_t hwirq;
+ unsigned int type = IRQ_TYPE_NONE;
+ struct irq_fwspec *fwspec = arg;
+
+ ret = irq_domain_translate_onecell(domain, fwspec, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < nr_irqs; i++)
+ irq_map_generic_chip(domain, virq + i, hwirq + i);
+
+ return 0;
+}
+
+static const struct irq_domain_ops dw_apb_ictl_irq_domain_ops = {
+ .translate = irq_domain_translate_onecell,
+ .alloc = dw_apb_ictl_irq_domain_alloc,
+ .free = irq_domain_free_irqs_top,
+};
+
#ifdef CONFIG_PM
static void dw_apb_ictl_resume(struct irq_data *d)
{
@@ -75,13 +121,20 @@ static int __init dw_apb_ictl_init(struct device_node *np,
void __iomem *iobase;
int ret, nrirqs, parent_irq, i;
u32 reg;
- const struct irq_domain_ops *domain_ops = &irq_generic_chip_ops;
-
- /* Map the parent interrupt for the chained handler */
- parent_irq = irq_of_parse_and_map(np, 0);
- if (parent_irq <= 0) {
- pr_err("%pOF: unable to parse irq\n", np);
- return -EINVAL;
+ const struct irq_domain_ops *domain_ops;
+
+ if (!parent || (np == parent)) {
+ /* It's used as the primary interrupt controller */
+ parent_irq = 0;
+ domain_ops = &dw_apb_ictl_irq_domain_ops;
+ } else {
+ /* Map the parent interrupt for the chained handler */
+ parent_irq = irq_of_parse_and_map(np, 0);
+ if (parent_irq <= 0) {
+ pr_err("%pOF: unable to parse irq\n", np);
+ return -EINVAL;
+ }
+ domain_ops = &irq_generic_chip_ops;
}
ret = of_address_to_resource(np, 0, &r);
@@ -146,8 +199,13 @@ static int __init dw_apb_ictl_init(struct device_node *np,
gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
}
- irq_set_chained_handler_and_data(parent_irq,
+ if (parent_irq) {
+ irq_set_chained_handler_and_data(parent_irq,
dw_apb_ictl_handle_irq_cascaded, domain);
+ } else {
+ dw_apb_ictl_irq_domain = domain;
+ set_handle_irq(dw_apb_ictl_handle_irq);
+ }
return 0;
--
1.8.3
[PATCH v6 5/6] dt-bindings: dw-apb-ictl: convert to json-schema
Convert the Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
binding to DT schema format using json-schema.
Signed-off-by: Zhen Lei
---
.../interrupt-controller/snps,dw-apb-ictl.txt | 43 -
.../interrupt-controller/snps,dw-apb-ictl.yaml | 74 ++
2 files changed, 74 insertions(+), 43 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
create mode 100644
Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
deleted file mode 100644
index 2db59df9408f4c6..000
---
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
-
-Synopsys DesignWare provides interrupt controller IP for APB known as
-dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
-APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt
-controller in some SoCs, e.g. Hisilicon SD5203.
-
-Required properties:
-- compatible: shall be "snps,dw-apb-ictl"
-- reg: physical base address of the controller and length of memory mapped
- region starting with ENABLE_LOW register
-- interrupt-controller: identifies the node as an interrupt controller
-- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be
1
-
-Additional required property when it's used as secondary interrupt controller:
-- interrupts: interrupt reference to primary interrupt controller
-
-The interrupt sources map to the corresponding bits in the interrupt
-registers, i.e.
-- 0 maps to bit 0 of low interrupts,
-- 1 maps to bit 1 of low interrupts,
-- 32 maps to bit 0 of high interrupts,
-- 33 maps to bit 1 of high interrupts,
-- (optional) fast interrupts start at 64.
-
-Example:
- /* dw_apb_ictl is used as secondary interrupt controller */
- aic: interrupt-controller@3000 {
- compatible = "snps,dw-apb-ictl";
- reg = <0x3000 0xc00>;
- interrupt-controller;
- #interrupt-cells = <1>;
- interrupt-parent = <&gic>;
- interrupts = ;
- };
-
- /* dw_apb_ictl is used as primary interrupt controller */
- vic: interrupt-controller@1013 {
- compatible = "snps,dw-apb-ictl";
- reg = <0x1013 0x1000>;
- interrupt-controller;
- #interrupt-cells = <1>;
- };
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
new file mode 100644
index 000..1b05d36b5f7b943
--- /dev/null
+++
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/snps,dw-apb-ictl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
+
+maintainers:
+ - Sebastian Hesselbarth
+
+description: |
+ Synopsys DesignWare provides interrupt controller IP for APB known as
+ dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs
+ with APB bus, e.g. Marvell Armada 1500. It can also be used as primary
+ interrupt controller in some SoCs, e.g. Hisilicon SD5203.
+
+ The interrupt sources map to the corresponding bits in the interrupt
+ registers, i.e.
+ - 0 maps to bit 0 of low interrupts,
+ - 1 maps to bit 1 of low interrupts,
+ - 32 maps to bit 0 of high interrupts,
+ - 33 maps to bit 1 of high interrupts,
+ - (optional) fast interrupts start at 64.
+
+allOf:
+ - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+ compatible:
+const: snps,dw-apb-ictl
+
+ interrupt-controller: true
+
+ reg:
+description: |
+ Physical base address of the controller and length of memory mapped
+ region starting with ENABLE_LOW register.
+maxItems: 1
+
+ interrupts:
+description: Interrupt reference to primary interrupt controller.
+maxItems: 1
+
+ "#interrupt-cells":
+description: Number of cells to encode an interrupt-specifier.
+const: 1
+
+required:
+ - compatible
+ - reg
+ - interrupt-controller
+ - '#interrupt-cells'
+
+examples:
+ - |
+/* dw_apb_ictl is used as secondary interrupt controller */
+aic: interrupt-controller@3000 {
+compatible = "snps,dw-apb-ictl";
+reg = <0x3000 0xc00>;
+interrupt-controller;
+#interrupt-cells = <1>;
+interrupt-parent = <&gic>;
+interrupts = <0 3 4>;
+};
+
+/* dw_apb_ictl is used as primary interrupt controller */
+vic: interrupt-contro
Re: [PATCH 1/7] usb: mtu3: convert to devm_platform_ioremap_resource_byname
Chunfeng Yun writes: > Hi Felip, > > > On Mon, 2020-09-07 at 10:42 +0300, Felipe Balbi wrote: >> Hi, >> >> Chunfeng Yun writes: >> > Use devm_platform_ioremap_resource_byname() to simplify code >> > >> > Signed-off-by: Chunfeng Yun >> >> why is it so that your patches always come base64 encoded? They look >> fine on the email client, but when I try to pipe the message to git am >> it always gives me a lot of trouble and I have to manually decode the >> body of your messages and recombine with the patch. >> >> Can you try to send your patches as actual plain text without encoding >> the body with base64? > Missed the email. > > Sorry for inconvenience! > Is only the commit message base64 encoded, or includes the codes? The entire thing :-) -- balbi signature.asc Description: PGP signature
Re: [PATCH v2] ide/macide: Convert Mac IDE driver to platform driver
Hi Finn,
On Thu, Sep 24, 2020 at 3:07 AM Finn Thain wrote:
> On Wed, 16 Sep 2020, Finn Thain wrote:
> > On Tue, 15 Sep 2020, Geert Uytterhoeven wrote:
> > > > > > --- a/drivers/ide/macide.c
> > > > > > +++ b/drivers/ide/macide.c
> > > > >
> > > > > > @@ -109,42 +110,61 @@ static const char *mac_ide_name[] =
> > > > > > * Probe for a Macintosh IDE interface
> > > > > > */
> > > > > >
> > > > > > -static int __init macide_init(void)
> > > > > > +static int mac_ide_probe(struct platform_device *pdev)
> > > > > > {
> > > > >
> > > > > > printk(KERN_INFO "ide: Macintosh %s IDE controller\n",
> > > > > > mac_ide_name[macintosh_config->ide_type -
> > > > > > 1]);
> > > > > >
> > > > > > - macide_setup_ports(&hw, base, irq);
> > > > > > + macide_setup_ports(&hw, mem->start, irq->start);
> > > > > >
> > > > > > - return ide_host_add(&d, hws, 1, NULL);
> > > > > > + rc = ide_host_add(&d, hws, 1, &host);
> > > > > > + if (rc)
> > > > > > + return rc;
> > > > > > +
> > > > > > + platform_set_drvdata(pdev, host);
> > > > >
> > > > > Move one up, to play it safe?
> > > > >
> > > >
> > > > You mean, before calling ide_host_add? The 'host' pointer is
> > > > uninitialized prior to that call.
> > >
> > > Oh right, so the IDE subsystem doesn't let you use the drvdata inside
> > > your driver (besides in remove()) in a safe way :-(
> > >
> >
> > The IDE subsystem does allow other patterns here. I could have changed
> > ide_host_alloc() into ide_host_register() followed by ide_host_add() but
> > I could not see any benefit from that change.
> >
>
> Sorry, I meant to say, "I could have changed ide_host_add() into
> ide_host_alloc() followed by ide_host_register() ..."
>
> > A quick search for "platform_device" shows that the driver does not use
> > any uninitialized driver_data pointer (because ide_ifr is a global). In
> > your message of September 9th you readily reached the same conclusion
> > when you reviewed v1.
> >
> > If mac_ide_probe() followed the usual pattern it might make review
> > easier (as reviewers may not wish to consider the entire driver) but
> > does that really make the code more "safe"?
>
> I still think that "if it ain't broke, don't fix it" is actually the
> "safe" option for macide.c. But I'm happy to make additional changes, test
> them and send v5 if that's preferred.
I'm fine with keeping this as-is, as it doesn't really matter for this
particular
driver.
> Looking further at the drivers using ide_host_register(), I see that
> falconide.c is missing a set_drvdata() call, while tx4939ide.c calls
> set_drvdata() after ide_host_register(). The latter example is not a bug.
>
> The pattern I used, that is, calling set_drvdata() after ide_host_add(),
> is actually more popular among IDE drivers than the pattern you suggested,
> that is, set_drvdata() followed by ide_host_register(). Either way, I
> don't see any bugs besides the one in falconide.c.
>
> Regarding falconide.c, my inclination is to send a fix following the more
> common pattern (among IDE drivers), as below. I guess that may prompt the
> subsystem maintainers to make known their views on the style question.
Please do so. Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] MAINTAINERS: Add entry for Broadcom BDC driver
Greg KH writes: > On Thu, Sep 17, 2020 at 02:49:54PM +0800, Chunfeng Yun wrote: >> On Sun, 2020-09-06 at 12:55 -0700, Florian Fainelli wrote: >> > >> > On 7/9/2020 8:48 PM, Florian Fainelli wrote: >> > > The Broadcom BDC driver did not have a MAINTAINERS entry which made it >> > > escape review from Al and myself, add an entry so the relevant mailing >> > > lists and people are copied. >> > > >> > > Signed-off-by: Florian Fainelli >> > >> > This patch still does not seem to have been picked up (not seeing it in >> > linux-next), can this be applied so we have an accurate maintainer >> > information for this driver? >> Ping > > Felipe should have picked this up. > > If not, please resend it again and I can. Applied -- balbi signature.asc Description: PGP signature
Re: [PATCH v4] ide/macide: Convert Mac IDE driver to platform driver
Hi Finn,
On Thu, Sep 24, 2020 at 1:17 AM Finn Thain wrote:
> Add platform devices for the Mac IDE controller variants. Convert the
> macide module into a platform driver to support two of those variants.
> For the third, use a generic "pata_platform" driver instead.
> This enables automatic loading of the appropriate module and begins
> the process of replacing the driver with libata alternatives.
>
> Cc: Bartlomiej Zolnierkiewicz
> Cc: Geert Uytterhoeven
> Cc: Joshua Thompson
> References: commit 5ed0794cde593 ("m68k/atari: Convert Falcon IDE drivers to
> platform drivers")
> References: commit 7ad19a99ad431 ("ide: officially deprecated the legacy IDE
> driver")
> Tested-by: Stan Johnson
> Signed-off-by: Finn Thain
> ---
> This patch was tested successfully on a Powerbook 190 (MAC_IDE_BABOON)
> using both pata_platform and ide_platform drivers.
> The next step will be to try using these generic drivers with the other
> IDE controller variants (MAC_IDE_QUADRA or MAC_IDE_PB) so that the macide
> driver can be entirely replaced with libata drivers.
>
> Changed since v3:
> - Updated Kconfig help text.
>
> Changed since v2:
> - Enabled CONFIG_BLK_DEV_PLATFORM in multi_defconfig.
> - Replaced dev_get_drvdata() with platform_get_drvdata().
Thanks for the updates!
Reviewed-by: Geert Uytterhoeven
i.e. will queue in the m68k for-v5.10 branch, hopefully with an
Acked-by from the IDE maintainer.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] MAINTAINERS: Add entry for Broadcom BDC driver
Felipe Balbi writes: > Greg KH writes: > >> On Thu, Sep 17, 2020 at 02:49:54PM +0800, Chunfeng Yun wrote: >>> On Sun, 2020-09-06 at 12:55 -0700, Florian Fainelli wrote: >>> > >>> > On 7/9/2020 8:48 PM, Florian Fainelli wrote: >>> > > The Broadcom BDC driver did not have a MAINTAINERS entry which made it >>> > > escape review from Al and myself, add an entry so the relevant mailing >>> > > lists and people are copied. >>> > > >>> > > Signed-off-by: Florian Fainelli >>> > >>> > This patch still does not seem to have been picked up (not seeing it in >>> > linux-next), can this be applied so we have an accurate maintainer >>> > information for this driver? >>> Ping >> >> Felipe should have picked this up. >> >> If not, please resend it again and I can. > > Applied scratch that, it's already in Linus' -- balbi signature.asc Description: PGP signature
RE: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings for nxp,pcf2127
On Thu, Sep 24, 2020 at 15:05AM +, Uwe Kleine-König wrote: > -Original Message- > From: Uwe Kleine-König > Sent: 2020年9月24日 15:05 > To: Qiang Zhao > Cc: Alexandre Belloni ; Wim Van Sebroeck > ; Guenter Roeck ; > linux-watch...@vger.kernel.org; a.zu...@towertech.it; robh...@kernel.org; > linux-...@vger.kernel.org; devicet...@vger.kernel.org; > linux-kernel@vger.kernel.org; ker...@pengutronix.de > Subject: Re: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings for > nxp,pcf2127 > > Hello, > > On Thu, Sep 24, 2020 at 03:20:33AM +, Qiang Zhao wrote: > > On 21/09/2020 13:48:19+0800, Qiang Zhao wrote: > > > > > -Original Message- > > > From: Alexandre Belloni > > > Sent: 2020年9月23日 17:45 > > > To: Qiang Zhao > > > Cc: Wim Van Sebroeck ; Guenter Roeck > > > ; linux-watch...@vger.kernel.org; > > > a.zu...@towertech.it; robh...@kernel.org; linux-...@vger.kernel.org; > > > devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Uwe > > > Kleine-König > > > Subject: Re: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings > > > for > > > nxp,pcf2127 > > > > > > Hi, > > > > > > You forgot to copy the watchdog maintainers, I think such a property > > > should be discussed with them. > > > > > > Note that I'm still convinced this is not a complete solution, see: > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo > > > re.kern > > > > el.org%2Flinux-rtc%2F20200716181816.GF3428%40piout.net%2F&data= > > > > 02%7C01%7Cqiang.zhao%40nxp.com%7Cb71f79a044b0493d6d4f08d85fa551c > > > > b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637364510931174 > > > > 355&sdata=%2BOxrzB8RIuxM9Let5slhfCVmMm6PMNoERDeHC9%2Fdxng > > > %3D&reserved=0 > > haha > > > Yes, you are right, There is not a fundamental solution. > > However it somewhat avoid this situation at least. > > > > And if without this issue, > > is it correct to register a rtc device as watchdog no matter it is used as > watchdog on the board? > > Every time Linux are booted up, watchdog device should be configured to the > right one manually. > > So the patch are useful, even though it is not for the issue. > > > > What should we do to really resolve this issue? > > I still think we need a kernel solution here. I would expect that most > assembled > pcf2127 chips are unable to act as a watchdog (i.e. don't have the RST output > connected to something that resets the machine). > > So my favoured solution would be a positive property like: > > has-watchdog; > > or something similar. In my eyes this is definitely something we want to > specify > in the device tree because it is a relevant hardware property. > I consider it a bug to give a watchdog device to userspace that isn't > functional. > > Best regards > Uwe I strongly agree with you! It should be positive property. However, we couldn't identify which board are using pcf2127 as watchdog, So we are unable to modify the boards' dts to correct (watchdog or not) in this patchset. I noticed that only LS series platforms and imx6 have pcf2127 node, as far as I know, the LS platforms don't use it as watchdog, But I am not sure about imx6 > > -- > Pengutronix e.K. | Uwe Kleine-König > | > Industrial Linux Solutions | https://www.pengutronix.de/ | Best Regards Qiang Zhao
Re: [PATCH v4 11/23] device-dax: Kill dax_kmem_res
On 23.09.20 23:41, Dan Williams wrote:
> On Wed, Sep 23, 2020 at 1:04 AM David Hildenbrand wrote:
>>
>> On 08.09.20 17:33, Joao Martins wrote:
>>> [Sorry for the late response]
>>>
>>> On 8/21/20 11:06 AM, David Hildenbrand wrote:
On 03.08.20 07:03, Dan Williams wrote:
> @@ -37,109 +45,94 @@ int dev_dax_kmem_probe(struct device *dev)
> * could be mixed in a node with faster memory, causing
> * unavoidable performance issues.
> */
> - numa_node = dev_dax->target_node;
> if (numa_node < 0) {
> dev_warn(dev, "rejecting DAX region with invalid node: %d\n",
> numa_node);
> return -EINVAL;
> }
>
> - /* Hotplug starting at the beginning of the next block: */
> - kmem_start = ALIGN(range->start, memory_block_size_bytes());
> -
> - kmem_size = range_len(range);
> - /* Adjust the size down to compensate for moving up kmem_start: */
> - kmem_size -= kmem_start - range->start;
> - /* Align the size down to cover only complete blocks: */
> - kmem_size &= ~(memory_block_size_bytes() - 1);
> - kmem_end = kmem_start + kmem_size;
> -
> - new_res_name = kstrdup(dev_name(dev), GFP_KERNEL);
> - if (!new_res_name)
> + res_name = kstrdup(dev_name(dev), GFP_KERNEL);
> + if (!res_name)
> return -ENOMEM;
>
> - /* Region is permanently reserved if hotremove fails. */
> - new_res = request_mem_region(kmem_start, kmem_size, new_res_name);
> - if (!new_res) {
> - dev_warn(dev, "could not reserve region [%pa-%pa]\n",
> -&kmem_start, &kmem_end);
> - kfree(new_res_name);
> + res = request_mem_region(range.start, range_len(&range), res_name);
I think our range could be empty after aligning. I assume
request_mem_region() would check that, but maybe we could report a
better error/warning in that case.
>>> dax_kmem_range() already returns a memory-block-aligned @range but
>>> IIUC request_mem_region() isn't checking for that. Having said that
>>> the returned @res wouldn't be different from the passed range.start.
>>>
> /*
> * Ensure that future kexec'd kernels will not treat this as RAM
> * automatically.
> */
> - rc = add_memory_driver_managed(numa_node, new_res->start,
> - resource_size(new_res), kmem_name);
> + rc = add_memory_driver_managed(numa_node, res->start,
> + resource_size(res), kmem_name);
> +
> + res->flags |= IORESOURCE_BUSY;
Hm, I don't think that's correct. Any specific reason why to mark the
not-added, unaligned parts BUSY? E.g., walk_system_ram_range() could
suddenly stumble over it - and e.g., similarly kexec code when trying to
find memory for placing kexec images. I think we should leave this
!BUSY, just as it is right now.
>>> Agreed.
>>>
> if (rc) {
> - release_resource(new_res);
> - kfree(new_res);
> - kfree(new_res_name);
> + release_mem_region(range.start, range_len(&range));
> + kfree(res_name);
> return rc;
> }
> - dev_dax->dax_kmem_res = new_res;
> +
> + dev_set_drvdata(dev, res_name);
>
> return 0;
> }
>
> #ifdef CONFIG_MEMORY_HOTREMOVE
> -static int dev_dax_kmem_remove(struct device *dev)
> +static void dax_kmem_release(struct dev_dax *dev_dax)
> {
> - struct dev_dax *dev_dax = to_dev_dax(dev);
> - struct resource *res = dev_dax->dax_kmem_res;
> - resource_size_t kmem_start = res->start;
> - resource_size_t kmem_size = resource_size(res);
> - const char *res_name = res->name;
> int rc;
> + struct device *dev = &dev_dax->dev;
> + const char *res_name = dev_get_drvdata(dev);
> + struct range range = dax_kmem_range(dev_dax);
>
> /*
> * We have one shot for removing memory, if some memory blocks were
> not
> * offline prior to calling this function remove_memory() will fail,
> and
> * there is no way to hotremove this memory until reboot because
> device
> -* unbind will succeed even if we return failure.
> +* unbind will proceed regardless of the remove_memory result.
> */
> - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size);
> - if (rc) {
> - any_hotremove_failed = true;
> - dev_err(dev,
> - "DAX region %pR cannot be hotremoved until the next
> reboot\n",
> - res);
> - return rc;
> + rc = remove_memory(dev_dax->target_node, range.start,
> range_len(&range));
> + if (rc == 0) {
if (!rc) ?
>>> B
Re: [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code
Tang Bin writes: > Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to > simplify code, avoid redundant judgements. > > Signed-off-by: Zhang Shengju > Signed-off-by: Tang Bin Applied for next merge window. Make sure to get this driver out of drivers/usb/phy and moved into drivers/phy ASAP. -- balbi signature.asc Description: PGP signature
Re: [RFC PATCH 01/24] vhost-vdpa: fix backend feature ioctls
On 2020/9/24 下午3:16, Eli Cohen wrote:
On Thu, Sep 24, 2020 at 11:21:02AM +0800, Jason Wang wrote:
Commit 653055b9acd4 ("vhost-vdpa: support get/set backend features")
introduces two malfunction backend features ioctls:
1) the ioctls was blindly added to vring ioctl instead of vdpa device
ioctl
2) vhost_set_backend_features() was called when dev mutex has already
been held which will lead a deadlock
I assume this patch requires some patch in qemu as well. Do you have
such patch?
It's this series: [PATCH 0/3] Vhost-vDPA: batch IOTLB updating.
You were copied.
Thanks
Re: possible deadlock in xfrm_policy_delete
On Thu, Sep 24, 2020 at 06:44:12AM +0200, Dmitry Vyukov wrote:
> On Thu, Sep 24, 2020 at 6:36 AM Herbert Xu
> wrote:
> > > (k-slock-AF_INET6){+.-.}-{2:2}
That's a seqlock.
> > What's going on with all these bogus lockdep reports?
> >
> > These are two completely different locks, one is for TCP and the
> > other is for SCTP. Why is lockdep suddenly beoming confused about
> > this?
> >
> > FWIW this flood of bogus reports started on 16/Sep.
>
>
> FWIW one of the dups of this issue was bisected to:
>
> commit 1909760f5fc3f123e47b4e24e0ccdc0fc8f3f106
> Author: Ahmed S. Darwish
> Date: Fri Sep 4 15:32:31 2020 +
>
> seqlock: PREEMPT_RT: Do not starve seqlock_t writers
>
> Can it be related?
Did that tree you're testing include 267580db047e ("seqlock: Unbreak
lockdep") ?
Re: [PATCH v9 11/20] gpiolib: cdev: support GPIO_V2_LINE_SET_VALUES_IOCTL
On Wed, Sep 23, 2020 at 07:18:08PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 22, 2020 at 5:36 AM Kent Gibson wrote:
> >
> > Add support for the GPIO_V2_LINE_SET_VALUES_IOCTL.
>
> > +static long linereq_set_values_unlocked(struct linereq *lr,
> > + struct gpio_v2_line_values *lv)
> > +{
> > + DECLARE_BITMAP(vals, GPIO_V2_LINES_MAX);
> > + struct gpio_desc **descs;
> > + unsigned int i, didx, num_set;
> > + int ret;
> > +
> > + bitmap_zero(vals, GPIO_V2_LINES_MAX);
> > + for (num_set = 0, i = 0; i < lr->num_lines; i++) {
> > + if (lv->mask & BIT_ULL(i)) {
>
> Similar idea
>
> DECLARE_BITMAP(mask, 64) = BITMAP_FROM_U64(lv->mask);
>
> num_set = bitmap_weight();
>
I had played with this option, but bitmap_weight() counts all
the bits set in the mask - which considers bits >= lr->num_lines.
So you would need to mask lv->mask before converting it to a bitmap.
(I'm ok with ignoring those bits in case userspace wants to be lazy and
use an all 1s mask.)
But since we're looping over the bitmap anyway we may as well just
count as we go.
> for_each_set_bit(i, mask, lr->num_lines)
>
Yeah, that should work. I vaguely recall trying this and finding it
generated larger object code, but I'll give it another try and if it
works out then include it in v10.
Cheers,
Kent.
Re: [PATCH -next] tee: optee: fix type warning of sizeof in pool_op_alloc()
On Thu, Sep 17, 2020 at 9:52 AM Liu Shixin wrote: > > sizeof() when applied to a pointer typed expression should gives the > size of the pointed data, even if the data is a pointer. > > Signed-off-by: Liu Shixin > --- > drivers/tee/optee/shm_pool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/tee/optee/shm_pool.c b/drivers/tee/optee/shm_pool.c > index d767eebf30bd..9fdc667b5df0 100644 > --- a/drivers/tee/optee/shm_pool.c > +++ b/drivers/tee/optee/shm_pool.c > @@ -31,7 +31,7 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm, > unsigned int nr_pages = 1 << order, i; > struct page **pages; > > - pages = kcalloc(nr_pages, sizeof(pages), GFP_KERNEL); > + pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL); In this case we want an array of pointers as you also can see in the type of "pages". Thanks, Jens > if (!pages) > return -ENOMEM; > > -- > 2.25.1 >
Re: [PATCH v2] soc: renesas: rmobile-sysc: Fix some leaks in rmobile_init_pm_domains()
On Wed, Sep 23, 2020 at 1:31 PM Dan Carpenter wrote:
> This code needs to call iounmap() on one error path.
>
> Fixes: 2173fc7cb681 ("ARM: shmobile: R-Mobile: Add DT support for PM domains")
> Signed-off-by: Dan Carpenter
> ---
> v2: The v1 patch potentially led to a use after free.
Reviewed-by: Geert Uytterhoeven
i.e. will queue in renesas-devel for v5.11.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
On Sep 14 2020, Aurelien Jarno wrote: > How should we proceed to get that fixed in time for 5.9? For the older > branches where it has been backported (so far 5.7 and 5.8), should we > just get that commit reverted instead? Can this please be resolved ASAP? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH 3/6] seccomp: Implement constant action bitmaps
On Thu, Sep 24, 2020 at 02:25:03AM +0200, Jann Horn wrote:
> On Thu, Sep 24, 2020 at 1:29 AM Kees Cook wrote:
> > +/* When no bits are set for a syscall, filters are run. */
> > +struct seccomp_bitmaps {
> > +#ifdef SECCOMP_ARCH
> > + /* "allow" are initialized to set and only ever get cleared. */
> > + DECLARE_BITMAP(allow, NR_syscalls);
>
> This bitmap makes sense.
>
> > + /* These are initialized to clear and only ever get set. */
> > + DECLARE_BITMAP(kill_thread, NR_syscalls);
> > + DECLARE_BITMAP(kill_process, NR_syscalls);
>
> I don't think these bitmaps make sense, this is not part of any fastpath.
That's a fair point. I think I arrived at this design because it ended
up making filter addition faster ("don't bother processing this one,
it's already 'kill'"), but it's likely not worse the memory usage
trade-off.
> (However, a "which syscalls have a fixed result" bitmap might make
> sense if we want to export the list of permitted syscalls as a text
> file in procfs, as I mentioned over at
> .)
I haven't found a data structure I'm happy with for this. It seemed like
NR_syscalls * sizeof(u32) was rather a lot (i.e. to store the BPF_RET
value). However, let me discuss that more in the "why in in thread?"
below...
> The "NR_syscalls" part assumes that the compat syscall tables will not
> be bigger than the native syscall table, right? I guess that's usually
> mostly true nowadays, thanks to the syscall table unification...
> (might be worth a comment though)
Hrm, I had convinced myself it was a max() of compat. But I see no
evidence of that now. Which means that I can add these to the per-arch
seccomp defines with something like:
# define SECCOMP_NR_NATIVE NR_syscalls
# define SECCOMP_NR_COMPAT X32_NR_syscalls
...
> > +#endif
> > +};
> > +
> > struct seccomp_filter;
> > /**
> > * struct seccomp - the state of a seccomp'ed process
> > @@ -45,6 +56,13 @@ struct seccomp {
> > #endif
> > atomic_t filter_count;
> > struct seccomp_filter *filter;
> > + struct seccomp_bitmaps native;
> > +#ifdef CONFIG_COMPAT
> > + struct seccomp_bitmaps compat;
> > +#endif
> > +#ifdef SECCOMP_MULTIPLEXED_SYSCALL_TABLE_ARCH
> > + struct seccomp_bitmaps multiplex;
> > +#endif
>
> Why do we have one bitmap per thread (in struct seccomp) instead of
> putting the bitmap for a given filter and all its ancestors into the
> seccomp_filter?
I explicitly didn't want to add code that was run per-filter; I wanted
O(1), not O(n) even if the n work was a small constant. There is
obviously a memory/perf tradeoff here. I wonder if the middle ground
would be to put a bitmap and "constant action" results in the filter
oh duh. The "top" filter is already going to be composed with its
ancestors. That's all that needs to be checked. Then the tri-state can
be:
bitmap accept[NR_syscalls]: accept or check "known" bitmap
bitmap filter[NR_syscalls]: run filter or return known action
u32 known_action[NR_syscalls];
(times syscall numbering "architecture" counts)
Though perhaps it would be just as fast as:
bitmap run_filter[NR_syscalls]: run filter or return known_action
u32 known_action[NR_syscalls];
where accept isn't treated special...
>
> > };
> >
> > #ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index 0a3ff8eb8aea..111a238bc532 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -318,7 +318,7 @@ static inline u8 seccomp_get_arch(u32 syscall_arch, u32
> > syscall_nr)
> >
> > #ifdef SECCOMP_MULTIPLEXED_SYSCALL_TABLE_ARCH
> > if (syscall_arch == SECCOMP_MULTIPLEXED_SYSCALL_TABLE_ARCH) {
> > - seccomp_arch |= (sd->nr &
> > SECCOMP_MULTIPLEXED_SYSCALL_TABLE_MASK) >>
> > + seccomp_arch |= (syscall_nr &
> > SECCOMP_MULTIPLEXED_SYSCALL_TABLE_MASK) >>
> > SECCOMP_MULTIPLEXED_SYSCALL_TABLE_SHIFT;
>
> This belongs over into patch 1.
Thanks! I was rushing to get this posted so YiFei Zhu wouldn't spend
time fighting with arch and Kconfig stuff. :) I'll clean this (and the
other random cruft) up.
--
Kees Cook
Re: [PATCH v2 3/5] dt-bindings: iio: ad9467: add entry for for AD9434 ADC
On Wed, Sep 23, 2020 at 11:42 PM Jonathan Cameron wrote: > > On Tue, 22 Sep 2020 16:25:57 +0300 > Alexandru Ardelean wrote: > > > Add entry for the AD9434 high-speed ADC which is supported by the 'ad9467' > > driver. > > > > Signed-off-by: Alexandru Ardelean > > Probably want to tweak a few other bits of the binding. > It has some specific statements about the ad9467 as this is only > second device to be added. Need to change to those to "ad9467 and similar" > or something like that + add description for this part. Yep, that was sloppy on my part. Will update. > > > --- > > Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml > > b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml > > index c4f57fa6aad1..0e8da8ee6975 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml > > @@ -19,6 +19,7 @@ description: | > > properties: > >compatible: > > enum: > > + - adi,ad9434 > >- adi,ad9467 > > > >reg: >
Re: [PATCH 2/3] dm: add support for passing through inline crypto support
On Wed, Sep 23, 2020 at 09:21:03PM -0400, Mike Snitzer wrote:
> On Wed, Sep 09 2020 at 7:44pm -0400,
> Satya Tangirala wrote:
>
> > From: Eric Biggers
> >
> > Update the device-mapper core to support exposing the inline crypto
> > support of the underlying device(s) through the device-mapper device.
> >
> > This works by creating a "passthrough keyslot manager" for the dm
> > device, which declares support for encryption settings which all
> > underlying devices support. When a supported setting is used, the bio
> > cloning code handles cloning the crypto context to the bios for all the
> > underlying devices. When an unsupported setting is used, the blk-crypto
> > fallback is used as usual.
> >
> > Crypto support on each underlying device is ignored unless the
> > corresponding dm target opts into exposing it. This is needed because
> > for inline crypto to semantically operate on the original bio, the data
> > must not be transformed by the dm target. Thus, targets like dm-linear
> > can expose crypto support of the underlying device, but targets like
> > dm-crypt can't. (dm-crypt could use inline crypto itself, though.)
> >
> > When a key is evicted from the dm device, it is evicted from all
> > underlying devices.
> >
> > Signed-off-by: Eric Biggers
> > Co-developed-by: Satya Tangirala
> > Signed-off-by: Satya Tangirala
> > ---
> > block/blk-crypto.c | 1 +
> > block/keyslot-manager.c | 34
> > drivers/md/dm-core.h| 4 ++
> > drivers/md/dm-table.c | 52 +++
> > drivers/md/dm.c | 92 -
> > include/linux/device-mapper.h | 6 +++
> > include/linux/keyslot-manager.h | 7 +++
> > 7 files changed, 195 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/blk-crypto.c b/block/blk-crypto.c
> > index 2d5e60023b08..33555cf0e3e7 100644
> > --- a/block/blk-crypto.c
> > +++ b/block/blk-crypto.c
> > @@ -402,3 +402,4 @@ int blk_crypto_evict_key(struct request_queue *q,
> > */
> > return blk_crypto_fallback_evict_key(key);
> > }
> > +EXPORT_SYMBOL_GPL(blk_crypto_evict_key);
> > diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
> > index 60ac406d54b9..e0f776c38d8a 100644
> > --- a/block/keyslot-manager.c
> > +++ b/block/keyslot-manager.c
> > @@ -416,6 +416,40 @@ void blk_ksm_unregister(struct request_queue *q)
> > {
> > q->ksm = NULL;
> > }
> > +EXPORT_SYMBOL_GPL(blk_ksm_unregister);
> > +
> > +/**
> > + * blk_ksm_intersect_modes() - restrict supported modes by child device
> > + * @parent: The keyslot manager for parent device
> > + * @child: The keyslot manager for child device, or NULL
> > + *
> > + * Clear any crypto mode support bits in @parent that aren't set in @child.
> > + * If @child is NULL, then all parent bits are cleared.
> > + *
> > + * Only use this when setting up the keyslot manager for a layered device,
> > + * before it's been exposed yet.
> > + */
> > +void blk_ksm_intersect_modes(struct blk_keyslot_manager *parent,
> > +const struct blk_keyslot_manager *child)
> > +{
> > + if (child) {
> > + unsigned int i;
> > +
> > + parent->max_dun_bytes_supported =
> > + min(parent->max_dun_bytes_supported,
> > + child->max_dun_bytes_supported);
> > + for (i = 0; i < ARRAY_SIZE(child->crypto_modes_supported);
> > +i++) {
> > + parent->crypto_modes_supported[i] &=
> > + child->crypto_modes_supported[i];
> > + }
> > + } else {
> > + parent->max_dun_bytes_supported = 0;
> > + memset(parent->crypto_modes_supported, 0,
> > + sizeof(parent->crypto_modes_supported));
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(blk_ksm_intersect_modes);
> >
> > /**
> > * blk_ksm_init_passthrough() - Init a passthrough keyslot manager
> > diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> > index c4ef1fceead6..4542050eebfc 100644
> > --- a/drivers/md/dm-core.h
> > +++ b/drivers/md/dm-core.h
> > @@ -12,6 +12,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> >
> > @@ -49,6 +50,9 @@ struct mapped_device {
> >
> > int numa_node_id;
> > struct request_queue *queue;
> > +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> > + struct blk_keyslot_manager ksm;
> > +#endif
> >
> > atomic_t holders;
> > atomic_t open_count;
>
> Any reason you placed the ksm member where you did?
>
> Looking at 'struct blk_keyslot_manager' I'm really hating adding that
> bloat to every DM device for a feature that really won't see much broad
> use (AFAIK).
>
> Any chance you could allocate 'struct blk_keyslot_manager' as needed so
> that most users of DM would only be carrying 1 extra pointer (set to
> NULL)?
I don't think there's any technical problem with doing that - the only
other thing that would need addressing is that the pa
Re: [PATCH 3/6] seccomp: Implement constant action bitmaps
On Wed, Sep 23, 2020 at 07:36:47PM -0500, YiFei Zhu wrote:
> On Wed, Sep 23, 2020 at 6:29 PM Kees Cook wrote:
> > In order to optimize these cases from O(n) to O(1), seccomp can
> > use bitmaps to immediately determine the desired action. A critical
> > observation in the prior paragraph bears repeating: the common case for
> > syscall tests do not check arguments. For any given filter, there is a
> > constant mapping from the combination of architecture and syscall to the
> > seccomp action result. (For kernels/architectures without CONFIG_COMPAT,
> > there is a single architecture.). As such, it is possible to construct
> > a mapping of arch/syscall to action, which can be updated as new filters
> > are attached to a process.
>
> Would you mind educating me how this patch plan one handling MIPS? For
> one kernel they seem to have up to three arch numbers per build,
> AUDIT_ARCH_MIPS{,64,64N32}. Though ARCH_TRACE_IGNORE_COMPAT_SYSCALLS
> does not seem to be defined for MIPS so I'm assuming the syscall
> numbers are the same, but I think it is possible some client uses that
> arch number to pose different constraints for different processes, so
> it would better not accelerate them rather than break them.
I'll take a look, but I'm hoping it won't be too hard to fit into what
I've got designed so for to deal with x86_x32. (Will MIPS want this
optimization at all?)
--
Kees Cook
Re: [PATCH v13 2/2] Add PWM fan controller driver for LGM SoC
On 24/9/2020 3:12 pm, Thierry Reding wrote: > On Thu, Sep 24, 2020 at 08:55:34AM +0200, Uwe Kleine-König wrote: >> Hello, >> >> (hhm Thierry already announced to have taken this patch, so my review is >> late.) > There's also a build warning in linux-next caused by this patch, so I'm > going to back it out. > > Rahul, please address Uwe's comments and make sure to fix the build > warning as well. Well noted, will do. Regards, Rahul > Thierry
Re: [RFC PATCH 01/24] vhost-vdpa: fix backend feature ioctls
On Thu, Sep 24, 2020 at 03:26:08PM +0800, Jason Wang wrote:
>
> On 2020/9/24 下午3:16, Eli Cohen wrote:
> > On Thu, Sep 24, 2020 at 11:21:02AM +0800, Jason Wang wrote:
> > > Commit 653055b9acd4 ("vhost-vdpa: support get/set backend features")
> > > introduces two malfunction backend features ioctls:
> > >
> > > 1) the ioctls was blindly added to vring ioctl instead of vdpa device
> > > ioctl
> > > 2) vhost_set_backend_features() was called when dev mutex has already
> > > been held which will lead a deadlock
> > >
> > I assume this patch requires some patch in qemu as well. Do you have
> > such patch?
> >
>
> It's this series: [PATCH 0/3] Vhost-vDPA: batch IOTLB updating.
>
> You were copied.
>
Right, I miss those.
Thanks.
Re: [PATCH v3] usb: dwc3: Stop active transfers before halting the controller
Hi,
Wesley Cheng writes:
> On 9/6/2020 11:20 PM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Wesley Cheng writes:
>>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>>> index 59f2e8c31bd1..456aa87e8778 100644
>>> --- a/drivers/usb/dwc3/ep0.c
>>> +++ b/drivers/usb/dwc3/ep0.c
>>> @@ -197,7 +197,7 @@ int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct
>>> usb_request *request,
>>> int ret;
>>>
>>> spin_lock_irqsave(&dwc->lock, flags);
>>> - if (!dep->endpoint.desc) {
>>> + if (!dep->endpoint.desc || !dwc->pullups_connected) {
>>
>> this looks odd. If we don't have pullups connected, we shouldn't have a
>> descriptor, likewise if we don't have a a description, we haven't been
>> enumerated, therefore we shouldn't have pullups connected.
>>
>> What am I missing here?
>>
>
> Hi Felipe,
>
> When we
> echo "" > /sys/kernel/config/usb_gadget/g1/UDC
>
> This triggers the usb_gadget_disconnect() routine to execute.
>
> int usb_gadget_disconnect(struct usb_gadget *gadget)
> {
> ...
> ret = gadget->ops->pullup(gadget, 0);
> if (!ret) {
> gadget->connected = 0;
> gadget->udc->driver->disconnect(gadget);
> }
>
> So it is possible that we've already disabled the pullup before running
> the disable() callbacks in the function drivers. The disable()
we used to have usage counts for those, are they gone? I think they're
still there.
> callbacks usually are the ones responsible for calling usb_ep_disable(),
> where we clear the desc field. This means there is a brief period where
> the pullups_connected = 0, but we still have valid ep desc, as it has
> not been disabled yet.
this is a valid point, though
> Also, for function drivers like mass storage, the fsg_disable() routine
> defers the actual usb_ep_disable() call to the fsg_thread, so its not
> always ensured that the disconnect() execution would result in the
> usb_ep_disable() to occur synchronously.
also a good point.
>>> @@ -1926,6 +1926,21 @@ static int dwc3_gadget_set_selfpowered(struct
>>> usb_gadget *g,
>>> return 0;
>>> }
>>>
>>> +static void dwc3_stop_active_transfers(struct dwc3 *dwc)
>>> +{
>>> + u32 epnum;
>>> +
>>> + for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
>>
>> dwc3 knows the number of endpoints available in the HW. Use dwc->num_eps
>> instead.
>>
>
> Sure, will do.
>
>>> @@ -1971,6 +1986,8 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int
>>> is_on, int suspend)
>>> return 0;
>>> }
>>>
>>> +static void __dwc3_gadget_stop(struct dwc3 *dwc);
>>> +
>>> static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
>>> {
>>> struct dwc3 *dwc = gadget_to_dwc(g);
>>> @@ -1994,9 +2011,37 @@ static int dwc3_gadget_pullup(struct usb_gadget *g,
>>> int is_on)
>>> }
>>> }
>>>
>>> + /*
>>> +* Synchronize and disable any further event handling while controller
>>> +* is being enabled/disabled.
>>> +*/
>>> + disable_irq(dwc->irq_gadget);
>>
>> why isn't dwc3_gadget_disable_irq() enough?
>>
>>> spin_lock_irqsave(&dwc->lock, flags);
>>
>> spin_lock_irqsave() will disable interrupts, why disable_irq() above?
>>
>
> In the discussion I had with Thinh, the concern was that with the newly
> added code to override the lpos here, if the interrupt routine
> (dwc3_check_event_buf()) runs, then it will reference the lpos for
that's running in hardirq context. All interrupts are disabled while
that runs, there's no risk of race, right?
> copying the event buffer contents to the event cache, and potentially
> process events. There is no locking in place, so it could be possible
> to have both run in parallel.
Is this academic or have you actually found a situation where this
could, indeed, happen? The spin_lock_irqsave() should be enough to
synchronize dwc3_gadget_pullup() and the interrupt handler.
> Hence, the reason if there was already a pending IRQ triggered, the
> dwc3_gadget_disable_irq() won't ensure the IRQ is handled. We can do
> something like:
> if (!is_on)
> dwc3_gadget_disable_irq()
> synchronize_irq()
> spin_lock_irqsave()
> if(!is_on) {
> ...
>
> But the logic to only apply this on the pullup removal case is a little
> messy. Also, from my understanding, the spin_lock_irqsave() will only
> disable the local CPU IRQs, but not the interrupt line on the GIC, which
> means other CPUs can handle it, unless we explicitly set the IRQ
> affinity to CPUX.
Yeah, the way I understand this can't really happen. But I'm open to
being educated. Maybe Alan can explain if this is really possibility?
>>> + dwc3_stop_active_transfers(dwc);
>>> + __dwc3_gadget_stop(dwc);
>>> +
>>> + count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
>>> + count &= DWC3_GEVNTCOUNT_MASK;
>>> + if (count > 0) {
>>> + dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);
>>> + dwc->ev_buf->lpos =
Re: [PATCH v2] ovl: introduce new "index=nouuid" option for inodes index feature
On 9/23/20 7:36 PM, Amir Goldstein wrote:
@@ -414,7 +415,7 @@ static int ovl_check_origin(struct ovl_fs *ofs, struct
dentry *upperdentry,
* Return 0 on match, -ESTALE on mismatch, < 0 on error.
*/
static int ovl_verify_fh(struct dentry *dentry, const char *name,
-const struct ovl_fh *fh)
+const struct ovl_fh *fh, bool nouuid)
{
struct ovl_fh *ofh = ovl_get_fh(dentry, name);
int err = 0;
@@ -425,8 +426,14 @@ static int ovl_verify_fh(struct dentry *dentry, const char
*name,
if (IS_ERR(ofh))
return PTR_ERR(ofh);
- if (fh->fb.len != ofh->fb.len || memcmp(&fh->fb, &ofh->fb, fh->fb.len))
+ if (fh->fb.len != ofh->fb.len) {
err = -ESTALE;
+ } else {
+ if (nouuid && !uuid_equal(&fh->fb.uuid, &ofh->fb.uuid))
+ ofh->fb.uuid = fh->fb.uuid;
+ if (memcmp(&fh->fb, &ofh->fb, fh->fb.len))
+ err = -ESTALE;
+ }
On second thought I am wondering if we should do that differently.
If users want to work with index=nouuid, they need to work with it from day 1.
index=nouuid should export null uuid in NFS handles and write null uuid
in trusted.overlay.origin xattr.
So in ovl_encode_real_fh() you set null uuid and
instead of relaxing uuid_equal() in ovl_decode_real_fh()
you change it to uuid_is_null().
Do you have a problem with that for Virtuozzo use case?
Actually we've enabled index=on by default in kernel config in Virtuozzo
only in the new update which is not yet released. So probably we can
switch to index=nouuid with null uuid in fh.
Thanks,
Amir.
--
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.
Re: KASAN: stack-out-of-bounds Read in xfrm_selector_match (2)
On Mon, Sep 21, 2020 at 07:56:20AM -0700, syzbot wrote: > Hello, > > syzbot found the following issue on: > > HEAD commit:eb5f95f1 Merge tag 's390-5.9-6' of git://git.kernel.org/pu.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=13996ad590 > kernel config: https://syzkaller.appspot.com/x/.config?x=ffe85b197a57c180 > dashboard link: https://syzkaller.appspot.com/bug?extid=577fbac3145a6eb2e7a5 > compiler: gcc (GCC) 10.1.0-syz 20200507 > > Unfortunately, I don't have any reproducer for this issue yet. > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+577fbac3145a6eb2e...@syzkaller.appspotmail.com > > == > BUG: KASAN: stack-out-of-bounds in xfrm_flowi_dport include/net/xfrm.h:877 > [inline] > BUG: KASAN: stack-out-of-bounds in __xfrm6_selector_match > net/xfrm/xfrm_policy.c:216 [inline] > BUG: KASAN: stack-out-of-bounds in xfrm_selector_match+0xf36/0xf60 > net/xfrm/xfrm_policy.c:229 > Read of size 2 at addr c9001914f55c by task syz-executor.4/15633 This is yet another ipv4 mapped ipv6 address with IPsec socket policy combination bug, and I'm sure it is not the last one. We could fix this one by adding another check to match the address family of the policy and the SA selector, but maybe it is better to think about how this should work at all. We can have only one socket policy for each direction and that policy accepts either ipv4 or ipv6. We treat this ipv4 mapped ipv6 address as ipv4 and pass it down the ipv4 stack, so this dual usage will not work with a socket policy. Maybe we can require IPV6_V6ONLY for sockets with policy attached. Thoughts?
[PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
From: Rui Feng
RTS5261 support legacy SD mode and SD Express mode.
In SD7.x, SD association introduce SD Express as a new mode.
This patch makes RTS5261 support SD Express mode,
and this patch is based on patch "mmc: core: Initial support
for SD express card/host" committed by Ulf Hansson.
Signed-off-by: Rui Feng
---
drivers/misc/cardreader/rts5261.c | 4 ++
drivers/misc/cardreader/rts5261.h | 23
drivers/misc/cardreader/rtsx_pcr.c | 5 +++
drivers/mmc/host/rtsx_pci_sdmmc.c | 59 ++
include/linux/rtsx_pci.h | 28 ++
5 files changed, 96 insertions(+), 23 deletions(-)
diff --git a/drivers/misc/cardreader/rts5261.c
b/drivers/misc/cardreader/rts5261.c
index 471961487ff8..536c90d4fd76 100644
--- a/drivers/misc/cardreader/rts5261.c
+++ b/drivers/misc/cardreader/rts5261.c
@@ -738,8 +738,12 @@ void rts5261_init_params(struct rtsx_pcr *pcr)
{
struct rtsx_cr_option *option = &pcr->option;
struct rtsx_hw_param *hw_param = &pcr->hw_param;
+ u8 val;
pcr->extra_caps = EXTRA_CAPS_SD_SDR50 | EXTRA_CAPS_SD_SDR104;
+ rtsx_pci_read_register(pcr, RTS5261_FW_STATUS, &val);
+ if (!(val & RTS5261_EXPRESS_LINK_FAIL_MASK))
+ pcr->extra_caps |= EXTRA_CAPS_SD_EXPRESS;
pcr->num_slots = 1;
pcr->ops = &rts5261_pcr_ops;
diff --git a/drivers/misc/cardreader/rts5261.h
b/drivers/misc/cardreader/rts5261.h
index ebfdd236a553..8d80f0d5d5d6 100644
--- a/drivers/misc/cardreader/rts5261.h
+++ b/drivers/misc/cardreader/rts5261.h
@@ -65,23 +65,6 @@
#define RTS5261_FW_EXPRESS_TEST_MASK (0x01<<0)
#define RTS5261_FW_EA_MODE_MASK(0x01<<5)
-/* FW config register */
-#define RTS5261_FW_CFG00xFF54
-#define RTS5261_FW_ENTER_EXPRESS (0x01<<0)
-
-#define RTS5261_FW_CFG10xFF55
-#define RTS5261_SYS_CLK_SEL_MCU_CLK(0x01<<7)
-#define RTS5261_CRC_CLK_SEL_MCU_CLK(0x01<<6)
-#define RTS5261_FAKE_MCU_CLOCK_GATING (0x01<<5)
-/*MCU_bus_mode_sel: 0=real 8051 1=fake mcu*/
-#define RTS5261_MCU_BUS_SEL_MASK (0x01<<4)
-/*MCU_clock_sel:VerA 00=aux16M 01=aux400K 1x=REFCLK100M*/
-/*MCU_clock_sel:VerB 00=aux400K 01=aux16M 10=REFCLK100M*/
-#define RTS5261_MCU_CLOCK_SEL_MASK (0x03<<2)
-#define RTS5261_MCU_CLOCK_SEL_16M (0x01<<2)
-#define RTS5261_MCU_CLOCK_GATING (0x01<<1)
-#define RTS5261_DRIVER_ENABLE_FW (0x01<<0)
-
/* FW status register */
#define RTS5261_FW_STATUS 0xFF56
#define RTS5261_EXPRESS_LINK_FAIL_MASK (0x01<<7)
@@ -121,12 +104,6 @@
#define RTS5261_DV3318_19 (0x04<<4)
#define RTS5261_DV3318_33 (0x07<<4)
-#define RTS5261_LDO1_CFG0 0xFF72
-#define RTS5261_LDO1_OCP_THD_MASK (0x07<<5)
-#define RTS5261_LDO1_OCP_EN(0x01<<4)
-#define RTS5261_LDO1_OCP_LMT_THD_MASK (0x03<<2)
-#define RTS5261_LDO1_OCP_LMT_EN(0x01<<1)
-
/* CRD6603-433 190319 request changed */
#define RTS5261_LDO1_OCP_THD_740 (0x00<<5)
#define RTS5261_LDO1_OCP_THD_800 (0x01<<5)
diff --git a/drivers/misc/cardreader/rtsx_pcr.c
b/drivers/misc/cardreader/rtsx_pcr.c
index 37ccc67f4914..6e5c16b4b7d1 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -990,6 +990,11 @@ static irqreturn_t rtsx_pci_isr(int irq, void *dev_id)
} else {
pcr->card_removed |= SD_EXIST;
pcr->card_inserted &= ~SD_EXIST;
+ if (PCI_PID(pcr) == PID_5261) {
+ rtsx_pci_write_register(pcr, RTS5261_FW_STATUS,
+ RTS5261_EXPRESS_LINK_FAIL_MASK, 0);
+ pcr->extra_caps |= EXTRA_CAPS_SD_EXPRESS;
+ }
}
pcr->dma_error_count = 0;
}
diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c
b/drivers/mmc/host/rtsx_pci_sdmmc.c
index 2763a376b054..efde374a4a5e 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -895,7 +895,9 @@ static int sd_set_bus_width(struct realtek_pci_sdmmc *host,
static int sd_power_on(struct realtek_pci_sdmmc *host)
{
struct rtsx_pcr *pcr = host->pcr;
+ struct mmc_host *mmc = host->mmc;
int err;
+ u32 val;
if (host->power_state == SDMMC_POWER_ON)
return 0;
@@ -922,6 +924,14 @@ static int sd_power_on(struct realtek_pci_sdmmc *host)
if (err < 0)
return err;
+ if (PCI_PID(pcr) == PID_5261) {
+ val = rtsx_pci_readl(pcr, RTSX_BIPR);
+ if (val & SD_WRITE_PROTECT) {
+ pcr->extra_caps &= ~EXTRA_CAPS_SD_EXPRESS;
+ mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
+ }
+ }
+
host->power_state = SDMMC_POWER_ON;
return 0;
}
@@ -1127,6 +1137,8 @@ static int sdmmc_get_cd(struct mm
Re: [RFC PATCH 8/9] surface_aggregator: Add DebugFS interface
On Thu, Sep 24, 2020 at 12:23 AM Maximilian Luz wrote: > On 9/23/20 8:51 PM, Arnd Bergmann wrote: > > On Wed, Sep 23, 2020 at 8:29 PM Maximilian Luz > > wrote: > >> On 9/23/20 6:48 PM, Arnd Bergmann wrote: > > > > The version I showed avoids the pointers and is compatible with > > 32-bit user space. > > I'm not completely convinced yet that the read/write approach is the way > I want to do it, especially with Greg suggesting a misc device, but I'll > keep your solution in mind. In case of a character device, I'd go with an ioctl to keep it extensible. The read/write based interface is what I'd use in debugfs. Arnd
Re: possible deadlock in xfrm_policy_delete
On Thu, Sep 24, 2020 at 09:30:03AM +0200, pet...@infradead.org wrote:
> On Thu, Sep 24, 2020 at 06:44:12AM +0200, Dmitry Vyukov wrote:
> > On Thu, Sep 24, 2020 at 6:36 AM Herbert Xu
> > wrote:
> > > > (k-slock-AF_INET6){+.-.}-{2:2}
>
> That's a seqlock.
Are you sure? Line 2503 in net/ipv4/tcp.c says
bh_lock_sock(sk);
> Did that tree you're testing include 267580db047e ("seqlock: Unbreak
> lockdep") ?
According to
https://syzkaller.appspot.com/bug?extid=c32502fd255cb3a44048
the git tree used was
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/include?id=5fa35f247b563a7893f3f68f19d00ace2ccf3dff
so no it doesn't contain that patch.
Hopefully this would address those seqlock backtraces. But what
about this particular one which is caused by the socket lock?
Thanks,
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: KASAN: stack-out-of-bounds Read in xfrm_selector_match (2)
On Thu, Sep 24, 2020 at 09:40:26AM +0200, Steffen Klassert wrote: > > This is yet another ipv4 mapped ipv6 address with IPsec socket policy > combination bug, and I'm sure it is not the last one. We could fix this > one by adding another check to match the address family of the policy > and the SA selector, but maybe it is better to think about how this > should work at all. > > We can have only one socket policy for each direction and that > policy accepts either ipv4 or ipv6. We treat this ipv4 mapped ipv6 > address as ipv4 and pass it down the ipv4 stack, so this dual usage > will not work with a socket policy. Maybe we can require IPV6_V6ONLY > for sockets with policy attached. Thoughts? I'm looking at the history of this and it used to work at the start because you'd always interpret the flow object with a family. This appears to have been lost with 8444cf712c5f71845cba9dc30d8f530ff0d5ff83. I'm working on a fix. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] KVM: Enable hardware before doing arch VM initialization
Hi, Paolo, On Thu, Sep 24, 2020 at 2:50 PM Paolo Bonzini wrote: > > On 24/09/20 08:31, Huacai Chen wrote: > > Hi, Sean, > > > > On Thu, Sep 24, 2020 at 3:00 AM Sean Christopherson > > wrote: > >> > >> Swap the order of hardware_enable_all() and kvm_arch_init_vm() to > >> accommodate Intel's Trust Domain Extension (TDX), which needs VMX to be > >> fully enabled during VM init in order to make SEAMCALLs. > >> > >> This also provides consistent ordering between kvm_create_vm() and > >> kvm_destroy_vm() with respect to calling kvm_arch_destroy_vm() and > >> hardware_disable_all(). > > Do you means that hardware_enable_all() enable VMX, kvm_arch_init_vm() > > enable TDX, and TDX depends on VMX enabled at first? If so, can TDX be > > also enabled at hardware_enable_all()? > > kvm_arch_init_vm() enables TDX *for the VM*, and to do that it needs VMX > instructions (specifically SEAMCALL, which is a hypervisor->"ultravisor" > call). Because that action is VM-specific it cannot be done in > hardware_enable_all(). > > Paolo OK, I know. Reviewed-by: Huacai Chen > > > The swapping seems not affect MIPS, but I observed a fact: > > kvm_arch_hardware_enable() not only be called at > > hardware_enable_all(), but also be called at kvm_starting_cpu(). Even > > if you swap the order, new starting CPUs are not enabled VMX before > > kvm_arch_init_vm(). (Maybe I am wrong because I'm not familiar with > > VMX/TDX). > > > > Huacai > >> > >> Cc: Marc Zyngier > >> Cc: James Morse > >> Cc: Julien Thierry > >> Cc: Suzuki K Poulose > >> Cc: linux-arm-ker...@lists.infradead.org > >> Cc: Huacai Chen > >> Cc: Aleksandar Markovic > >> Cc: linux-m...@vger.kernel.org > >> Cc: Paul Mackerras > >> Cc: kvm-...@vger.kernel.org > >> Cc: Christian Borntraeger > >> Cc: Janosch Frank > >> Cc: David Hildenbrand > >> Cc: Cornelia Huck > >> Cc: Claudio Imbrenda > >> Cc: Vitaly Kuznetsov > >> Cc: Wanpeng Li > >> Cc: Jim Mattson > >> Cc: Joerg Roedel > >> Signed-off-by: Sean Christopherson > >> --- > >> > >> Obviously not required until the TDX series comes along, but IMO KVM > >> should be consistent with respect to enabling and disabling virt support > >> in hardware. > >> > >> Tested only on Intel hardware. Unless I missed something, this only > >> affects x86, Arm and MIPS as hardware enabling is a nop for s390 and PPC. > >> Arm looks safe (based on my mostly clueless reading of the code), but I > >> have no idea if this will cause problem for MIPS, which is doing all kinds > >> of things in hardware_enable() that I don't pretend to fully understand. > >> > >> virt/kvm/kvm_main.c | 16 > >> 1 file changed, 8 insertions(+), 8 deletions(-) > >> > >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > >> index cf88233b819a..58fa19bcfc90 100644 > >> --- a/virt/kvm/kvm_main.c > >> +++ b/virt/kvm/kvm_main.c > >> @@ -766,7 +766,7 @@ static struct kvm *kvm_create_vm(unsigned long type) > >> struct kvm_memslots *slots = kvm_alloc_memslots(); > >> > >> if (!slots) > >> - goto out_err_no_arch_destroy_vm; > >> + goto out_err_no_disable; > >> /* Generations must be different for each address space. */ > >> slots->generation = i; > >> rcu_assign_pointer(kvm->memslots[i], slots); > >> @@ -776,19 +776,19 @@ static struct kvm *kvm_create_vm(unsigned long type) > >> rcu_assign_pointer(kvm->buses[i], > >> kzalloc(sizeof(struct kvm_io_bus), > >> GFP_KERNEL_ACCOUNT)); > >> if (!kvm->buses[i]) > >> - goto out_err_no_arch_destroy_vm; > >> + goto out_err_no_disable; > >> } > >> > >> kvm->max_halt_poll_ns = halt_poll_ns; > >> > >> - r = kvm_arch_init_vm(kvm, type); > >> - if (r) > >> - goto out_err_no_arch_destroy_vm; > >> - > >> r = hardware_enable_all(); > >> if (r) > >> goto out_err_no_disable; > >> > >> + r = kvm_arch_init_vm(kvm, type); > >> + if (r) > >> + goto out_err_no_arch_destroy_vm; > >> + > >> #ifdef CONFIG_HAVE_KVM_IRQFD > >> INIT_HLIST_HEAD(&kvm->irq_ack_notifier_list); > >> #endif > >> @@ -815,10 +815,10 @@ static struct kvm *kvm_create_vm(unsigned long type) > >> mmu_notifier_unregister(&kvm->mmu_notifier, current->mm); > >> #endif > >> out_err_no_mmu_notifier: > >> - hardware_disable_all(); > >> -out_err_no_disable: > >> kvm_arch_destroy_vm(kvm); > >> out_err_no_arch_destroy_vm: > >> + hardware_disable_all(); > >> +out_err_no_disable: > >> WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count)); > >> for (i = 0; i < KVM_NR_BUSES; i++) > >> kfree(kvm_get_bus(kvm, i)); > >> -- > >> 2.28.0 > >> > > >
Re: [PATCH 4/6] seccomp: Emulate basic filters for constant action results
On Thu, Sep 24, 2020 at 01:47:47AM +0200, Jann Horn wrote:
> On Thu, Sep 24, 2020 at 1:29 AM Kees Cook wrote:
> > This emulates absolutely the most basic seccomp filters to figure out
> > if they will always give the same results for a given arch/nr combo.
> >
> > Nearly all seccomp filters are built from the following ops:
> >
> > BPF_LD | BPF_W| BPF_ABS
> > BPF_JMP | BPF_JEQ | BPF_K
> > BPF_JMP | BPF_JGE | BPF_K
> > BPF_JMP | BPF_JGT | BPF_K
> > BPF_JMP | BPF_JSET | BPF_K
> > BPF_JMP | BPF_JA
> > BPF_RET | BPF_K
> >
> > These are now emulated to check for accesses beyond seccomp_data::arch
> > or unknown instructions.
> >
> > Not yet implemented are:
> >
> > BPF_ALU | BPF_AND (generated by libseccomp and Chrome)
>
> BPF_AND is normally only used on syscall arguments, not on the syscall
> number or the architecture, right? And when a syscall argument is
> loaded, we abort execution anyway. So I think there is no need to
> implement those?
Is that right? I can't actually tell what libseccomp is doing with
ALU|AND. It looks like it's using it for building jump lists?
Paul, Tom, under what cases does libseccomp emit ALU|AND into filters?
> > Suggested-by: Jann Horn
> > Link:
> > https://lore.kernel.org/lkml/CAG48ez1p=dR_2ikKq=xVxkoGg0fYpTBpkhJSv1w-6BG=76p...@mail.gmail.com/
> > Signed-off-by: Kees Cook
> > ---
> > kernel/seccomp.c | 82 ---
> > net/core/filter.c | 3 +-
> > 2 files changed, 79 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index 111a238bc532..9921f6f39d12 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -610,7 +610,12 @@ static struct seccomp_filter
> > *seccomp_prepare_filter(struct sock_fprog *fprog)
> > {
> > struct seccomp_filter *sfilter;
> > int ret;
> > - const bool save_orig = IS_ENABLED(CONFIG_CHECKPOINT_RESTORE);
> > + const bool save_orig =
> > +#if defined(CONFIG_CHECKPOINT_RESTORE) || defined(SECCOMP_ARCH)
> > + true;
> > +#else
> > + false;
> > +#endif
>
> You could probably write this as something like:
>
> const bool save_orig = IS_ENABLED(CONFIG_CHECKPOINT_RESTORE) ||
> __is_defined(SECCOMP_ARCH);
Ah! Thank you. I went looking for __is_defined() and failed. :)
>
> [...]
> > diff --git a/net/core/filter.c b/net/core/filter.c
> [...]
> > -static void bpf_release_orig_filter(struct bpf_prog *fp)
> > +void bpf_release_orig_filter(struct bpf_prog *fp)
> > {
> > struct sock_fprog_kern *fprog = fp->orig_prog;
> >
> > @@ -1154,6 +1154,7 @@ static void bpf_release_orig_filter(struct bpf_prog
> > *fp)
> > kfree(fprog);
> > }
> > }
> > +EXPORT_SYMBOL_GPL(bpf_release_orig_filter);
>
> If this change really belongs into this patch (which I don't think it
> does), please describe why in the commit message.
Yup, more cruft I failed to remove.
--
Kees Cook
Re: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings for nxp,pcf2127
Hi, On 24/09/2020 07:23:18+, Qiang Zhao wrote: > > > Yes, you are right, There is not a fundamental solution. > > > However it somewhat avoid this situation at least. > > > > > > And if without this issue, > > > is it correct to register a rtc device as watchdog no matter it is used as > > watchdog on the board? > > > Every time Linux are booted up, watchdog device should be configured to > > > the > > right one manually. > > > So the patch are useful, even though it is not for the issue. > > > > > > What should we do to really resolve this issue? > > > > I still think we need a kernel solution here. I would expect that most > > assembled > > pcf2127 chips are unable to act as a watchdog (i.e. don't have the RST > > output > > connected to something that resets the machine). > > > > So my favoured solution would be a positive property like: > > > > has-watchdog; > > > > or something similar. In my eyes this is definitely something we want to > > specify > > in the device tree because it is a relevant hardware property. > > I consider it a bug to give a watchdog device to userspace that isn't > > functional. > > > > Best regards > > Uwe > > I strongly agree with you! It should be positive property. > However, we couldn't identify which board are using pcf2127 as watchdog, > So we are unable to modify the boards' dts to correct (watchdog or not) in > this patchset. > > I noticed that only LS series platforms and imx6 have pcf2127 node, as far as > I know, the LS platforms don't use it as watchdog, > But I am not sure about imx6 > I don't think there is any user upstream and it is recent engouh that we can probably make that a positive property. Bruno, is it ok for you? you are the only know user of the feature. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 2/3] dm: add support for passing through inline crypto support
On Wed, Sep 23, 2020 at 09:21:03PM -0400, Mike Snitzer wrote:
> On Wed, Sep 09 2020 at 7:44pm -0400,
> Satya Tangirala wrote:
>
> > From: Eric Biggers
> >
> > Update the device-mapper core to support exposing the inline crypto
> > support of the underlying device(s) through the device-mapper device.
> >
> > This works by creating a "passthrough keyslot manager" for the dm
> > device, which declares support for encryption settings which all
> > underlying devices support. When a supported setting is used, the bio
> > cloning code handles cloning the crypto context to the bios for all the
> > underlying devices. When an unsupported setting is used, the blk-crypto
> > fallback is used as usual.
> >
> > Crypto support on each underlying device is ignored unless the
> > corresponding dm target opts into exposing it. This is needed because
> > for inline crypto to semantically operate on the original bio, the data
> > must not be transformed by the dm target. Thus, targets like dm-linear
> > can expose crypto support of the underlying device, but targets like
> > dm-crypt can't. (dm-crypt could use inline crypto itself, though.)
> >
> > When a key is evicted from the dm device, it is evicted from all
> > underlying devices.
> >
> > Signed-off-by: Eric Biggers
> > Co-developed-by: Satya Tangirala
> > Signed-off-by: Satya Tangirala
> > ---
> > block/blk-crypto.c | 1 +
> > block/keyslot-manager.c | 34
> > drivers/md/dm-core.h| 4 ++
> > drivers/md/dm-table.c | 52 +++
> > drivers/md/dm.c | 92 -
> > include/linux/device-mapper.h | 6 +++
> > include/linux/keyslot-manager.h | 7 +++
> > 7 files changed, 195 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/blk-crypto.c b/block/blk-crypto.c
> > index 2d5e60023b08..33555cf0e3e7 100644
> > --- a/block/blk-crypto.c
> > +++ b/block/blk-crypto.c
> > @@ -402,3 +402,4 @@ int blk_crypto_evict_key(struct request_queue *q,
> > */
> > return blk_crypto_fallback_evict_key(key);
> > }
> > +EXPORT_SYMBOL_GPL(blk_crypto_evict_key);
> > diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
> > index 60ac406d54b9..e0f776c38d8a 100644
> > --- a/block/keyslot-manager.c
> > +++ b/block/keyslot-manager.c
> > @@ -416,6 +416,40 @@ void blk_ksm_unregister(struct request_queue *q)
> > {
> > q->ksm = NULL;
> > }
> > +EXPORT_SYMBOL_GPL(blk_ksm_unregister);
> > +
> > +/**
> > + * blk_ksm_intersect_modes() - restrict supported modes by child device
> > + * @parent: The keyslot manager for parent device
> > + * @child: The keyslot manager for child device, or NULL
> > + *
> > + * Clear any crypto mode support bits in @parent that aren't set in @child.
> > + * If @child is NULL, then all parent bits are cleared.
> > + *
> > + * Only use this when setting up the keyslot manager for a layered device,
> > + * before it's been exposed yet.
> > + */
> > +void blk_ksm_intersect_modes(struct blk_keyslot_manager *parent,
> > +const struct blk_keyslot_manager *child)
> > +{
> > + if (child) {
> > + unsigned int i;
> > +
> > + parent->max_dun_bytes_supported =
> > + min(parent->max_dun_bytes_supported,
> > + child->max_dun_bytes_supported);
> > + for (i = 0; i < ARRAY_SIZE(child->crypto_modes_supported);
> > +i++) {
> > + parent->crypto_modes_supported[i] &=
> > + child->crypto_modes_supported[i];
> > + }
> > + } else {
> > + parent->max_dun_bytes_supported = 0;
> > + memset(parent->crypto_modes_supported, 0,
> > + sizeof(parent->crypto_modes_supported));
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(blk_ksm_intersect_modes);
> >
> > /**
> > * blk_ksm_init_passthrough() - Init a passthrough keyslot manager
> > diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> > index c4ef1fceead6..4542050eebfc 100644
> > --- a/drivers/md/dm-core.h
> > +++ b/drivers/md/dm-core.h
> > @@ -12,6 +12,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> >
> > @@ -49,6 +50,9 @@ struct mapped_device {
> >
> > int numa_node_id;
> > struct request_queue *queue;
> > +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> > + struct blk_keyslot_manager ksm;
> > +#endif
> >
> > atomic_t holders;
> > atomic_t open_count;
>
> Any reason you placed the ksm member where you did?
As in, any reason why it's placed right after the struct request_queue
*queue? The ksm is going to be set up in the request_queue and is a part
of the request_queue is some sense, so it seemed reasonable to me to
group them togetherbut I don't think there's any reason it *has* to
be there, if you think it should be put elsewhere (or maybe I'm
misunderstanding your question :) ).
>
> Looking at 'struct blk_keyslot_manager' I'm really hat
Re: [PATCH v9 12/20] gpiolib: cdev: support setting debounce
On Wed, Sep 23, 2020 at 07:27:37PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 22, 2020 at 5:36 AM Kent Gibson wrote:
> >
> > Add support for setting debounce on a line via the GPIO uAPI.
> > Where debounce is not supported by hardware, a software debounce is
> > provided.
> >
> > The implementation of the software debouncer waits for the line to be
> > stable for the debounce period before determining if a level change,
> > and a corresponding edge event, has occurred. This provides maximum
> > protection against glitches, but also introduces a debounce_period
> > latency to edge events.
> >
> > The software debouncer is integrated with the edge detection as it
> > utilises the line interrupt, and integration is simpler than getting
> > the two to interwork. Where software debounce AND edge detection is
> > required, the debouncer provides both.
>
>
> > +static unsigned int debounced_value(struct line *line)
> > +{
> > + unsigned int value;
> > +
> > + /*
> > +* minor race - debouncer may be stopped here, so edge_detector_stop
>
> () ?
>
> > +* must leave the value unchanged so the following will read the
> > level
> > +* from when the debouncer was last running.
> > +*/
> > + value = READ_ONCE(line->level);
> > +
>
> > + if (test_bit(FLAG_ACTIVE_LOW, &line->desc->flags))
> > + value = !value;
>
> I'm not sure what this means in terms of unsingned int to be returned.
>
> > + return value;
>
> Shouldn't we rather return 0/1 guaranteed?
>
> Perhaps
>
> if (active_low)
> return !value;
>
> return !!value;
>
> ?
>
Or just make the return value a bool?
[snip]
> > +
> > +static void debounce_work_func(struct work_struct *work)
> > +{
> > + struct gpio_v2_line_event le;
> > + struct line *line = container_of(work, struct line, work.work);
> > + struct linereq *lr;
> > + int level;
> > +
> > + level = gpiod_get_raw_value_cansleep(line->desc);
> > + if (level < 0) {
> > + pr_debug_ratelimited("debouncer failed to read line
> > value\n");
> > + return;
> > + }
> > +
> > + if (READ_ONCE(line->level) == level)
> > + return;
> > +
> > + WRITE_ONCE(line->level, level);
> > +
> > + /* -- edge detection -- */
> > + if (!line->eflags)
> > + return;
>
> > + /* switch from physical level to logical - if they differ */
> > + if (test_bit(FLAG_ACTIVE_LOW, &line->desc->flags))
> > + level = !level;
>
> Seems to me a good candidate to have
>
> static inline bool convert_with_active_low_respected(desc, value)
> {
> if (active_low)
>return !value;
> return !!value;
> }
>
Not sure it is worth the effort - it would only be used twice - here
and in debounced_value() - which is only a couple of lines itself.
[snip]
> > +
> > +static int debounce_setup(struct line *line,
> > + unsigned int debounce_period_us)
> > +{
> > + unsigned long irqflags;
> > + int ret, level, irq;
> > +
> > + /* try hardware */
> > + ret = gpiod_set_debounce(line->desc, debounce_period_us);
> > + if (!ret) {
> > + WRITE_ONCE(line->desc->debounce_period_us,
> > debounce_period_us);
> > + return ret;
> > + }
> > + if (ret != -ENOTSUPP)
> > + return ret;
> > +
> > + if (debounce_period_us) {
> > + /* setup software debounce */
> > + level = gpiod_get_raw_value_cansleep(line->desc);
> > + if (level < 0)
> > + return level;
> > +
> > + irq = gpiod_to_irq(line->desc);
> > + if (irq <= 0)
>
> Same question about return code...
>
Same answer...
[snip]
> > return 0;
> >
> > - edge_detector_stop(line);
> > + /* sw debounced and still will be...*/
>
> > + if ((debounce_period_us != 0) && READ_ONCE(line->sw_debounced)) {
>
> '( != 0)' are redundant. But I think you want to show that it's not
> boolean and we compare to 0...
>
Yeah, I guess I thought that was clearer, though I use the bare form
just below as well, and the bare form seems clear enough to me now, so
will change it for v10.
Cheers,
Kent.
> > + line->eflags = eflags;
> > + WRITE_ONCE(line->desc->debounce_period_us,
> > debounce_period_us);
> > + return 0;
> > + }
> > +
> > + /* reconfiguring edge detection or sw debounce being disabled */
> > + if ((line->irq && !READ_ONCE(line->sw_debounced)) ||
> > + (!debounce_period_us && READ_ONCE(line->sw_debounced)))
> > + edge_detector_stop(line);
> >
Re: [RFC PATCH 02/24] vhost-vdpa: fix vqs leak in vhost_vdpa_open()
On Thu, Sep 24, 2020 at 11:21:03AM +0800, Jason Wang wrote:
> We need to free vqs during the err path after it has been allocated
> since vhost won't do that for us.
>
> Signed-off-by: Jason Wang
> ---
> drivers/vhost/vdpa.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 796fe979f997..9c641274b9f3 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -764,6 +764,12 @@ static void vhost_vdpa_free_domain(struct vhost_vdpa *v)
> v->domain = NULL;
> }
>
> +static void vhost_vdpa_cleanup(struct vhost_vdpa *v)
> +{
> + vhost_dev_cleanup(&v->vdev);
> + kfree(v->vdev.vqs);
> +}
> +
Wouldn't it be cleaner to call kfree(vqs) explicilty inside
vhost_vdpa_open() in case of failure and keep the symetry of
vhost_dev_init()/vhost_dev_cleanup()?
> static int vhost_vdpa_open(struct inode *inode, struct file *filep)
> {
> struct vhost_vdpa *v;
> @@ -809,7 +815,7 @@ static int vhost_vdpa_open(struct inode *inode, struct
> file *filep)
> return 0;
>
> err_init_iotlb:
> - vhost_dev_cleanup(&v->vdev);
> + vhost_vdpa_cleanup(v);
> err:
> atomic_dec(&v->opened);
> return r;
> @@ -840,8 +846,7 @@ static int vhost_vdpa_release(struct inode *inode, struct
> file *filep)
> vhost_vdpa_free_domain(v);
> vhost_vdpa_config_put(v);
> vhost_vdpa_clean_irq(v);
> - vhost_dev_cleanup(&v->vdev);
> - kfree(v->vdev.vqs);
> + vhost_vdpa_cleanup(v);
> mutex_unlock(&d->mutex);
>
> atomic_dec(&v->opened);
> --
> 2.20.1
>
Re: [PATCH] usb: gadget: bcm63xx_udc: fix up the error of undeclared usb_debug_root
Chunfeng Yun writes:
> Fix up the build error caused by undeclared usb_debug_root
>
> Cc: stable
> Fixes: a66ada4f241c("usb: gadget: bcm63xx_udc: create debugfs directory under
> usb root")
> Reported-by: kernel test robot
> Signed-off-by: Chunfeng Yun
$ patch -p1 --dry-run p.patch
/usr/bin/patch: Only garbage was found in the patch input.
--
balbi
signature.asc
Description: PGP signature
Re: [RFC PATCH 01/24] vhost-vdpa: fix backend feature ioctls
On Thu, Sep 24, 2020 at 11:21:02AM +0800, Jason Wang wrote:
> Commit 653055b9acd4 ("vhost-vdpa: support get/set backend features")
> introduces two malfunction backend features ioctls:
>
> 1) the ioctls was blindly added to vring ioctl instead of vdpa device
>ioctl
> 2) vhost_set_backend_features() was called when dev mutex has already
>been held which will lead a deadlock
>
> This patch fixes the above issues.
>
> Cc: Eli Cohen
> Reported-by: Zhu Lingshan
> Fixes: 653055b9acd4 ("vhost-vdpa: support get/set backend features")
> Signed-off-by: Jason Wang
Don't we want the fixes queued right now, as opposed to the rest of the
RFC?
> ---
> drivers/vhost/vdpa.c | 30 --
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 3fab94f88894..796fe979f997 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -353,8 +353,6 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
> unsigned int cmd,
> struct vdpa_callback cb;
> struct vhost_virtqueue *vq;
> struct vhost_vring_state s;
> - u64 __user *featurep = argp;
> - u64 features;
> u32 idx;
> long r;
>
> @@ -381,18 +379,6 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
> unsigned int cmd,
>
> vq->last_avail_idx = vq_state.avail_index;
> break;
> - case VHOST_GET_BACKEND_FEATURES:
> - features = VHOST_VDPA_BACKEND_FEATURES;
> - if (copy_to_user(featurep, &features, sizeof(features)))
> - return -EFAULT;
> - return 0;
> - case VHOST_SET_BACKEND_FEATURES:
> - if (copy_from_user(&features, featurep, sizeof(features)))
> - return -EFAULT;
> - if (features & ~VHOST_VDPA_BACKEND_FEATURES)
> - return -EOPNOTSUPP;
> - vhost_set_backend_features(&v->vdev, features);
> - return 0;
> }
>
> r = vhost_vring_ioctl(&v->vdev, cmd, argp);
> @@ -440,8 +426,20 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> struct vhost_vdpa *v = filep->private_data;
> struct vhost_dev *d = &v->vdev;
> void __user *argp = (void __user *)arg;
> + u64 __user *featurep = argp;
> + u64 features;
> long r;
>
> + if (cmd == VHOST_SET_BACKEND_FEATURES) {
> + r = copy_from_user(&features, featurep, sizeof(features));
> + if (r)
> + return r;
> + if (features & ~VHOST_VDPA_BACKEND_FEATURES)
> + return -EOPNOTSUPP;
> + vhost_set_backend_features(&v->vdev, features);
> + return 0;
> + }
> +
> mutex_lock(&d->mutex);
>
> switch (cmd) {
> @@ -476,6 +474,10 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> case VHOST_VDPA_SET_CONFIG_CALL:
> r = vhost_vdpa_set_config_call(v, argp);
> break;
> + case VHOST_GET_BACKEND_FEATURES:
> + features = VHOST_VDPA_BACKEND_FEATURES;
> + r = copy_to_user(featurep, &features, sizeof(features));
> + break;
> default:
> r = vhost_dev_ioctl(&v->vdev, cmd, argp);
> if (r == -ENOIOCTLCMD)
> --
> 2.20.1
Re: [PATCH v9 17/20] tools: gpio: port gpio-hammer to v2 uAPI
On Wed, Sep 23, 2020 at 07:30:52PM +0300, Andy Shevchenko wrote: > On Tue, Sep 22, 2020 at 5:36 AM Kent Gibson wrote: > > > > Port the gpio-hammer tool to the latest GPIO uAPI. > > _BITUL() and _BITULL() are part of Linux uAPI. Why not to use them? > const.h > Yeah, that is an oversight on my part - will change for v10. Cheers, Kent.
Re: [PATCH 3/6] seccomp: Implement constant action bitmaps
On Thu, Sep 24, 2020 at 2:38 AM Kees Cook wrote:
> > Would you mind educating me how this patch plan one handling MIPS? For
> > one kernel they seem to have up to three arch numbers per build,
> > AUDIT_ARCH_MIPS{,64,64N32}. Though ARCH_TRACE_IGNORE_COMPAT_SYSCALLS
> > does not seem to be defined for MIPS so I'm assuming the syscall
> > numbers are the same, but I think it is possible some client uses that
> > arch number to pose different constraints for different processes, so
> > it would better not accelerate them rather than break them.
>
> I'll take a look, but I'm hoping it won't be too hard to fit into what
> I've got designed so for to deal with x86_x32. (Will MIPS want this
> optimization at all?)
I just took a slightly closer look at MIPS and it seems that they have
sparse syscall numbers (defines HAVE_SPARSE_SYSCALL_NR). I don't know
how the different "regions of syscall numbers" are affected by arch
numbers, however...
Re: [PATCH v3 3/4] venus: core: vote with average bandwidth and peak bandwidth as zero
Quoting Mansur Alisha Shaik (2020-09-23 23:51:05)
> As per bandwidth table video driver is voting with average bandwidth
> for "video-mem" and "cpu-cfg" paths as peak bandwidth is zero
> in bandwidth table.
>
> Fixes: 7482a983d ("media: venus: redesign clocks and pm domains control")
> Signed-off-by: Mansur Alisha Shaik
> ---
> Changes in v3:
> - Added fixes tag
>
> drivers/media/platform/qcom/venus/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.c
> b/drivers/media/platform/qcom/venus/core.c
> index fa363b8..d5bfd6f 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -385,11 +385,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->video_path, 0, kbps_to_icc(1000));
> + ret = icc_set_bw(core->video_path, kbps_to_icc(2), 0);
This gets added in the previous patch. Why not put this patch before
that one?
Anyway..
Reviewed-by: Stephen Boyd
Re: [PATCH v3 2/4] venus: core: vote for video-mem path
Quoting Mansur Alisha Shaik (2020-09-23 23:51:04)
> Currently video driver is 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().
>
> Fixes: 7482a983d ("media: venus: redesign clocks and pm domains control")
> Signed-off-by: Mansur Alisha Shaik
> ---
> Changes in v3:
> - Addressed review comments by Stephen Boyd
Please Cc me on patches I review.
>
> drivers/media/platform/qcom/venus/core.c | 17 -
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.c
> b/drivers/media/platform/qcom/venus/core.c
> index 52a3886..fa363b8 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -363,7 +363,18 @@ static __maybe_unused int venus_runtime_suspend(struct
> device *dev)
>
> ret = icc_set_bw(core->cpucfg_path, 0, 0);
> if (ret)
> - return ret;
> + goto err_cpucfg_path;
> +
> + ret = icc_set_bw(core->video_path, 0, 0);
> + if (ret)
> + goto err_video_path;
> +
> + return ret;
> +
> +err_video_path:
> + icc_set_bw(core->cpucfg_path, kbps_to_icc(1000), 0);
> +err_cpucfg_path:
> + pm_ops->core_power(dev, POWER_ON);
>
> return ret;
> }
> @@ -374,6 +385,10 @@ 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->video_path, 0, kbps_to_icc(1000));
> + if (ret)
> + return ret;
> +
Feels like this patch should come after the next one but OK.
Reviewed-by: Stephen Boyd
Re: [PATCH v3 4/4] venus: put dummy vote on video-mem path after last session release
Quoting Mansur Alisha Shaik (2020-09-23 23:51:06)
> As per current implementation, video driver is unvoting "videom-mem" path
> for last video session during vdec_session_release().
> While video playback when we try to suspend device, we see video clock
> warnings since votes are already removed during vdec_session_release().
>
> corrected this by putting dummy vote on "video-mem" after last video
> session release and unvoting it during suspend.
>
> Fixes: 7482a983d ("media: venus: redesign clocks and pm domains control")
> Signed-off-by: Mansur Alisha Shaik
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH] dt-bindings: iio: sx9310: Add various settings as DT properties
Quoting Stephen Boyd (2020-09-23 16:25:43) > > > > > + > > > > > + semtech,close-debounce-samples: > > > > > +allOf: > > > > > + - $ref: /schemas/types.yaml#definitions/uint32 > > > > > + - enum: [0, 2, 4, 8] > > > > > +default: 0 > > > > > +description: > > > > > + The number of close samples debounced for proximity/body > > > > > thresholds. > > > > > > > > This feels like something that has more to do with the object motion > > > > than > > > > the sensor setup, so perhaps should be controlled from userspace? > > > > > > Sure. Is there an IIO sample property? Or I should make a custom > > > knob for this? > > > > It's kind of close to in_proximity0_thresh_period and that may be how they > > have implemented it. > > > > That control specifies a number of samples for which a condition should be > > true > > before it is reported. > > Sounds good. I can do that. It looks like the driver reports close/far > via an event and these debounce values are the same for me so I can > write both fields (close and far) with the same thresh_period value from > userspace. If they need to be different between the two then this can be > reevaluated? > Or I can assign thresh_period to falling and rising corresponding to close/far debounce. Seems that the direction is the same, but that can be split apart and each direction gets a different sysfs file?
Re: [PATCH v9 00/20] gpio: cdev: add uAPI v2
On Wed, Sep 23, 2020 at 07:35:30PM +0300, Andy Shevchenko wrote: > On Tue, Sep 22, 2020 at 5:34 AM Kent Gibson wrote: > > > > This patchset defines and implements a new version of the > > GPIO CDEV uAPI to address existing 32/64-bit alignment issues, add > > support for debounce, event sequence numbers, and allow for requested > > lines with different configurations. > > It provides some future proofing by adding optional configuration fields > > and padding reserved for future use. > > > > The series can be partitioned into three blocks; the first two patches > > are minor fixes that impact later patches, the next eleven contain the > > v2 uAPI definition and implementation, and the final seven port the GPIO > > tools to the v2 uAPI and extend them to use new uAPI features. > > > > The more complicated patches include their own commentary where > > appropriate. > > For tools (there are minor comments, which not prevent to have a tag): > Reviewed-by: Andy Shevchenko > > For the rest I gave some comments but most of them are simply to > address. The uAPI definition I agree with after Arnd's comment. I > don't see big impediments to having this for v5.10. > > Thanks! > Thanks for your review - I nearly always learn something new from them, and you can be picky pain in the arse at times - which is a good thing for a reviewer. Apart from the pain in the arse ;-). Cheers, Kent.
[PATCH] cpuidle: change #ifdef for the declaration of cpuidle_enter_s2idle()
From: zhuguangqing
Currently, if CONFIG_SUSPEND=n and CONFIG_CPU_IDLE=y, the function
cpuidle_enter_s2idle() is declared but not defined, it may cause error
when cpuidle_enter_s2idle() is called.
If CONFIG_SUSPEND=y and CONFIG_CPU_IDLE=n, the function
cpuidle_enter_s2idle() is defined as "return -ENODEV;" which is not
supposed to be.
Change #ifdef CONFIG_CPU_IDLE to #ifdef CONFIG_SUSPEND for
cpuidle_enter_s2idle() in cpuidle.h, which is consistent with its
defination in cpuidle.c.
Signed-off-by: zhuguangqing
---
include/linux/cpuidle.h | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 6175c77bf25e..2aa8cead1727 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -216,22 +216,26 @@ static inline struct cpuidle_device
*cpuidle_get_device(void) {return NULL; }
extern int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
struct cpuidle_device *dev,
u64 latency_limit_ns);
-extern int cpuidle_enter_s2idle(struct cpuidle_driver *drv,
- struct cpuidle_device *dev);
extern void cpuidle_use_deepest_state(u64 latency_limit_ns);
#else
static inline int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
struct cpuidle_device *dev,
u64 latency_limit_ns)
{return -ENODEV; }
-static inline int cpuidle_enter_s2idle(struct cpuidle_driver *drv,
- struct cpuidle_device *dev)
-{return -ENODEV; }
static inline void cpuidle_use_deepest_state(u64 latency_limit_ns)
{
}
#endif
+#ifdef CONFIG_SUSPEND
+extern int cpuidle_enter_s2idle(struct cpuidle_driver *drv,
+ struct cpuidle_device *dev);
+#else
+static inline int cpuidle_enter_s2idle(struct cpuidle_driver *drv,
+ struct cpuidle_device *dev)
+{return -ENODEV; }
+#endif
+
/* kernel/sched/idle.c */
extern void sched_idle_set_state(struct cpuidle_state *idle_state);
extern void default_idle_call(void);
--
2.17.1
Re: KASAN: stack-out-of-bounds Read in xfrm_selector_match (2)
On Thu, Sep 24, 2020 at 05:43:51PM +1000, Herbert Xu wrote: > On Thu, Sep 24, 2020 at 09:40:26AM +0200, Steffen Klassert wrote: > > > > This is yet another ipv4 mapped ipv6 address with IPsec socket policy > > combination bug, and I'm sure it is not the last one. We could fix this > > one by adding another check to match the address family of the policy > > and the SA selector, but maybe it is better to think about how this > > should work at all. > > > > We can have only one socket policy for each direction and that > > policy accepts either ipv4 or ipv6. We treat this ipv4 mapped ipv6 > > address as ipv4 and pass it down the ipv4 stack, so this dual usage > > will not work with a socket policy. Maybe we can require IPV6_V6ONLY > > for sockets with policy attached. Thoughts? > > I'm looking at the history of this and it used to work at the start > because you'd always interpret the flow object with a family. This > appears to have been lost with 8444cf712c5f71845cba9dc30d8f530ff0d5ff83. I'm sure it can be fixed to work with either ipv4 or ipv6. If I understand that right, it should be possible to talk ipv4 and ipv6 through that socket, but the policy will accept only one address family. > I'm working on a fix. Thanks!
Re: [Linux-stm32] [PATCH 3/3] ARM: dts: stm32: update stm32mp151 for remote proc synchronisation support
Hi Ahmad,
On 9/24/20 7:45 AM, Ahmad Fatoum wrote:
> Hello Arnaud,
>
> On 8/27/20 9:21 AM, Arnaud Pouliquen wrote:
>> Two backup registers are used to store the Cortex-M4 state and the resource
>> table address.
>> Declare the tamp node and add associated properties in m4_rproc node
>> to allow Linux to attach to a firmware loaded by the first boot stages.
>>
>> Associated driver implementation is available in commit 9276536f455b3
>> ("remoteproc: stm32: Parse syscon that will manage M4 synchronisation").
>>
>> Signed-off-by: Arnaud Pouliquen
>> ---
>> arch/arm/boot/dts/stm32mp151.dtsi | 7 +++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32mp151.dtsi
>> b/arch/arm/boot/dts/stm32mp151.dtsi
>> index bfe29023fbd5..842ecffae73a 100644
>> --- a/arch/arm/boot/dts/stm32mp151.dtsi
>> +++ b/arch/arm/boot/dts/stm32mp151.dtsi
>> @@ -1541,6 +1541,11 @@
>> status = "disabled";
>> };
>>
>> +tamp: tamp@5c00a000 {
>> +compatible = "st,stm32-tamp", "syscon";
>> +reg = <0x5c00a000 0x400>;
>> +};
>> +
>
> Just saw this now. I have a pending patch adding this node as well:
> https://lore.kernel.org/patchwork/patch/1306971/
>
> For my use case, I need a "simple-mfd" compatible to allow child
> nodes to be probed.
>
> Could you CC me when you send out your v2, so I can rebase?
> (Or if you don't mind, just add the "simple-mfd" into the compatible
> list yourself :-)
I prefer that you propose the "simple-mfd" compatibility. I do not master the
consequence of adding it...
I will add you in cc of my v2 , but as your patch could trig some discussions
I would suggest to send in advance a patchset including this patch and your
patch
on top of, just add a reference to this series in the cover letter and explain
potential impact (or non impact) on the legacy.
Regards,
Arnaud
>
> Cheers
> Ahmad
>
>> /*
>> * Break node order to solve dependency probe issue between
>> * pinctrl and exti.
>> @@ -1717,6 +1722,8 @@
>> st,syscfg-holdboot = <&rcc 0x10C 0x1>;
>> st,syscfg-tz = <&rcc 0x000 0x1>;
>> st,syscfg-pdds = <&pwr_mcu 0x0 0x1>;
>> +st,syscfg-rsc-tbl = <&tamp 0x144 0x>;
>> +st,syscfg-m4-state = <&tamp 0x148 0x>;
>> status = "disabled";
>> };
>> };
>>
>
[PATCH v3 0/4] iio: adc: ad9467: add support for AD9434 & AD9265
This is a V2 & V3 for: https://lore.kernel.org/linux-iio/CA+U=DspC_W=j-eet38q1v+ytbntuxqwvucbkx0dj9hdgvtv...@mail.gmail.com/T/ V2 is: https://lore.kernel.org/linux-iio/20200922132559.38456-1-alexandru.ardel...@analog.com/T/ It does a bit of rework/unification of the 2 chip-info constants, so that it's easier to add new devices 2 this driver. V1 only added only AD9434, but when adding AD9265, I noticed that some things could be a bit more unified for vref_mask & default_output_mode. In V3 the dt-bindings patches were merged and the docs polished a bit. Changelog v2 -> v3: * merge dt-bindings patches from v2 into a single one * polish the dt-bindings docs to better cover/describe new parts Changelog v1 -> v2: * add AD9265 support * wrap axi-adc chip-info, to also define vref_mask & default_output_mode in the chip-info table Alexandru Ardelean (2): iio: adc: ad9467: wrap a axi-adc chip-info into a ad9467_chip_info type dt-bindings: iio: ad9467: add entries for for AD9434 & AD9265 ADCs Michael Hennerich (2): iio: adc: ad9467: add support for AD9434 high-speed ADC iio: adc: ad9467: add support for AD9265 high-speed ADC .../bindings/iio/adc/adi,ad9467.yaml | 15 ++- drivers/iio/adc/ad9467.c | 121 +- 2 files changed, 100 insertions(+), 36 deletions(-) -- 2.25.1
[PATCH v3 3/4] iio: adc: ad9467: add support for AD9265 high-speed ADC
From: Michael Hennerich
The AD9265 is a 16-bit, 125 MSPS analog-to-digital converter (ADC). The
AD9265 is designed to support communications applications where high
performance combined with low cost, small size, and versatility is
desired.
The ADC core features a multistage, differential pipelined architecture
with integrated output error correction logic to provide 16-bit accuracy at
125 MSPS data rates and guarantees no missing codes over the full operating
temperature range.
The ADC features a wide bandwidth differential sample-and-hold analog input
amplifier supporting a variety of user-selectable input ranges. It is
suitable for multiplexed systems that switch full-scale voltage levels in
successive channels and for sampling single-channel inputs at frequencies
well beyond the Nyquist rate. Combined with power and cost savings over
previously available ADCs, the AD9265 is suitable for applications in
communications, instrumentation and medical imaging.
Link:
https://www.analog.com/media/en/technical-documentation/data-sheets/AD9434.pdf
The driver supports the same register set as the AD9467, so the support for
this chip is added to the 'ad9467' driver.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/iio/adc/ad9467.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 54a5864bc698..19a45dd43796 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -76,6 +76,14 @@
/* AN877_ADC_REG_OUTPUT_DELAY */
#define AN877_ADC_DCO_DELAY_ENABLE 0x80
+/*
+ * Analog Devices AD9265 16-Bit, 125/105/80 MSPS ADC
+ */
+
+#define CHIPID_AD9265 0x64
+#define AD9265_DEF_OUTPUT_MODE 0x40
+#define AD9265_REG_VREF_MASK 0xC0
+
/*
* Analog Devices AD9434 12-Bit, 370/500 MSPS ADC
*/
@@ -93,6 +101,7 @@
#define AD9467_REG_VREF_MASK 0x0F
enum {
+ ID_AD9265,
ID_AD9434,
ID_AD9467,
};
@@ -167,6 +176,10 @@ static int ad9467_reg_access(struct adi_axi_adc_conv
*conv, unsigned int reg,
return 0;
}
+static const unsigned int ad9265_scale_table[][2] = {
+ {1250, 0x00}, {1500, 0x40}, {1750, 0x80}, {2000, 0xC0},
+};
+
static const unsigned int ad9434_scale_table[][2] = {
{1600, 0x1C}, {1580, 0x1D}, {1550, 0x1E}, {1520, 0x1F}, {1500, 0x00},
{1470, 0x01}, {1440, 0x02}, {1420, 0x03}, {1390, 0x04}, {1360, 0x05},
@@ -216,6 +229,18 @@ static const struct iio_chan_spec ad9467_channels[] = {
};
static const struct ad9467_chip_info ad9467_chip_tbl[] = {
+ [ID_AD9265] = {
+ .axi_adc_info = {
+ .id = CHIPID_AD9265,
+ .max_rate = 12500UL,
+ .scale_table = ad9265_scale_table,
+ .num_scales = ARRAY_SIZE(ad9265_scale_table),
+ .channels = ad9467_channels,
+ .num_channels = ARRAY_SIZE(ad9467_channels),
+ },
+ .default_output_mode = AD9265_DEF_OUTPUT_MODE,
+ .vref_mask = AD9265_REG_VREF_MASK,
+ },
[ID_AD9434] = {
.axi_adc_info = {
.id = CHIPID_AD9434,
@@ -432,6 +457,7 @@ static int ad9467_probe(struct spi_device *spi)
}
static const struct of_device_id ad9467_of_match[] = {
+ { .compatible = "adi,ad9265", .data = &ad9467_chip_tbl[ID_AD9265], },
{ .compatible = "adi,ad9434", .data = &ad9467_chip_tbl[ID_AD9434], },
{ .compatible = "adi,ad9467", .data = &ad9467_chip_tbl[ID_AD9467], },
{}
--
2.25.1
[PATCH] habanalabs/gaudi: configure QMAN LDMA registers properly
From: Ofir Bitton
LDMA registers are configured with a fixed value.
We add new define set which gives the configuration
a proper meaning.
Signed-off-by: Ofir Bitton
Reviewed-by: Oded Gabbay
Signed-off-by: Oded Gabbay
---
drivers/misc/habanalabs/gaudi/gaudi.c | 62 +-
drivers/misc/habanalabs/gaudi/gaudiP.h | 8
2 files changed, 49 insertions(+), 21 deletions(-)
diff --git a/drivers/misc/habanalabs/gaudi/gaudi.c
b/drivers/misc/habanalabs/gaudi/gaudi.c
index 1b51e670bd4e..a227806be328 100644
--- a/drivers/misc/habanalabs/gaudi/gaudi.c
+++ b/drivers/misc/habanalabs/gaudi/gaudi.c
@@ -1865,9 +1865,11 @@ static void gaudi_init_pci_dma_qman(struct hl_device
*hdev, int dma_id,
WREG32(mmDMA0_QM_PQ_PI_0 + q_off, 0);
WREG32(mmDMA0_QM_PQ_CI_0 + q_off, 0);
- WREG32(mmDMA0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off, 0x74);
- WREG32(mmDMA0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off, 0x14);
- WREG32(mmDMA0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off, 0x1C);
+ WREG32(mmDMA0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off, QMAN_LDMA_SIZE_OFFSET);
+ WREG32(mmDMA0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off,
+ QMAN_LDMA_SRC_OFFSET);
+ WREG32(mmDMA0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off,
+ QMAN_LDMA_DST_OFFSET);
WREG32(mmDMA0_QM_CP_MSG_BASE0_ADDR_LO_0 + q_off, mtr_base_en_lo);
WREG32(mmDMA0_QM_CP_MSG_BASE0_ADDR_HI_0 + q_off, mtr_base_en_hi);
@@ -2025,13 +2027,19 @@ static void gaudi_init_hbm_dma_qman(struct hl_device
*hdev, int dma_id,
WREG32(mmDMA0_QM_PQ_PI_0 + q_off, 0);
WREG32(mmDMA0_QM_PQ_CI_0 + q_off, 0);
- WREG32(mmDMA0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off, 0x81BC);
- WREG32(mmDMA0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off, 0x81B4);
- WREG32(mmDMA0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off, 0x1C);
+ WREG32(mmDMA0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off,
+ QMAN_CPDMA_SIZE_OFFSET);
+ WREG32(mmDMA0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off,
+ QMAN_CPDMA_SRC_OFFSET);
+ WREG32(mmDMA0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off,
+ QMAN_CPDMA_DST_OFFSET);
} else {
- WREG32(mmDMA0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off, 0x74);
- WREG32(mmDMA0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off, 0x14);
- WREG32(mmDMA0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off, 0x1C);
+ WREG32(mmDMA0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off,
+ QMAN_LDMA_SIZE_OFFSET);
+ WREG32(mmDMA0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off,
+ QMAN_LDMA_SRC_OFFSET);
+ WREG32(mmDMA0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off,
+ QMAN_LDMA_SIZE_OFFSET);
/* Configure RAZWI IRQ */
dma_qm_err_cfg = HBM_DMA_QMAN_GLBL_ERR_CFG_MSG_EN_MASK;
@@ -2135,13 +2143,19 @@ static void gaudi_init_mme_qman(struct hl_device *hdev,
u32 mme_offset,
WREG32(mmMME0_QM_PQ_PI_0 + q_off, 0);
WREG32(mmMME0_QM_PQ_CI_0 + q_off, 0);
- WREG32(mmMME0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off, 0x81BC);
- WREG32(mmMME0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off, 0x81B4);
- WREG32(mmMME0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off, 0x1C);
+ WREG32(mmMME0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off,
+ QMAN_CPDMA_SIZE_OFFSET);
+ WREG32(mmMME0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off,
+ QMAN_CPDMA_SRC_OFFSET);
+ WREG32(mmMME0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off,
+ QMAN_CPDMA_DST_OFFSET);
} else {
- WREG32(mmMME0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off, 0x74);
- WREG32(mmMME0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off, 0x14);
- WREG32(mmMME0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off, 0x1C);
+ WREG32(mmMME0_QM_CP_LDMA_TSIZE_OFFSET_0 + q_off,
+ QMAN_LDMA_SIZE_OFFSET);
+ WREG32(mmMME0_QM_CP_LDMA_SRC_BASE_LO_OFFSET_0 + q_off,
+ QMAN_LDMA_SRC_OFFSET);
+ WREG32(mmMME0_QM_CP_LDMA_DST_BASE_LO_OFFSET_0 + q_off,
+ QMAN_LDMA_DST_OFFSET);
/* Configure RAZWI IRQ */
mme_id = mme_offset /
@@ -2249,13 +2263,19 @@ static void gaudi_init_tpc_qman(struct hl_device *hdev,
u32 tpc_offset,
WR
[PATCH v3 2/4] iio: adc: ad9467: add support for AD9434 high-speed ADC
From: Michael Hennerich
The AD9434 is a 12-bit monolithic sampling analog-to-digital converter
(ADC) optimized for high performance, low power, and ease of use. The part
operates at up to a 500 MSPS conversion rate and is optimized for
outstanding dynamic performance in wideband carrier and broadband systems.
All necessary functions, including a sample-and-hold and voltage reference,
are included on the chip to provide a complete signal conversion solution.
The VREF pin can be used to monitor the internal reference or provide an
external voltage reference (external reference mode must be enabled through
the SPI port).
The ADC requires a 1.8 V analog voltage supply and a differential clock
for full performance operation. The digital outputs are LVDS (ANSI-644)
compatible and support twos complement, offset binary format, or Gray code.
A data clock output is available for proper output data timing.
Link:
https://www.analog.com/media/en/technical-documentation/data-sheets/AD9434.pdf
The driver supports the same register set as the AD9467, so the support for
this chip is added to the 'ad9467' driver.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/iio/adc/ad9467.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 85f5a014bd2d..54a5864bc698 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -76,6 +76,14 @@
/* AN877_ADC_REG_OUTPUT_DELAY */
#define AN877_ADC_DCO_DELAY_ENABLE 0x80
+/*
+ * Analog Devices AD9434 12-Bit, 370/500 MSPS ADC
+ */
+
+#define CHIPID_AD9434 0x6A
+#define AD9434_DEF_OUTPUT_MODE 0x00
+#define AD9434_REG_VREF_MASK 0xC0
+
/*
* Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
*/
@@ -85,6 +93,7 @@
#define AD9467_REG_VREF_MASK 0x0F
enum {
+ ID_AD9434,
ID_AD9467,
};
@@ -158,6 +167,13 @@ static int ad9467_reg_access(struct adi_axi_adc_conv
*conv, unsigned int reg,
return 0;
}
+static const unsigned int ad9434_scale_table[][2] = {
+ {1600, 0x1C}, {1580, 0x1D}, {1550, 0x1E}, {1520, 0x1F}, {1500, 0x00},
+ {1470, 0x01}, {1440, 0x02}, {1420, 0x03}, {1390, 0x04}, {1360, 0x05},
+ {1340, 0x06}, {1310, 0x07}, {1280, 0x08}, {1260, 0x09}, {1230, 0x0A},
+ {1200, 0x0B}, {1180, 0x0C},
+};
+
static const unsigned int ad9467_scale_table[][2] = {
{2000, 0}, {2100, 6}, {2200, 7},
{2300, 8}, {2400, 9}, {2500, 10},
@@ -191,11 +207,27 @@ static void __ad9467_get_scale(struct adi_axi_adc_conv
*conv, int index,
}, \
}
+static const struct iio_chan_spec ad9434_channels[] = {
+ AD9467_CHAN(0, 0, 12, 'S'),
+};
+
static const struct iio_chan_spec ad9467_channels[] = {
AD9467_CHAN(0, 0, 16, 'S'),
};
static const struct ad9467_chip_info ad9467_chip_tbl[] = {
+ [ID_AD9434] = {
+ .axi_adc_info = {
+ .id = CHIPID_AD9434,
+ .max_rate = 5UL,
+ .scale_table = ad9434_scale_table,
+ .num_scales = ARRAY_SIZE(ad9434_scale_table),
+ .channels = ad9434_channels,
+ .num_channels = ARRAY_SIZE(ad9434_channels),
+ },
+ .default_output_mode = AD9434_DEF_OUTPUT_MODE,
+ .vref_mask = AD9434_REG_VREF_MASK,
+ },
[ID_AD9467] = {
.axi_adc_info = {
.id = CHIPID_AD9467,
@@ -400,6 +432,7 @@ static int ad9467_probe(struct spi_device *spi)
}
static const struct of_device_id ad9467_of_match[] = {
+ { .compatible = "adi,ad9434", .data = &ad9467_chip_tbl[ID_AD9434], },
{ .compatible = "adi,ad9467", .data = &ad9467_chip_tbl[ID_AD9467], },
{}
};
--
2.25.1
[PATCH v3 4/4] dt-bindings: iio: ad9467: add entries for for AD9434 & AD9265 ADCs
Add entries for the AD9434 & AD9265 high-speed ADCs which are supported by the 'ad9467' driver. Better describe the family of ADCs similar to AD9467 in the description. Signed-off-by: Alexandru Ardelean --- .../devicetree/bindings/iio/adc/adi,ad9467.yaml | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml index c4f57fa6aad1..579dbc63e3fe 100644 --- a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml @@ -4,21 +4,30 @@ $id: http://devicetree.org/schemas/iio/adc/adi,ad9467.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Analog Devices AD9467 High-Speed ADC +title: Analog Devices AD9467 and similar High-Speed ADCs maintainers: - Michael Hennerich - Alexandru Ardelean description: | - The AD9467 is a 16-bit, monolithic, IF sampling analog-to-digital - converter (ADC). + The AD9467 and the parts similar with it, are high-speed analog-to-digital + converters (ADCs), operating in the range of 100 to 500 mega samples + per second (MSPS). Depending on the part, some support higher MSPS and some + lower MSPS, depending on the application each part is intended for. + All the parts support the register map described by Application Note AN-877 + https://www.analog.com/media/en/technical-documentation/application-notes/AN-877.pdf + + https://www.analog.com/media/en/technical-documentation/data-sheets/AD9265.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/AD9434.pdf https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf properties: compatible: enum: + - adi,ad9265 + - adi,ad9434 - adi,ad9467 reg: -- 2.25.1
[PATCH v3 1/4] iio: adc: ad9467: wrap a axi-adc chip-info into a ad9467_chip_info type
There are 2 chip constants that can be added to the chip-info part. The
default output-mode and the VREF mask.
When adding new chips to this driver, these can be easily omitted, because
these also need to be updated in 2 switch statements.
However, if adding them in the chip-info constants, they are updated in a
single place and propagated in both switch statements.
Signed-off-by: Alexandru Ardelean
---
drivers/iio/adc/ad9467.c | 62 +++-
1 file changed, 29 insertions(+), 33 deletions(-)
diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index f068256cfca9..85f5a014bd2d 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -88,6 +88,15 @@ enum {
ID_AD9467,
};
+struct ad9467_chip_info {
+ struct adi_axi_adc_chip_infoaxi_adc_info;
+ unsigned intdefault_output_mode;
+ unsigned intvref_mask;
+};
+
+#define to_ad9467_chip_info(_info) \
+ container_of(_info, struct ad9467_chip_info, axi_adc_info)
+
struct ad9467_state {
struct spi_device *spi;
struct clk *clk;
@@ -186,35 +195,31 @@ static const struct iio_chan_spec ad9467_channels[] = {
AD9467_CHAN(0, 0, 16, 'S'),
};
-static const struct adi_axi_adc_chip_info ad9467_chip_tbl[] = {
+static const struct ad9467_chip_info ad9467_chip_tbl[] = {
[ID_AD9467] = {
- .id = CHIPID_AD9467,
- .max_rate = 25000UL,
- .scale_table = ad9467_scale_table,
- .num_scales = ARRAY_SIZE(ad9467_scale_table),
- .channels = ad9467_channels,
- .num_channels = ARRAY_SIZE(ad9467_channels),
+ .axi_adc_info = {
+ .id = CHIPID_AD9467,
+ .max_rate = 25000UL,
+ .scale_table = ad9467_scale_table,
+ .num_scales = ARRAY_SIZE(ad9467_scale_table),
+ .channels = ad9467_channels,
+ .num_channels = ARRAY_SIZE(ad9467_channels),
+ },
+ .default_output_mode = AD9467_DEF_OUTPUT_MODE,
+ .vref_mask = AD9467_REG_VREF_MASK,
},
};
static int ad9467_get_scale(struct adi_axi_adc_conv *conv, int *val, int *val2)
{
const struct adi_axi_adc_chip_info *info = conv->chip_info;
+ const struct ad9467_chip_info *info1 = to_ad9467_chip_info(info);
struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
- unsigned int i, vref_val, vref_mask;
+ unsigned int i, vref_val;
vref_val = ad9467_spi_read(st->spi, AN877_ADC_REG_VREF);
- switch (info->id) {
- case CHIPID_AD9467:
- vref_mask = AD9467_REG_VREF_MASK;
- break;
- default:
- vref_mask = 0x;
- break;
- }
-
- vref_val &= vref_mask;
+ vref_val &= info1->vref_mask;
for (i = 0; i < info->num_scales; i++) {
if (vref_val == info->scale_table[i][1])
@@ -316,18 +321,6 @@ static int ad9467_preenable_setup(struct adi_axi_adc_conv
*conv)
return ad9467_outputmode_set(st->spi, st->output_mode);
}
-static int ad9467_setup(struct ad9467_state *st, unsigned int chip_id)
-{
- switch (chip_id) {
- case CHIPID_AD9467:
- st->output_mode = AD9467_DEF_OUTPUT_MODE |
- AN877_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
- return 0;
- default:
- return -ENODEV;
- }
-}
-
static void ad9467_clk_disable(void *data)
{
struct ad9467_state *st = data;
@@ -337,7 +330,7 @@ static void ad9467_clk_disable(void *data)
static int ad9467_probe(struct spi_device *spi)
{
- const struct adi_axi_adc_chip_info *info;
+ const struct ad9467_chip_info *info;
struct adi_axi_adc_conv *conv;
struct ad9467_state *st;
unsigned int id;
@@ -386,7 +379,7 @@ static int ad9467_probe(struct spi_device *spi)
spi_set_drvdata(spi, st);
- conv->chip_info = info;
+ conv->chip_info = &info->axi_adc_info;
id = ad9467_spi_read(spi, AN877_ADC_REG_CHIP_ID);
if (id != conv->chip_info->id) {
@@ -400,7 +393,10 @@ static int ad9467_probe(struct spi_device *spi)
conv->read_raw = ad9467_read_raw;
conv->preenable_setup = ad9467_preenable_setup;
- return ad9467_setup(st, id);
+ st->output_mode = info->default_output_mode |
+ AN877_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
+
+ return 0;
}
static const struct of_device_id ad9467_of_match[] = {
--
2.25.1
Re: [PATCH 22/24] membarrier.2: Note that glibc does not provide a wrapper
Hi Branden,
On 9/21/20 4:36 PM, G. Branden Robinson wrote:
> At 2020-09-11T12:58:08+, Walter Harms wrote:
>> the groff commands are ducument in man 7 groff
>> .nf No filling or adjusting of output-lines.
>> .fi Fill output lines
>>
>> (for me) a typical use is like this:
>> .nf
>>
>> struct timeval {
>> time_t tv_sec; /* seconds */
>> suseconds_t tv_usec;/* microseconds */
>> };
>> .fi
>>
>> In the top section you prevent indenting (if any).
>
> The above will not work as desired for typesetter output, a.k.a., "troff
> devices", such as PostScript or PDF. The initial code indent might work
> okay but the alignment of the field names will become
> ragged/mis-registered and the comments even more so.
Yes.
> This is because a proportional font is used by default for troff
> devices. The classical man macros, going back to Version 7 Unix (1979)
> had no good solution for this problem and Unix room tradition at Murray
> Hill going all the way back to (what we now call) the First Edition
> manual in 1971 was to read the man pages on a typewriter--a Teletype
> Model 33 or Model 37. Typewriters, of course, always[1] used monospaced
> fonts.
>
> Version 9 Unix (1986) introduced .EX and .EE for setting material in a
> monospaced font even if the device used proportional type by default.
> (Plan 9 troff inherited them.) GNU roff has supporteds .EX and .EE as
> well, for over 13 years, and its implementations are ultra-permissively
> licensed so other *roffs like Heirloom Doctools have picked them up.
> Therefore I recommend .EX and .EE for all code examples.
>
> They are very simple to use. In the above, simply replace ".nf" with
> ".EX" and ".fi" with ".EE".
>
> Regards,
> Branden
>
> [1] Not completely true; variable-pitch typewriters (such as 10/12 point
> selectable) were fairly common and some expensive models like the IBM
> Executive even featured true proportional type.
Thanks for the interesting history, Branden!
>From time toi time I wonder if the function prototypes in
the SYNOPSIS should also be inside .EX/.EE. Your thoughts?
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Re: [PATCH 3/6] seccomp: Implement constant action bitmaps
On Thu, Sep 24, 2020 at 2:37 AM Kees Cook wrote: > > > > This belongs over into patch 1. > > Thanks! I was rushing to get this posted so YiFei Zhu wouldn't spend > time fighting with arch and Kconfig stuff. :) I'll clean this (and the > other random cruft) up. Wait, what? I'm sorry. We have already begun fixing the mentioned issues (mostly the split bitmaps for different arches). Although yes it's nice to have another implementation to refer to so we get the best of both worlds (and yes I'm already copying some of the code I think are better here over there), don't you think it's not nice to say "Hey I've worked on this in June, it needed rework but I didn't send the newer version. Now you sent yours so I'll rush mine so your work is redundant."? That said, I do think this should be configurable. Users would be free to experiment with the bitmap on or off, just like users may turn seccomp off entirely. A choice also allows users to select different implementations, a few whom I work with have ideas on how to accelerate / cache argument dependent syscalls, for example. YiFei Zhu
Re: [PATCH v9 07/20] gpiolib: cdev: support GPIO_V2_GET_LINE_IOCTL and GPIO_V2_LINE_GET_VALUES_IOCTL
On Wed, Sep 23, 2020 at 02:11:54PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 22, 2020 at 5:35 AM Kent Gibson wrote:
> >
> > Add support for requesting lines using the GPIO_V2_GET_LINE_IOCTL, and
> > returning their current values using GPIO_V2_LINE_GET_VALUES_IOCTL.
> >
> > The struct linereq implementation is based on the v1 struct linehandle
> > implementation.
>
> ...
>
Ooops, nearly missed this one...
> > + /*
> > +* Do not allow OPEN_SOURCE & OPEN_DRAIN flags in a single request.
> > If
>
> You see, in some cases you are using "OR:ed" as understandable for
> programmers, and here & which should be and in plain English and
> really confusing from a programmer's perspective. That's why I prefer
> to see plain English rather than something which is full of encoded
> meanings.
>
Understand these are pulled directly from the v1 implementation, so I
think that is actually one of Bart's.
But, yeah, it should be 'and'.
> > +* the hardware actually supports enabling both at the same time the
> > +* electrical result would be disastrous.
> > +*/
>
> ...
>
> > + /* Bias requires explicit direction. */
> > + if ((flags & GPIO_V2_LINE_BIAS_FLAGS) &&
> > + !(flags & GPIO_V2_LINE_DIRECTION_FLAGS))
> > + return -EINVAL;
>
> Okay, since this is strict we probably may relax it in the future if
> it will be a use case.
> ...
>
Again, this is drawn directly from the v1 implementation, and I didn't go
changing anything from v1 without good reason.
> > + /* Only one bias flag can be set. */
>
> Ditto. (Some controllers allow to set both simultaneously, though I
> can't imagine good use case for that)
>
> > + if (((flags & GPIO_V2_LINE_FLAG_BIAS_DISABLED) &&
> > +(flags & (GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN |
> > + GPIO_V2_LINE_FLAG_BIAS_PULL_UP))) ||
> > + ((flags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN) &&
> > +(flags & GPIO_V2_LINE_FLAG_BIAS_PULL_UP)))
> > + return -EINVAL;
>
> ...
>
> > +static void gpio_v2_line_config_flags_to_desc_flags(u64 flags,
> > + unsigned long *flagsp)
> > +{
>
> > + assign_bit(FLAG_ACTIVE_LOW, flagsp,
> > + flags & GPIO_V2_LINE_FLAG_ACTIVE_LOW);
>
> What I meant is to attach also this to the other assign_bit():s below.
> And just in case a question: why not __asign_bit() do we really need
> atomicity?
>
These are initialized as per their order in the flags so it is easier to
tell if any are missing.
The atomicity is not required here, but it is elsewhere so you are
oblidged to use it for all accesses, no?
> > + if (flags & GPIO_V2_LINE_FLAG_OUTPUT)
> > + set_bit(FLAG_IS_OUT, flagsp);
> > + else if (flags & GPIO_V2_LINE_FLAG_INPUT)
> > + clear_bit(FLAG_IS_OUT, flagsp);
> > +
> > + assign_bit(FLAG_OPEN_DRAIN, flagsp,
> > + flags & GPIO_V2_LINE_FLAG_OPEN_DRAIN);
> > + assign_bit(FLAG_OPEN_SOURCE, flagsp,
> > + flags & GPIO_V2_LINE_FLAG_OPEN_SOURCE);
> > + assign_bit(FLAG_PULL_UP, flagsp,
> > + flags & GPIO_V2_LINE_FLAG_BIAS_PULL_UP);
> > + assign_bit(FLAG_PULL_DOWN, flagsp,
> > + flags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN);
> > + assign_bit(FLAG_BIAS_DISABLE, flagsp,
> > + flags & GPIO_V2_LINE_FLAG_BIAS_DISABLED);
> > +}
>
> ...
>
> > +static long linereq_get_values(struct linereq *lr, void __user *ip)
> > +{
> > + struct gpio_v2_line_values lv;
> > + DECLARE_BITMAP(vals, GPIO_V2_LINES_MAX);
> > + struct gpio_desc **descs;
> > + unsigned int i, didx, num_get;
> > + int ret;
>
> > + /* NOTE: It's ok to read values of output lines. */
> > + if (copy_from_user(&lv, ip, sizeof(lv)))
> > + return -EFAULT;
> > +
> > + for (num_get = 0, i = 0; i < lr->num_lines; i++) {
> > + if (lv.mask & BIT_ULL(i)) {
> > + num_get++;
> > + descs = &lr->lines[i].desc;
> > + }
> > + }
>
> So what you can do here is something like
>
> DECLARE_BITMAP(mask, u64);
>
> ...
>
> bitmap_from_u64(mask, lv.mask);
> num_get = bitmap_weight(mask, lr->num_lines);
> if (num_get == 0)
> return -EINVAL;
>
> for_each_set_bit(i, mask, lr->num_lines)
> descs = &lr->lines[i].desc;
> // I'm not sure I understood a purpose of the above
> // ah, looks like malloc() avoidance, but you may move it below...
>
> > + if (num_get == 0)
> > + return -EINVAL;
> > +
>
> > + if (num_get != 1) {
>
> ...something like
>
> if (num_get == 1)
> descs = ...[find_first_bit(mask, lr->num_lines)];
> else {
> ...
> for_each_set_bit() {
> ...
> }
> }
>
As per elsewhere - will give it a shot.
> > + descs = kmalloc_array(num_get, sizeof(*descs), GFP_KERNEL);
> >
Re: [PATCH v3 3/9] lib: zstd: Upgrade to latest upstream zstd version 1.4.6
Hi Nick, Thanks for the feedback, we'll take a look at these errors that maybe false positives. Best Regards, Rong Chen On 9/24/20 11:05 AM, Nick Terrell wrote: On Wed, Sep 23, 2020 at 7:28 PM kernel test robot wrote: Hi Nick, Thank you for the patch! Yet something to improve: [auto build test ERROR on kdave/for-next] [also build test ERROR on f2fs/dev-test linus/master v5.9-rc6 next-20200923] [cannot apply to cryptodev/master crypto/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Nick-Terrell/Update-to-zstd-1-4-6/20200924-064102 base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next config: h8300-randconfig-p002-20200923 (attached as .config) compiler: h8300-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=h8300 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): h8300-linux-ld: lib/zstd/common/entropy_common.o: in function `MEM_swap32': lib/zstd/common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/common/fse_decompress.o: in function `MEM_swap32': lib/zstd/common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/common/fse_decompress.o:lib/zstd/common/mem.h:179: more undefined references to `__bswapsi2' follow h8300-linux-ld: lib/zstd/compress/zstd_compress.o: in function `MEM_swap64': lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/zstd_compress.o: in function `MEM_swap32': lib/zstd/compress/../common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/compress/zstd_compress.o:lib/zstd/compress/../common/mem.h:179: more undefined references to `__bswapsi2' follow h8300-linux-ld: lib/zstd/compress/zstd_double_fast.o: in function `MEM_swap64': lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/zstd_double_fast.o:lib/zstd/compress/../common/mem.h:192: more undefined references to `__bswapdi2' follow h8300-linux-ld: lib/zstd/compress/zstd_opt.o: in function `MEM_swap32': lib/zstd/compress/../common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/compress/zstd_opt.o: in function `MEM_swap64': lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/../common/mem.h:192: undefined reference to `__bswapdi2' h8300-linux-ld: lib/zstd/compress/zstd_opt.o:lib/zstd/compress/../common/mem.h:192: more undefined references to `__bswapdi2' follow h8300-linux-ld: lib/zstd/decompress/huf_decompress.o: in function `MEM_swap32': lib/zstd/decompress/../common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/decompress/../common/mem.h:179: undefined reference to `__bswapsi2' h8300-linux-ld: lib/zstd/decompress/../common/mem.h:179: undefined
[PATCH v2 0/4] mtd: hyperbus: hbmc-am654: Add DMA support
This series add DMA support for reading data from HyperBus memory devices for TI's AM654/J721e SoCs With DMA there is ~5x improvement in read througput. v2: Fix DMAengine APIs usage issues pointed out by Peter. Vignesh Raghavendra (4): mtd: hyperbus: Provide per device private pointer mtd: hyperbus: hbmc-am654: Fix direct mapping setup flash access mtd: hyperbus: hbmc-am654: Drop pm_runtime* calls from probe mtd: hyperbus: hbmc-am654: Add DMA support for reads drivers/mtd/hyperbus/hbmc-am654.c | 144 ++ include/linux/mtd/hyperbus.h | 2 + 2 files changed, 130 insertions(+), 16 deletions(-) -- 2.28.0
[PATCH v2 3/4] mtd: hyperbus: hbmc-am654: Drop pm_runtime* calls from probe
Recent genpd changes for K3 platform ensure device is ON before driver
probe is called. Therefore, drop redundant pm_runtime_* calls from
driver to simplify the code.
Signed-off-by: Vignesh Raghavendra
---
drivers/mtd/hyperbus/hbmc-am654.c | 16 ++--
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/hyperbus/hbmc-am654.c
b/drivers/mtd/hyperbus/hbmc-am654.c
index 1e70ecfffa39..b6a2400fcaa9 100644
--- a/drivers/mtd/hyperbus/hbmc-am654.c
+++ b/drivers/mtd/hyperbus/hbmc-am654.c
@@ -13,7 +13,6 @@
#include
#include
#include
-#include
#include
#define AM654_HBMC_CALIB_COUNT 25
@@ -89,13 +88,6 @@ static int am654_hbmc_probe(struct platform_device *pdev)
priv->mux_ctrl = control;
}
- pm_runtime_enable(dev);
- ret = pm_runtime_get_sync(dev);
- if (ret < 0) {
- pm_runtime_put_noidle(dev);
- goto disable_pm;
- }
-
priv->hbdev.map.size = resource_size(&res);
priv->hbdev.map.virt = devm_ioremap_resource(dev, &res);
if (IS_ERR(priv->hbdev.map.virt))
@@ -107,13 +99,11 @@ static int am654_hbmc_probe(struct platform_device *pdev)
ret = hyperbus_register_device(&priv->hbdev);
if (ret) {
dev_err(dev, "failed to register controller\n");
- pm_runtime_put_sync(&pdev->dev);
- goto disable_pm;
+ goto disable_mux;
}
return 0;
-disable_pm:
- pm_runtime_disable(dev);
+disable_mux:
if (priv->mux_ctrl)
mux_control_deselect(priv->mux_ctrl);
return ret;
@@ -127,8 +117,6 @@ static int am654_hbmc_remove(struct platform_device *pdev)
ret = hyperbus_unregister_device(&priv->hbdev);
if (priv->mux_ctrl)
mux_control_deselect(priv->mux_ctrl);
- pm_runtime_put_sync(&pdev->dev);
- pm_runtime_disable(&pdev->dev);
return ret;
}
--
2.28.0
[PATCH v2 2/4] mtd: hyperbus: hbmc-am654: Fix direct mapping setup flash access
Setting up of direct mapping should be done with flash node's IO
address space and not with controller's IO region.
Fixes: b6fe8bc67d2d3 ("mtd: hyperbus: move direct mapping setup to AM654 HBMC
driver")
Signed-off-by: Vignesh Raghavendra
---
drivers/mtd/hyperbus/hbmc-am654.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/hyperbus/hbmc-am654.c
b/drivers/mtd/hyperbus/hbmc-am654.c
index e0e33f6bf513..1e70ecfffa39 100644
--- a/drivers/mtd/hyperbus/hbmc-am654.c
+++ b/drivers/mtd/hyperbus/hbmc-am654.c
@@ -70,7 +70,8 @@ static int am654_hbmc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, priv);
- ret = of_address_to_resource(np, 0, &res);
+ priv->hbdev.np = of_get_next_child(np, NULL);
+ ret = of_address_to_resource(priv->hbdev.np, 0, &res);
if (ret)
return ret;
@@ -103,7 +104,6 @@ static int am654_hbmc_probe(struct platform_device *pdev)
priv->ctlr.dev = dev;
priv->ctlr.ops = &am654_hbmc_ops;
priv->hbdev.ctlr = &priv->ctlr;
- priv->hbdev.np = of_get_next_child(dev->of_node, NULL);
ret = hyperbus_register_device(&priv->hbdev);
if (ret) {
dev_err(dev, "failed to register controller\n");
--
2.28.0
[PATCH v2 1/4] mtd: hyperbus: Provide per device private pointer
Provide per device private pointer that can be used by controller
drivers to store device specific private data.
Signed-off-by: Vignesh Raghavendra
---
include/linux/mtd/hyperbus.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/mtd/hyperbus.h b/include/linux/mtd/hyperbus.h
index 2129f7d3b6eb..d8cb1aec826d 100644
--- a/include/linux/mtd/hyperbus.h
+++ b/include/linux/mtd/hyperbus.h
@@ -20,6 +20,7 @@ enum hyperbus_memtype {
* @mtd: pointer to MTD struct
* @ctlr: pointer to HyperBus controller struct
* @memtype: type of memory device: HyperFlash or HyperRAM
+ * @priv: pointer to controller specific per device private data
*/
struct hyperbus_device {
@@ -28,6 +29,7 @@ struct hyperbus_device {
struct mtd_info *mtd;
struct hyperbus_ctlr *ctlr;
enum hyperbus_memtype memtype;
+ void *priv;
};
/**
--
2.28.0
