* Re: [PATCH 1/2] PCI: Introduce PCI_FIXUP_IOMMU
From: Christoph Hellwig @ 2020-05-26 14:46 UTC (permalink / raw)
To: Zhangfei Gao
Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
linux-pci, Greg Kroah-Hartman, Joerg Roedel, Hanjun Guo,
Rafael J. Wysocki, linux-kernel, iommu, linux-acpi, Wangzhou,
linux-crypto, Sudeep Holla, Bjorn Helgaas, kenneth-lee-2012,
linux-arm-kernel, Len Brown
In-Reply-To: <1590493749-13823-2-git-send-email-zhangfei.gao@linaro.org>
On Tue, May 26, 2020 at 07:49:08PM +0800, Zhangfei Gao wrote:
> Some platform devices appear as PCI but are actually on the AMBA bus,
> and they need fixup in drivers/pci/quirks.c handling iommu_fwnode.
> Here introducing PCI_FIXUP_IOMMU, which is called after iommu_fwnode
> is allocated, instead of reusing PCI_FIXUP_FINAL since it will slow
> down iommu probing as all devices in fixup final list will be
> reprocessed.
Who is going to use this? I don't see a single user in the series.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 5/6] mm: tlb: Provide flush_*_tlb_range wrappers
From: Catalin Marinas @ 2020-05-26 14:52 UTC (permalink / raw)
To: Zhenyu Ye
Cc: mark.rutland, peterz, linux-mm, guohanjun, will, linux-arch,
yuzhao, aneesh.kumar, steven.price, arm, Dave.Martin, arnd,
suzuki.poulose, npiggin, zhangshaokun, broonie, rostedt,
prime.zeng, kuhn.chenqun, tglx, linux-arm-kernel, xiexiangyou,
linux-kernel, maz, akpm
In-Reply-To: <ddba6d98-29b5-0748-ba74-ec022f509270@huawei.com>
On Mon, May 25, 2020 at 03:19:42PM +0800, Zhenyu Ye wrote:
> On 2020/5/22 23:42, Catalin Marinas wrote:
> > On Thu, Apr 23, 2020 at 09:56:55PM +0800, Zhenyu Ye wrote:
> >> diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
> >> index 3d7c01e76efc..3eff199d3507 100644
> >> --- a/mm/pgtable-generic.c
> >> +++ b/mm/pgtable-generic.c
> >> @@ -101,6 +101,28 @@ pte_t ptep_clear_flush(struct vm_area_struct *vma, unsigned long address,
> >>
> >> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> >>
> >> +#ifndef __HAVE_ARCH_FLUSH_PMD_TLB_RANGE
> >> +
> >> +#define FLUSH_Pxx_TLB_RANGE(_pxx) \
> >> +void flush_##_pxx##_tlb_range(struct vm_area_struct *vma, \
> >> + unsigned long addr, unsigned long end) \
> >> +{ \
> >> + struct mmu_gather tlb; \
> >> + \
> >> + tlb_gather_mmu(&tlb, vma->vm_mm, addr, end); \
> >> + tlb_start_vma(&tlb, vma); \
> >> + tlb_flush_##_pxx##_range(&tlb, addr, end - addr); \
> >> + tlb_end_vma(&tlb, vma); \
> >> + tlb_finish_mmu(&tlb, addr, end); \
> >> +}
> >
> > I may have confused myself (flush_p??_tlb_* vs. tlb_flush_p??_*) but do
> > actually need this whole tlb_gather thing here? IIUC (by grep'ing),
> > flush_p?d_tlb_range() is only called on huge pages, so we should know
> > the level already.
>
> tlb_flush_##_pxx##_range() is used to set tlb->cleared_*,
> flush_##_pxx##_tlb_range() will actually flush the TLB entry.
>
> In arch64, tlb_flush_p?d_range() is defined as:
>
> #define flush_pmd_tlb_range(vma, addr, end) flush_tlb_range(vma, addr, end)
> #define flush_pud_tlb_range(vma, addr, end) flush_tlb_range(vma, addr, end)
Currently, flush_p??_tlb_range() are generic and defined as above. I
think in the generic code they can remain an alias for
flush_tlb_range().
On arm64, we can redefine them as:
#define flush_pte_tlb_range(vma, addr, end) __flush_tlb_range(vma, addr, end, 3)
#define flush_pmd_tlb_range(vma, addr, end) __flush_tlb_range(vma, addr, end, 2)
#define flush_pud_tlb_range(vma, addr, end) __flush_tlb_range(vma, addr, end, 1)
#define flush_p4d_tlb_range(vma, addr, end) __flush_tlb_range(vma, addr, end, 0)
(unless the compiler optimises away all the mmu_gather stuff in your
macro above but they don't look trivial to me)
Also, I don't see the new flush_pte_* and flush_p4d_* macros used
anywhere and I don't think they are needed. The pte equivalent is
flush_tlb_page() (we need to make sure it's not used on a pmd in the
hugetlb context).
> So even if we know the level here, we can not pass the value to tlbi
> instructions (flush_tlb_range() is a common kernel interface and retro-fit it
> needs lots of changes), according to Peter's suggestion, I finally decide to
> pass the value of TTL by the tlb_gather_* frame.[1]
My comment was about the generic implementation using mmu_gather as you
are proposing. We don't need to change the flush_tlb_range() interface,
nor do we need to rewrite flush_p??_tlb_range().
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 6/6] arm64: tlb: Set the TTL field in flush_tlb_range
From: Catalin Marinas @ 2020-05-26 14:56 UTC (permalink / raw)
To: Zhenyu Ye
Cc: mark.rutland, peterz, linux-mm, guohanjun, will, linux-arch,
yuzhao, aneesh.kumar, steven.price, arm, Dave.Martin, arnd,
suzuki.poulose, npiggin, zhangshaokun, broonie, rostedt,
prime.zeng, kuhn.chenqun, tglx, linux-arm-kernel, xiexiangyou,
linux-kernel, maz, akpm
In-Reply-To: <20200423135656.2712-7-yezhenyu2@huawei.com>
On Thu, Apr 23, 2020 at 09:56:56PM +0800, Zhenyu Ye wrote:
> This patch uses the cleared_* in struct mmu_gather to set the
> TTL field in flush_tlb_range().
>
> Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
From: Catalin Marinas @ 2020-05-26 15:01 UTC (permalink / raw)
To: Anshuman Khandual
Cc: mark.rutland, Suzuki K Poulose, linux-kernel, Mark Brown,
Will Deacon, linux-arm-kernel
In-Reply-To: <1590500353-28082-1-git-send-email-anshuman.khandual@arm.com>
On Tue, May 26, 2020 at 07:09:13PM +0530, Anshuman Khandual wrote:
> @@ -632,8 +654,6 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
> const struct arm64_ftr_bits *ftrp;
> struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
>
> - BUG_ON(!reg);
> -
> for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
> u64 ftr_mask = arm64_ftr_mask(ftrp);
> s64 ftr_new = arm64_ftr_value(ftrp, new);
> @@ -762,7 +782,6 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
> {
> struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>
> - BUG_ON(!regp);
> update_cpu_ftr_reg(regp, val);
> if ((boot & regp->strict_mask) == (val & regp->strict_mask))
> return 0;
> @@ -776,9 +795,6 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)
> const struct arm64_ftr_bits *ftrp;
> struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>
> - if (WARN_ON(!regp))
> - return;
I think Will wanted an early return in all these functions not just
removing the BUG_ON(). I'll let him clarify.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/2] PCI: Introduce PCI_FIXUP_IOMMU
From: Zhangfei Gao @ 2020-05-26 15:09 UTC (permalink / raw)
To: Christoph Hellwig
Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
linux-pci, Greg Kroah-Hartman, Joerg Roedel, Hanjun Guo,
Rafael J. Wysocki, linux-kernel, iommu, linux-acpi, Wangzhou,
linux-crypto, Sudeep Holla, Bjorn Helgaas, kenneth-lee-2012,
linux-arm-kernel, Len Brown
In-Reply-To: <20200526144644.GA20784@infradead.org>
Hi, Christoph
On 2020/5/26 下午10:46, Christoph Hellwig wrote:
> On Tue, May 26, 2020 at 07:49:08PM +0800, Zhangfei Gao wrote:
>> Some platform devices appear as PCI but are actually on the AMBA bus,
>> and they need fixup in drivers/pci/quirks.c handling iommu_fwnode.
>> Here introducing PCI_FIXUP_IOMMU, which is called after iommu_fwnode
>> is allocated, instead of reusing PCI_FIXUP_FINAL since it will slow
>> down iommu probing as all devices in fixup final list will be
>> reprocessed.
> Who is going to use this? I don't see a single user in the series.
We will add iommu fixup in drivers/pci/quirks.c, handling
fwspec->can_stall, which is introduced in
https://www.spinics.net/lists/linux-pci/msg94559.html
Unfortunately, the patch does not catch v5.8, so we have to wait.
And we want to check whether this is a right method to solve this issue.
Thanks
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 03/14] PCI: cadence: Convert all r/w accessors to perform only 32-bit accesses
From: Rob Herring @ 2020-05-26 15:12 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: devicetree, Lorenzo Pieralisi, Arnd Bergmann, PCI,
linux-kernel@vger.kernel.org, Tom Joseph, Greg Kroah-Hartman,
Bjorn Helgaas, linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <df29309d-8401-4040-eb1e-90bb3af93a82@ti.com>
On Sun, May 24, 2020 at 9:30 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi Rob,
>
> On 5/22/2020 9:24 PM, Rob Herring wrote:
> > On Thu, May 21, 2020 at 9:37 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >>
> >> Certain platforms like TI's J721E using Cadence PCIe IP can perform only
> >> 32-bit accesses for reading or writing to Cadence registers. Convert all
> >> read and write accesses to 32-bit in Cadence PCIe driver in preparation
> >> for adding PCIe support in TI's J721E SoC.
> >
> > Looking more closely I don't think cdns_pcie_ep_assert_intx is okay
> > with this and never can be given the PCI_COMMAND and PCI_STATUS
> > registers are in the same word (IIRC, that's the main reason 32-bit
> > config space accesses are broken). So this isn't going to work at
>
> right, PCI_STATUS has write '1' to clear bits and there's a chance that it
> could be reset while raising legacy interrupt. While this cannot be avoided for
> TI's J721E, other platforms doesn't have to have this limitation.
> > least for EP accesses. And maybe you need a custom .raise_irq() hook
> > to minimize any problems (such as making the RMW atomic at least from
> > the endpoint's perspective).
>
> This is to make sure EP doesn't update in-consistent state when RC is updating
> the PCI_STATUS register? Since this involves two different systems, how do we
> make this atomic?
You can't make it atomic WRT both systems, but is there locking around
each RMW? Specifically, are preemption and interrupts disabled to
ensure time between a read and write are minimized? You wouldn't want
interrupts disabled during the delay too though (i.e. around
.raise_irq()).
BTW, I've asked this question before, but aren't PCI legacy interrupts
level triggered? If so, isn't generating a pulse wrong?
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC 2/3] cpufreq: governor: Use minimum load QoS
From: Benjamin Gaignard @ 2020-05-26 15:16 UTC (permalink / raw)
To: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
alexandre.torgue, pavel, len.brown, valentin.schneider,
vincent.guittot
Cc: Benjamin Gaignard, linux-pm, linux-kernel, linux-stm32,
linux-arm-kernel, linux-media
In-Reply-To: <20200526151619.8779-1-benjamin.gaignard@st.com>
Make sure that the returned load is above the system-wide minimum
load QoS.
Devices could set this specific QoS to inform governors about their
need in terms of CPU load when computing it from idle time isn't accurate.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
drivers/cpufreq/cpufreq_governor.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index f99ae45efaea..1494e5e4c788 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -118,6 +118,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
unsigned int ignore_nice = dbs_data->ignore_nice_load;
unsigned int max_load = 0, idle_periods = UINT_MAX;
unsigned int sampling_rate, io_busy, j;
+ unsigned int qos_min_load;
/*
* Sometimes governors may use an additional multiplier to increase
@@ -225,6 +226,10 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
policy_dbs->idle_periods = idle_periods;
+ qos_min_load = cpufreq_minload_qos_limit();
+ if (qos_min_load > max_load)
+ max_load = qos_min_load;
+
return max_load;
}
EXPORT_SYMBOL_GPL(dbs_update);
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
From: Benjamin Gaignard @ 2020-05-26 15:16 UTC (permalink / raw)
To: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
alexandre.torgue, pavel, len.brown, valentin.schneider,
vincent.guittot
Cc: Benjamin Gaignard, linux-pm, linux-kernel, linux-stm32,
linux-arm-kernel, linux-media
A first round [1] of discussions and suggestions have already be done on
this series but without found a solution to the problem. I resend it to
progress on this topic.
When start streaming from the sensor the CPU load could remain very low
because almost all the capture pipeline is done in hardware (i.e. without
using the CPU) and let believe to cpufreq governor that it could use lower
frequencies. If the governor decides to use a too low frequency that
becomes a problem when we need to acknowledge the interrupt during the
blanking time.
The delay to ack the interrupt and perform all the other actions before
the next frame is very short and doesn't allow to the cpufreq governor to
provide the required burst of power. That led to drop the half of the frames.
To avoid this problem, DCMI driver informs the cpufreq governors by adding
a cpufreq minimum load QoS resquest.
Benjamin
[1] https://lkml.org/lkml/2020/4/24/360
Benjamin Gaignard (3):
PM: QoS: Introduce cpufreq minimum load QoS
cpufreq: governor: Use minimum load QoS
media: stm32-dcmi: Inform cpufreq governors about cpu load needs
drivers/cpufreq/cpufreq_governor.c | 5 +
drivers/media/platform/stm32/stm32-dcmi.c | 8 ++
include/linux/pm_qos.h | 12 ++
kernel/power/qos.c | 213 ++++++++++++++++++++++++++++++
4 files changed, 238 insertions(+)
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC 3/3] media: stm32-dcmi: Inform cpufreq governors about cpu load needs
From: Benjamin Gaignard @ 2020-05-26 15:16 UTC (permalink / raw)
To: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
alexandre.torgue, pavel, len.brown, valentin.schneider,
vincent.guittot
Cc: Benjamin Gaignard, linux-pm, linux-kernel, linux-stm32,
linux-arm-kernel, linux-media
In-Reply-To: <20200526151619.8779-1-benjamin.gaignard@st.com>
When start streaming the CPU load could remain very low because almost
all the capture pipeline is done in hardware (i.e. without using the CPU)
and let believe to cpufreq governor that it could use lower frequencies.
If the governor decides to use a too low frequency that becomes a problem
when we need to acknowledge the interrupt during the blanking time.
To avoid this problem, DCMI driver informs the cpufreq governors by adding
a cpufreq minimum load QoS resquest.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
drivers/media/platform/stm32/stm32-dcmi.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index b8931490b83b..774f2506b2f1 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -24,6 +24,7 @@
#include <linux/of_graph.h>
#include <linux/pinctrl/consumer.h>
#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
#include <linux/pm_runtime.h>
#include <linux/reset.h>
#include <linux/videodev2.h>
@@ -173,6 +174,8 @@ struct stm32_dcmi {
struct media_device mdev;
struct media_pad vid_cap_pad;
struct media_pipeline pipeline;
+
+ struct pm_qos_request qos_request;
};
static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n)
@@ -827,6 +830,9 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
else
reg_set(dcmi->regs, DCMI_IER, IT_OVR | IT_ERR);
+ cpufreq_minload_qos_add_request(&dcmi->qos_request,
+ CPUFREQ_GOV_QOS_MIN_LOAD_MAX_VALUE);
+
return 0;
err_pipeline_stop:
@@ -859,6 +865,8 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
struct dcmi_buf *buf, *node;
+ cpufreq_minload_qos_remove_request(&dcmi->qos_request);
+
dcmi_pipeline_stop(dcmi);
media_pipeline_stop(&dcmi->vdev->entity);
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RFC 1/3] PM: QoS: Introduce cpufreq minimum load QoS
From: Benjamin Gaignard @ 2020-05-26 15:16 UTC (permalink / raw)
To: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
alexandre.torgue, pavel, len.brown, valentin.schneider,
vincent.guittot
Cc: Benjamin Gaignard, linux-pm, linux-kernel, linux-stm32,
linux-arm-kernel, linux-media
In-Reply-To: <20200526151619.8779-1-benjamin.gaignard@st.com>
Introduce cpufreq minimum load QoS, based on the "raw" low-level
PM QoS, to represent the minimum expected cpu load by various devices.
The cpufreq_minload_qos_limit() helper is defined to retrieve the
aggregated constraints.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
include/linux/pm_qos.h | 12 +++
kernel/power/qos.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 225 insertions(+)
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 4a69d4af3ff8..e2cc099322e3 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -316,4 +316,16 @@ int freq_qos_remove_notifier(struct freq_constraints *qos,
enum freq_qos_req_type type,
struct notifier_block *notifier);
+/* Definitions related to the cpufreq minimum load QoS. */
+
+#define CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE 0
+#define CPUFREQ_GOV_QOS_MIN_LOAD_MAX_VALUE 10
+
+s32 cpufreq_minload_qos_limit(void);
+bool cpufreq_minload_qos_request_active(struct pm_qos_request *req);
+void cpufreq_minload_qos_add_request(struct pm_qos_request *req, s32 value);
+void cpufreq_minload_qos_update_request(struct pm_qos_request *req,
+ s32 new_value);
+void cpufreq_minload_qos_remove_request(struct pm_qos_request *req);
+
#endif
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index db0bed2cae26..df2fdd962f35 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -671,3 +671,216 @@ int freq_qos_remove_notifier(struct freq_constraints *qos,
return ret;
}
EXPORT_SYMBOL_GPL(freq_qos_remove_notifier);
+
+/* Definitions related to the cpufreq minimum load QoS. */
+
+static struct pm_qos_constraints cpufreq_minload_constraints = {
+ .list = PLIST_HEAD_INIT(cpufreq_minload_constraints.list),
+ .target_value = CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE,
+ .default_value = CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE,
+ .no_constraint_value = CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE,
+ .type = PM_QOS_MAX,
+};
+
+/**
+ * cpufreq_minload_qos_limit - Return current system-wide cpufreq
+ * minimum load QoS limit.
+ */
+s32 cpufreq_minload_qos_limit(void)
+{
+ return pm_qos_read_value(&cpufreq_minload_constraints);
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_limit);
+
+/**
+ * cpufreq_minload_qos_request_active - Check the given PM QoS request.
+ * @req: PM QoS request to check.
+ *
+ * Return: 'true' if @req has been added to the cpufreq minimum load
+ * QoS list, 'false' otherwise.
+ */
+bool cpufreq_minload_qos_request_active(struct pm_qos_request *req)
+{
+ return req->qos == &cpufreq_minload_constraints;
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_request_active);
+
+/**
+ * cpufreq_minload_qos_add_request - Add new cpufreq minimum load QoS request.
+ * @req: Pointer to a preallocated handle.
+ * @value: Requested constraint value.
+ *
+ * Use @value to initialize the request handle pointed to by @req, insert it as
+ * a new entry to the cpufreq minimum load QoS list and recompute the effective
+ * QoS constraint for that list.
+ *
+ * Callers need to save the handle for later use in updates and removal of the
+ * QoS request represented by it.
+ */
+void cpufreq_minload_qos_add_request(struct pm_qos_request *req, s32 value)
+{
+ if (!req)
+ return;
+
+ if (cpufreq_minload_qos_request_active(req)) {
+ WARN(1, KERN_ERR "%s called for already added request\n", __func__);
+ return;
+ }
+
+ trace_pm_qos_add_request(value);
+
+ req->qos = &cpufreq_minload_constraints;
+ pm_qos_update_target(req->qos, &req->node, PM_QOS_ADD_REQ, value);
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_add_request);
+
+/**
+ * cpufreq_minload_qos_update_request - Modify existing cpufreq minimum load
+ * QoS request.
+ * @req : QoS request to update.
+ * @new_value: New requested constraint value.
+ *
+ * Use @new_value to update the QoS request represented by @req in the cpufreq
+ * minimum load QoS list along with updating the effective constraint value for
+ * that list.
+ */
+void cpufreq_minload_qos_update_request(struct pm_qos_request *req,
+ s32 new_value)
+{
+ if (!req)
+ return;
+
+ if (!cpufreq_minload_qos_request_active(req)) {
+ WARN(1, KERN_ERR "%s called for unknown object\n", __func__);
+ return;
+ }
+
+ trace_pm_qos_update_request(new_value);
+
+ if (new_value == req->node.prio)
+ return;
+
+ pm_qos_update_target(req->qos, &req->node, PM_QOS_UPDATE_REQ, new_value);
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_update_request);
+
+/**
+ * cpufreq_minload_qos_remove_request - Remove existing cpufreq minimum load QoS
+ * request.
+ * @req: QoS request to remove.
+ *
+ * Remove the cpufreq minimum load QoS request represented by @req from the
+ * cpufreq minimum load QoS list along with updating the effective constraint
+ * value for that list.
+ */
+void cpufreq_minload_qos_remove_request(struct pm_qos_request *req)
+{
+ if (!req)
+ return;
+
+ if (!cpufreq_minload_qos_request_active(req)) {
+ WARN(1, KERN_ERR "%s called for unknown object\n", __func__);
+ return;
+ }
+
+ trace_pm_qos_remove_request(PM_QOS_DEFAULT_VALUE);
+
+ pm_qos_update_target(req->qos, &req->node,
+ PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
+ memset(req, 0, sizeof(*req));
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_remove_request);
+
+/* User space interface to the cpufreq minimum load QoS via misc device. */
+
+static int cpufreq_minload_qos_open(struct inode *inode, struct file *filp)
+{
+ struct pm_qos_request *req;
+
+ req = kzalloc(sizeof(*req), GFP_KERNEL);
+ if (!req)
+ return -ENOMEM;
+
+ cpufreq_minload_qos_add_request(req, PM_QOS_DEFAULT_VALUE);
+ filp->private_data = req;
+
+ return 0;
+}
+
+static int cpufreq_minload_qos_release(struct inode *inode, struct file *filp)
+{
+ struct pm_qos_request *req = filp->private_data;
+
+ filp->private_data = NULL;
+
+ cpufreq_minload_qos_remove_request(req);
+ kfree(req);
+
+ return 0;
+}
+
+static ssize_t cpufreq_minload_qos_read(struct file *filp, char __user *buf,
+ size_t count, loff_t *f_pos)
+{
+ struct pm_qos_request *req = filp->private_data;
+ unsigned long flags;
+ s32 value;
+
+ if (!req || !cpufreq_minload_qos_request_active(req))
+ return -EINVAL;
+
+ spin_lock_irqsave(&pm_qos_lock, flags);
+ value = pm_qos_get_value(&cpufreq_minload_constraints);
+ spin_unlock_irqrestore(&pm_qos_lock, flags);
+
+ return simple_read_from_buffer(buf, count, f_pos, &value, sizeof(s32));
+}
+
+static ssize_t cpufreq_minload_qos_write(struct file *filp,
+ const char __user *buf,
+ size_t count, loff_t *f_pos)
+{
+ s32 value;
+
+ if (count == sizeof(s32)) {
+ if (copy_from_user(&value, buf, sizeof(s32)))
+ return -EFAULT;
+ } else {
+ int ret;
+
+ ret = kstrtos32_from_user(buf, count, 16, &value);
+ if (ret)
+ return ret;
+ }
+
+ cpufreq_minload_qos_update_request(filp->private_data, value);
+
+ return count;
+}
+
+static const struct file_operations cpufreq_minload_qos_fops = {
+ .write = cpufreq_minload_qos_write,
+ .read = cpufreq_minload_qos_read,
+ .open = cpufreq_minload_qos_open,
+ .release = cpufreq_minload_qos_release,
+ .llseek = noop_llseek,
+};
+
+static struct miscdevice cpufreq_minload_qos_miscdev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "cpufreq_minimum_load",
+ .fops = &cpufreq_minload_qos_fops,
+};
+
+static int __init cpufreq_minload_qos_init(void)
+{
+ int ret;
+
+ ret = misc_register(&cpufreq_minload_qos_miscdev);
+ if (ret < 0)
+ pr_err("%s: %s setup failed\n", __func__,
+ cpufreq_minload_qos_miscdev.name);
+
+ return ret;
+}
+late_initcall(cpufreq_minload_qos_init);
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [soc:mediatek/dt] BUILD REGRESSION 189881af810d452b592ee958db43eb4c57df9803
From: Matthias Brugger @ 2020-05-26 15:21 UTC (permalink / raw)
To: Rong Chen, kbuild test robot, arm; +Cc: linux-arm-kernel
In-Reply-To: <2ea1b666-1052-d403-f477-59ee6e1cf9ab@intel.com>
Hi Rong,
On 26/05/2020 11:18, Rong Chen wrote:
> Hi Matthias,
>
> On 5/23/20 4:54 AM, Matthias Brugger wrote:
>> Hi all,
>> Hi Arnd and Olof,
>>
>> On 22/05/2020 13:28, kbuild test robot wrote:
>>> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git
>>> mediatek/dt
>>> branch HEAD: 189881af810d452b592ee958db43eb4c57df9803 arm: dts: mt2701: Add
>>> usb2 device nodes
>>>
>>> Error/Warning in current branch:
>>>
>>> ERROR: Input tree has errors, aborting (use -f to force output)
>>>
>>> Error/Warning ids grouped by kconfigs:
>>>
>>> recent_errors
>>> `-- arm-randconfig-r035-20200520
>>> `-- ERROR:Input-tree-has-errors-aborting-(use-f-to-force-output)
>> Can you please explain me how to reproduce this. I'm not able to deduce that
>> form this email. I can then look into this and how to fix it. Although up to now
>> I don't understand where the relation with my dts32 changes is.
>
> Sorry for the inconvenience, the error was reported in
> "[mediatek:v5.7-next/dts32 2/5] ERROR: Input tree has errors, aborting (use -f
> to force output)",
> the head commit 189881af810d452b592ee958db43eb4c57df9803 of soc:mediatek/dt
> branch is the same with mediatek:v5.7-next/dts32 too, so the bot
> printed the error here wrongly. BTW, recent_errors doesn't mean the errors in
> current branch, it only means recent found errors.
>
Thanks for your explanation. Wouldn't it make sense to re-run a broken test with
-f to force some output?
Anyway I suppose the error is due to the fact that the binding description for
"mediatek,mt7623-mali" is added in commit 3, so in commit 2 the DTS checks fail.
Arnd, Olof, I think the error isn't worth a resubmission. I you think it is, let
me know and I'll provide a new pull request.
Regards,
Matthias
> Best Regards,
> Rong Chen
>
>>
>> Regards,
>> Matthias
>>
>>> elapsed time: 7969m
>>>
>>> configs tested: 274
>>> configs skipped: 59
>>>
>>> arm defconfig
>>> arm allyesconfig
>>> arm allmodconfig
>>> arm allnoconfig
>>> arm64 allyesconfig
>>> arm64 defconfig
>>> arm64 allmodconfig
>>> arm64 allnoconfig
>>> sparc allyesconfig
>>> mips allyesconfig
>>> m68k allyesconfig
>>> sh sh03_defconfig
>>> arm vf610m4_defconfig
>>> h8300 defconfig
>>> m68k hp300_defconfig
>>> powerpc pq2fads_defconfig
>>> parisc alldefconfig
>>> sh sh7770_generic_defconfig
>>> arm mvebu_v7_defconfig
>>> arm u300_defconfig
>>> mips cavium_octeon_defconfig
>>> microblaze defconfig
>>> powerpc64 alldefconfig
>>> arm integrator_defconfig
>>> arc nsimosci_hs_defconfig
>>> powerpc mpc5200_defconfig
>>> arm clps711x_defconfig
>>> mips xway_defconfig
>>> mips maltaup_defconfig
>>> arm hisi_defconfig
>>> um defconfig
>>> arm davinci_all_defconfig
>>> sh r7780mp_defconfig
>>> arm sunxi_defconfig
>>> arm mmp2_defconfig
>>> mips loongson1c_defconfig
>>> arm ep93xx_defconfig
>>> arc nsimosci_defconfig
>>> um allnoconfig
>>> sh secureedge5410_defconfig
>>> sh apsh4ad0a_defconfig
>>> powerpc allmodconfig
>>> s390 debug_defconfig
>>> sh ap325rxa_defconfig
>>> c6x defconfig
>>> h8300 h8300h-sim_defconfig
>>> arc axs103_smp_defconfig
>>> mips qi_lb60_defconfig
>>> ia64 bigsur_defconfig
>>> openrisc or1ksim_defconfig
>>> sh lboxre2_defconfig
>>> powerpc chrp32_defconfig
>>> sh polaris_defconfig
>>> c6x evmc6457_defconfig
>>> arm corgi_defconfig
>>> mips cu1000-neo_defconfig
>>> arm ixp4xx_defconfig
>>> arm mps2_defconfig
>>> sh microdev_defconfig
>>> mips allnoconfig
>>> arm exynos_defconfig
>>> mips db1xxx_defconfig
>>> arm64 alldefconfig
>>> sh sh2007_defconfig
>>> arm bcm2835_defconfig
>>> sparc sparc32_defconfig
>>> ia64 alldefconfig
>>> sh ecovec24_defconfig
>>> arm shannon_defconfig
>>> riscv nommu_k210_defconfig
>>> powerpc ppc64e_defconfig
>>> parisc generic-32bit_defconfig
>>> arc alldefconfig
>>> sh sh7710voipgw_defconfig
>>> arm pleb_defconfig
>>> h8300 alldefconfig
>>> microblaze mmu_defconfig
>>> powerpc ppc44x_defconfig
>>> mips tb0219_defconfig
>>> parisc defconfig
>>> s390 allyesconfig
>>> m68k m5307c3_defconfig
>>> mips mtx1_defconfig
>>> arm mv78xx0_defconfig
>>> arm oxnas_v6_defconfig
>>> arc vdk_hs38_defconfig
>>> sh se7751_defconfig
>>> arm netwinder_defconfig
>>> arm iop32x_defconfig
>>> mips ath25_defconfig
>>> arm pxa_defconfig
>>> arm mini2440_defconfig
>>> alpha defconfig
>>> mips malta_defconfig
>>> arm assabet_defconfig
>>> m68k defconfig
>>> arm h3600_defconfig
>>> ia64 allnoconfig
>>> mips loongson3_defconfig
>>> mips malta_kvm_guest_defconfig
>>> arc hsdk_defconfig
>>> arc tb10x_defconfig
>>> powerpc allnoconfig
>>> sh sdk7786_defconfig
>>> arm u8500_defconfig
>>> sparc64 allyesconfig
>>> mips lasat_defconfig
>>> mips tb0287_defconfig
>>> powerpc mgcoge_defconfig
>>> um x86_64_defconfig
>>> powerpc storcenter_defconfig
>>> sh shmin_defconfig
>>> powerpc ps3_defconfig
>>> sparc64 defconfig
>>> arm keystone_defconfig
>>> m68k m5272c3_defconfig
>>> parisc allyesconfig
>>> mips decstation_defconfig
>>> mips lemote2f_defconfig
>>> powerpc tqm8xx_defconfig
>>> arm shmobile_defconfig
>>> arm stm32_defconfig
>>> powerpc holly_defconfig
>>> mips tb0226_defconfig
>>> mips rt305x_defconfig
>>> sh se7750_defconfig
>>> arm s3c6400_defconfig
>>> arm pcm027_defconfig
>>> powerpc ppc64_defconfig
>>> h8300 h8s-sim_defconfig
>>> m68k m5208evb_defconfig
>>> sh se7721_defconfig
>>> mips sb1250_swarm_defconfig
>>> powerpc amigaone_defconfig
>>> parisc generic-64bit_defconfig
>>> c6x dsk6455_defconfig
>>> arm multi_v5_defconfig
>>> xtensa alldefconfig
>>> arm spear13xx_defconfig
>>> sh rts7751r2dplus_defconfig
>>> mips nlm_xlr_defconfig
>>> sh migor_defconfig
>>> powerpc mpc512x_defconfig
>>> nds32 defconfig
>>> arm moxart_defconfig
>>> um allyesconfig
>>> mips rb532_defconfig
>>> arm imote2_defconfig
>>> i386 allnoconfig
>>> i386 allyesconfig
>>> i386 defconfig
>>> i386 debian-10.3
>>> ia64 allmodconfig
>>> ia64 defconfig
>>> ia64 allyesconfig
>>> m68k allmodconfig
>>> m68k allnoconfig
>>> m68k sun3_defconfig
>>> nios2 defconfig
>>> nios2 allyesconfig
>>> openrisc defconfig
>>> c6x allyesconfig
>>> c6x allnoconfig
>>> openrisc allyesconfig
>>> nds32 allnoconfig
>>> csky allyesconfig
>>> csky defconfig
>>> alpha allyesconfig
>>> xtensa allyesconfig
>>> h8300 allyesconfig
>>> h8300 allmodconfig
>>> xtensa defconfig
>>> arc defconfig
>>> arc allyesconfig
>>> sh allmodconfig
>>> sh allnoconfig
>>> microblaze allnoconfig
>>> mips allmodconfig
>>> parisc allnoconfig
>>> parisc allmodconfig
>>> powerpc allyesconfig
>>> powerpc rhel-kconfig
>>> powerpc defconfig
>>> x86_64 randconfig-a005-20200517
>>> x86_64 randconfig-a003-20200517
>>> x86_64 randconfig-a006-20200517
>>> x86_64 randconfig-a004-20200517
>>> x86_64 randconfig-a001-20200517
>>> x86_64 randconfig-a002-20200517
>>> i386 randconfig-a006-20200518
>>> i386 randconfig-a005-20200518
>>> i386 randconfig-a001-20200518
>>> i386 randconfig-a003-20200518
>>> i386 randconfig-a004-20200518
>>> i386 randconfig-a002-20200518
>>> i386 randconfig-a006-20200519
>>> i386 randconfig-a005-20200519
>>> i386 randconfig-a001-20200519
>>> i386 randconfig-a003-20200519
>>> i386 randconfig-a004-20200519
>>> i386 randconfig-a002-20200519
>>> i386 randconfig-a006-20200517
>>> i386 randconfig-a005-20200517
>>> i386 randconfig-a003-20200517
>>> i386 randconfig-a001-20200517
>>> i386 randconfig-a004-20200517
>>> i386 randconfig-a002-20200517
>>> i386 randconfig-a001-20200520
>>> i386 randconfig-a004-20200520
>>> i386 randconfig-a006-20200520
>>> i386 randconfig-a003-20200520
>>> i386 randconfig-a002-20200520
>>> i386 randconfig-a005-20200520
>>> x86_64 randconfig-a003-20200519
>>> x86_64 randconfig-a005-20200519
>>> x86_64 randconfig-a004-20200519
>>> x86_64 randconfig-a006-20200519
>>> x86_64 randconfig-a002-20200519
>>> x86_64 randconfig-a001-20200519
>>> x86_64 randconfig-a016-20200518
>>> x86_64 randconfig-a012-20200518
>>> x86_64 randconfig-a015-20200518
>>> x86_64 randconfig-a013-20200518
>>> x86_64 randconfig-a011-20200518
>>> x86_64 randconfig-a014-20200518
>>> x86_64 randconfig-a013-20200520
>>> x86_64 randconfig-a015-20200520
>>> x86_64 randconfig-a016-20200520
>>> x86_64 randconfig-a012-20200520
>>> x86_64 randconfig-a014-20200520
>>> x86_64 randconfig-a011-20200520
>>> i386 randconfig-a012-20200518
>>> i386 randconfig-a014-20200518
>>> i386 randconfig-a016-20200518
>>> i386 randconfig-a011-20200518
>>> i386 randconfig-a015-20200518
>>> i386 randconfig-a013-20200518
>>> i386 randconfig-a012-20200517
>>> i386 randconfig-a016-20200517
>>> i386 randconfig-a014-20200517
>>> i386 randconfig-a011-20200517
>>> i386 randconfig-a013-20200517
>>> i386 randconfig-a015-20200517
>>> i386 randconfig-a012-20200519
>>> i386 randconfig-a014-20200519
>>> i386 randconfig-a016-20200519
>>> i386 randconfig-a011-20200519
>>> i386 randconfig-a015-20200519
>>> i386 randconfig-a013-20200519
>>> i386 randconfig-a013-20200520
>>> i386 randconfig-a012-20200520
>>> i386 randconfig-a015-20200520
>>> i386 randconfig-a011-20200520
>>> i386 randconfig-a016-20200520
>>> i386 randconfig-a014-20200520
>>> riscv allyesconfig
>>> riscv allnoconfig
>>> riscv defconfig
>>> riscv allmodconfig
>>> s390 allnoconfig
>>> s390 allmodconfig
>>> s390 defconfig
>>> x86_64 defconfig
>>> sparc defconfig
>>> sparc64 allnoconfig
>>> sparc64 allmodconfig
>>> um allmodconfig
>>> x86_64 rhel-7.6-kselftests
>>> x86_64 rhel
>>> x86_64 rhel-7.6
>>> x86_64 rhel-7.2-clear
>>> x86_64 lkp
>>> x86_64 fedora-25
>>> x86_64 kexec
>>>
>>> ---
>>> 0-DAY CI Kernel Test Service, Intel Corporation
>>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: sram: add documentation for reserved-only flag
From: Mian Yousaf Kaukab @ 2020-05-26 15:28 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, arnd, devicetree, gregkh, Stephen Warren,
linux-kernel, jonathanh, talho, linux-tegra, robin.murphy,
afaerber, linux-arm-kernel
In-Reply-To: <20200520085558.GB2136208@ulmo>
On Wed, May 20, 2020 at 10:55:58AM +0200, Thierry Reding wrote:
> On Tue, May 19, 2020 at 05:03:26PM -0600, Rob Herring wrote:
> > On Tue, May 19, 2020 at 10:16:43AM -0600, Stephen Warren wrote:
> > > On 5/13/20 4:41 AM, Mian Yousaf Kaukab wrote:
> > > > On Tue, May 12, 2020 at 01:45:28PM -0600, Stephen Warren wrote:
> > > >> On 5/12/20 8:48 AM, Mian Yousaf Kaukab wrote:
> > > >>> Add documentation for the new optional flag added for SRAM driver.
> > > >>
> > > >>> diff --git a/Documentation/devicetree/bindings/sram/sram.yaml b/Documentation/devicetree/bindings/sram/sram.yaml
> > > >>
> > > >>> + reserved-only:
> > > >>> + description:
> > > >>> + The flag indicating, that only SRAM reserved regions have to be remapped.
> > > >>> + remapping type is selected depending upon no-memory-wc as usual.
> > > >>> + type: boolean
> > > >>
> > > >> This feels a bit like a SW flag rather than a HW description, so I'm not
> > > >> sure it's appropriate to put it into DT.
> > > >
> > > > Reserved regions themselves are software descriptions, no? Then we have 'pool'
> > > > flag which is again a software flag and so on. This flag falls into same
> > > > category and nothing out of ordinary.
> > >
> > > I suppose that's true to some extent. This is indeed a description of
> > > the system environment presented to the SW that consumes the DT, which
> > > is a bit more than pure HW description but still a description of
> > > something imposed externally rather than describing something that's up
> > > to the discretion of the consuming SW. So, go ahead.
> > >
> > > >> Are there any cases where the SW should map all of the SRAM, i.e. where
> > > >> we wouldn't expect to set reserved-only? [...]
> > > >
> > > > Yes, here are a few examples:
> > > > arch/arm/boot/dts/aspeed-g*.dtsi
> > > > arch/arm/boot/dts/at91*.dtsi
> > > > arch/arm/boot/dts/bcm7445.dtsi
> > > > Then arch/arm/boot/dts/dra7.dtsi is an example where we should map everything
> > > > except the reserved region.
> > > >
> > > >> [...] I'd expect reserved-only to be
> > > >> the default, and perhaps only, mode of operation for the SRAM driver.
> > > >
> > > > It will break compatibility with existing dtbs.
> > > >
> > > >> If we can't do that because some SW currently expects to be able to map
> > > >> arbitrary portions of the SRAM, shouldn't that SW be fixed to tell the
> > > >> SRAM driver which parts it's using, hence still allowing the driver to
> > > >> only map in-use portions?
> > > >
> > > > User doesn’t need sram driver in that case. It can use genalloc api directly.
> > >
> > > This sounds a bit odd. Without a driver for the reserved region, nothing
> > > should be touching it, since otherwise there's no code that owns an
> > > manages the region. If any code needs to consume the region, it should
> > > obtain info about the region from some form of provider code that can
> > > handle both the allocation and mapping. Anything else sounds like some
> > > consumer code directly making use of DT nodes it doesn't own. But since
> > > I'm not familiar enough with the SRAM driver and genalloc code that you
> > > mention to fully understand the allocation paths I guess I won't object
> > > for now, although it does still sound fishy.
> >
> > I'm fine with the concept, but I don't think a single flag is adequate.
> > If there are reserved regions within the SRAM, then define child nodes
> > to mark those regions reserved. I don't think you need a new flag. Just
> > a 'reg' property and nothing else.
>
> It sounds to me like there are two different interpretations of SRAM and
> reserved regions. On one hand, as you suggest, we have one SRAM that's
> made available as genalloc pool and then individual regions can be
> marked as reserved so that they aren't added to that pool.
>
> At the same time, each reserved region is also exposed as a separate
> pool and that's in fact used by many consumers as a way of getting a
> specific chunk of the SRAM for their own use (via phandle to the region
> from the consumer's device tree node).
>
> In addition to that, the reserved region code doesn't actually fully do
> its job because while the reserved region isn't actually added to the
> "top-level" SRAM pool, the memory is still mapped. At the same time this
> is something that we actually want because, like I mentioned, some of
> the consumers do want to get at their SRAM chunks via references to the
> partitions.
>
> The problem that this patch series is really trying to solve is another
> still: the complete SRAM is always mapped to kernel memory, irrespective
> of whether any regions are marked reserved or not and that can cause
> speculative accesses to memory outside of the defined regions.
>
> Stephen's suggestion is to default to only mapping memory for which a
> partition has been defined in the SRAM and assuming that all SRAM
> outside of those partitions is off limits. I think that's a sensible
> default and it's unambiguous.
>
> But as Yousaf points out that would break compatibility with existing
> device trees. Depending on how you interpret the bindings one could
> argue that those device trees are buggy and should have partitions
> defined (in the cases I've looked at they end up using a fixed region
> anyway, so that could've just been made explicit in the device tree).
>
> However, it also looks like all of the users that rely on the original
> behaviour where they can just access the full pool are those that don't
> define any reserved regions, whereas all users that do reserve regions
> will actually use those reserved regions.
>
> So I think we can make use of this by differentiating in the driver
> between SRAM nodes with or without children and change the behaviour
> accordingly. I think that has the big advantage that it makes things
> work as (I think) most people would expect and doesn't further
> complicate the binding with extra flags.
I tend to agree on mapping partitions only if they exist. So far I could
only find one exception. It is arch/arm/boot/dts/armada-370.dtsi which is
using the top level pool as well as a partition to reserve 32 bytes at the
bottom of sram. This can be fixed along with the sram driver change, by
adding another partition for the rest of the sram and using its handle in
the crypto@90000 instead of top-level sram node handle. Do you see anymore
exceptions where both top level pool and the partitions both are being used?
Then on the backward compatibility topic, another issue is that boot code
could add sram nodes dynamically. For example arch/arm/mach-k3/common.c in
u-boot does it. This particular case will not break after the suggested change
because it is not adding any partitions. However, there could be other
boot-loaders which are not this lucky.
>
> Thierry
/Yousaf
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 3/3] PM / AVS: SVS: Introduce SVS engine
From: Matthias Brugger @ 2020-05-26 15:29 UTC (permalink / raw)
To: Roger Lu
Cc: Mark Rutland, Nicolas Boichat, Nishanth Menon, Kevin Hilman,
Enric Balletbo Serra, Linux PM list, Angus Lin, Xiaoqing Liu,
linux-kernel, Stephen Boyd, devicetree@vger.kernel.org,
Rob Herring, moderated list:ARM/Mediatek SoC support, HenryC Chen,
Charles Yang, YT Lee, Fan Chen, Linux ARM
In-Reply-To: <1590484328.4392.44.camel@mtksdaap41>
On 26/05/2020 11:12, Roger Lu wrote:
> Hi Matthias,
>
> Thanks for the feedback.
>
> On Fri, 2020-05-22 at 17:38 +0200, Matthias Brugger wrote:
>>
>> On 22/05/2020 11:40, Roger Lu wrote:
>>>
>>> Hi Enric,
>>>
>>> On Tue, 2020-05-19 at 17:30 +0200, Enric Balletbo Serra wrote:
>>>> Hi Roger,
>>>>
>>>> Thank you for your patch. I have the feeling that this driver is
>>>> complex and difficult to follow and I am wondering if it wouldn't be
>>>> better if you can send a version that simply adds basic functionality
>>>> for now. Some comments below.
>>>
>>> Thanks for the advices. I'll submit SVS v9 with basic functionality
>>> patch + step by step functionalities' patches.
>>>
>>>>
>>>> Missatge de Roger Lu <roger.lu@mediatek.com> del dia dl., 18 de maig
>>>> 2020 a les 11:25:
>>>>>
>>>>> The SVS (Smart Voltage Scaling) engine is a piece
>>>>> of hardware which is used to calculate optimized
>>>>> voltage values of several power domains,
>>>>> e.g. CPU/GPU/CCI, according to chip process corner,
>>>>> temperatures, and other factors. Then DVFS driver
>>>>> could apply those optimized voltage values to reduce
>>>>> power consumption.
>>>>>
>>>>> Signed-off-by: Roger Lu <roger.lu@mediatek.com>
>>>>> ---
>>>>> drivers/power/avs/Kconfig | 10 +
>>>>> drivers/power/avs/Makefile | 1 +
>>>>> drivers/power/avs/mtk_svs.c | 2119 +++++++++++++++++++++++++++++++++
>>>>> include/linux/power/mtk_svs.h | 23 +
>>>>> 4 files changed, 2153 insertions(+)
>>>>> create mode 100644 drivers/power/avs/mtk_svs.c
>>>>> create mode 100644 include/linux/power/mtk_svs.h
>>>>>
>>>>> diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
>>>>> index cdb4237bfd02..67089ac6040e 100644
>>>>> --- a/drivers/power/avs/Kconfig
>>>>> +++ b/drivers/power/avs/Kconfig
>>>>> @@ -35,3 +35,13 @@ config ROCKCHIP_IODOMAIN
>>>>> Say y here to enable support io domains on Rockchip SoCs. It is
>>>>> necessary for the io domain setting of the SoC to match the
>>>>> voltage supplied by the regulators.
>>>>> +
>>>>> +config MTK_SVS
>>>>> + bool "MediaTek Smart Voltage Scaling(SVS)"
>>>>
>>>> Can't be this a module? Why? In such case, you should use tristate option
>>>
>>> Generally, MTK_SVS is needed in MTK SoC(mt81xx) products. So, we don't provide
>>> module option in config. If, somehow, SVS isn't needed, we suggest
>>> CONFIG_MTK_SVS=n to be set.
>>>
>>
>> The question here is if it needs to be probed before we probe the modules. If
>> not, we should add a Kconfig option for MT81xx SoCs to select MTK_SVS.
>
> Excuse me to make you confuse. MT81xx SoCs is the subset MTK ICs that
> will use CONFIG_MTK_SVS. In other words, CONFIG_MTK_SVS will be used
> with other MTK ICs as well. So, MTK_SVS is the general naming for MTK IC
> to enable SVS power feature. Anyway, back to Enric's question, I'll make
> MTK_SVS become a tristate feature in the next patch. Thanks.
>
>>
[...]
>>>>> +
>>>>> +static const u32 svs_regs_v2[] = {
>>>>
>>>> Is this SoC specific or shared between SoCs?
>>>
>>> Shared between SoCs. Some SVS in MTK SoCs use v2 register map.
>>>
>>
>> And which silicon uses v1 then? Is v2 a MediaTek internal naming you want to keep?
>
> 1. MT8173 IC uses v1 register map.
> 2. Yes, I'll keep v2 postfix.
>
Sounds good, thanks for clarification.
Regards,
Matthias
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1] clk: mediatek: assign the initial value to clk_init_data of mtk_mux
From: Matthias Brugger @ 2020-05-26 15:42 UTC (permalink / raw)
To: Weiyi Lu
Cc: James Liao, Nicolas Boichat, srv_heupstream, Stephen Boyd,
linux-kernel, stable, Fan Chen, linux-mediatek, Owen Chen,
linux-clk, linux-arm-kernel
In-Reply-To: <1590460982.28324.17.camel@mtksdaap41>
Hi Weiyi,
On 26/05/2020 04:43, Weiyi Lu wrote:
> On Mon, 2020-05-25 at 11:08 +0200, Matthias Brugger wrote:
>>
>> On 25/05/2020 08:41, Weiyi Lu wrote:
>>> It'd be dangerous when struct clk_core have new memebers.
>>> Add the missing initial value to clk_init_data.
>>>
>>
>> Sorry I don't really understand this commit message, can please explain.
>> In any case if this is a problem, then we probably we should fix it for all drivers.
>> Apart from drivers/clk/mediatek/clk-cpumux.c
>>
>
> Actually, we were looking into an android kernel patch "ANDROID: GKI:
> clk: Add support for voltage voting" [1]
>
> In this patch, there adds a new member struct clk_vdd_class *vdd_class;
> in struct clk_init_data and struct clk_core
>
> And then in clk_register(...)
> core->vdd_class = hw->init->vdd_class;
>
> In many clock APIs, it will check the core->vdd_class to select the
> correct control flow.
> So, if we don't assign an initial value to clk_init_data of mtk_mux
> clock type, something might go wrong. And assigning an initial value
> might be the easiest and good way to avoid such problem if any new clock
> support added in the future.
>
Thanks for your explanation. You mean that as clk_init_data is on the stack it
can have random values, and would lead to the fact that vdd_class is interpreted
as allocated although it should be NULL.
Sounds reasonable to me. You might want to resend with a better commit message.
Feel free to add my
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
Regards,
Matthias
> [1] https://android-review.googlesource.com/c/kernel/common/+/1278046
>
>> It's a widely used pattern:
>> $ git grep "struct clk_init_data init;"| wc -l
>> 235
>>
>> Regards,
>> Matthias
>>
>>> Fixes: a3ae549917f1 ("clk: mediatek: Add new clkmux register API")
>>> Cc: <stable@vger.kernel.org>
>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>> ---
>>> drivers/clk/mediatek/clk-mux.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
>>> index 76f9cd0..14e127e 100644
>>> --- a/drivers/clk/mediatek/clk-mux.c
>>> +++ b/drivers/clk/mediatek/clk-mux.c
>>> @@ -160,7 +160,7 @@ struct clk *mtk_clk_register_mux(const struct mtk_mux *mux,
>>> spinlock_t *lock)
>>> {
>>> struct mtk_clk_mux *clk_mux;
>>> - struct clk_init_data init;
>>> + struct clk_init_data init = {};
>>> struct clk *clk;
>>>
>>> clk_mux = kzalloc(sizeof(*clk_mux), GFP_KERNEL);
>>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 7/7] [not for merge] netstats: example use of stats_fs API
From: Emanuele Giuseppe Esposito @ 2020-05-26 15:45 UTC (permalink / raw)
To: Andrew Lunn
Cc: linux-s390, kvm, linux-doc, netdev, Emanuele Giuseppe Esposito,
linux-kernel, kvm-ppc, Jonathan Adams, Christian Borntraeger,
Alexander Viro, David Rientjes, linux-fsdevel, Paolo Bonzini,
linux-mips, linuxppc-dev, linux-arm-kernel, Jim Mattson
In-Reply-To: <20200526141605.GJ768009@lunn.ch>
Hi Andrew
> How do you atomically get and display a group of statistics?
>
> If you look at how the netlink socket works, you will see code like:
>
> do {
> start = u64_stats_fetch_begin_irq(&cpu_stats->syncp);
> rx_packets = cpu_stats->rx_packets;
> rx_bytes = cpu_stats->rx_bytes;
> ....
> } while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, start));
>
> It will ensure that rx_packets and rx_bytes are consistent with each
> other. If the value of the sequence counter changes while inside the
> loop, the loop so repeated until it does not change.
>
> In general, hardware counters in NICs are the same. You tell it to
> take a snapshot of the statistics counters, and then read them all
> back, to give a consistent view across all the statistics.
>
> I've not looked at this new code in detail, but it looks like you have
> one file per statistic, and assume each statistic is independent of
> every other statistic. This independence can limit how you use the
> values, particularly when debugging. The netlink interface we use does
> not have this limitation.
You're right, statistics are treated independently so what you describe
is currently not supported.
In KVM the utilization is more qualitative, so there isn't such problem.
But as long as the interface is based on file access, the possibility of
snapshotting might not be useful; however, it could still be considered
to be added later together with the binary access.
Jonathan, how is your metricfs handling this case?
Thank you,
Emanuele
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3] iio: stm32-adc: remove usage of iio_priv_to_dev() helper
From: Fabrice Gasnier @ 2020-05-26 15:46 UTC (permalink / raw)
To: Alexandru Ardelean, linux-iio, linux-stm32, linux-arm-kernel,
linux-kernel
Cc: olivier.moysan, mcoquelin.stm32, jic23, alexandre.torgue
In-Reply-To: <20200526134448.7328-1-alexandru.ardelean@analog.com>
On 5/26/20 3:44 PM, Alexandru Ardelean wrote:
> We may want to get rid of the iio_priv_to_dev() helper. The reason is that
> we will hide some of the members of the iio_dev structure (to prevent
> drivers from accessing them directly), and that will also mean hiding the
> implementation of the iio_priv_to_dev() helper inside the IIO core.
>
> Hiding the implementation of iio_priv_to_dev() implies that some fast-paths
> may not be fast anymore, so a general idea is to try to get rid of the
> iio_priv_to_dev() altogether.
> The iio_priv() helper won't be affected by the rework, as the iio_dev
> struct will keep a reference to the private information.
>
> For this driver, not using iio_priv_to_dev(), means reworking some paths to
> pass the iio device and using iio_priv() to access the private information.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>
> Changelog v2 -> v3:
> - update doc-strings; warnings show-up during build with W=1 arg
>
> Changelog v1 -> v2:
> - converted to pass reference to IIO device in function hooks (vs
> reference
> to adc private data)
>
> drivers/iio/adc/stm32-adc.c | 118 +++++++++++++++++++-----------------
> 1 file changed, 63 insertions(+), 55 deletions(-)
Hi Alexandru,
Acked-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Thanks,
Fabrice
>
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index ae622ee6d08c..1dd97ec5571c 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -162,10 +162,10 @@ struct stm32_adc_cfg {
> struct stm32_adc_trig_info *trigs;
> bool clk_required;
> bool has_vregready;
> - int (*prepare)(struct stm32_adc *);
> - void (*start_conv)(struct stm32_adc *, bool dma);
> - void (*stop_conv)(struct stm32_adc *);
> - void (*unprepare)(struct stm32_adc *);
> + int (*prepare)(struct iio_dev *);
> + void (*start_conv)(struct iio_dev *, bool dma);
> + void (*stop_conv)(struct iio_dev *);
> + void (*unprepare)(struct iio_dev *);
> const unsigned int *smp_cycles;
> };
>
> @@ -538,10 +538,11 @@ static void stm32_adc_set_res(struct stm32_adc *adc)
>
> static int stm32_adc_hw_stop(struct device *dev)
> {
> - struct stm32_adc *adc = dev_get_drvdata(dev);
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct stm32_adc *adc = iio_priv(indio_dev);
>
> if (adc->cfg->unprepare)
> - adc->cfg->unprepare(adc);
> + adc->cfg->unprepare(indio_dev);
>
> if (adc->clk)
> clk_disable_unprepare(adc->clk);
> @@ -551,7 +552,8 @@ static int stm32_adc_hw_stop(struct device *dev)
>
> static int stm32_adc_hw_start(struct device *dev)
> {
> - struct stm32_adc *adc = dev_get_drvdata(dev);
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct stm32_adc *adc = iio_priv(indio_dev);
> int ret;
>
> if (adc->clk) {
> @@ -563,7 +565,7 @@ static int stm32_adc_hw_start(struct device *dev)
> stm32_adc_set_res(adc);
>
> if (adc->cfg->prepare) {
> - ret = adc->cfg->prepare(adc);
> + ret = adc->cfg->prepare(indio_dev);
> if (ret)
> goto err_clk_dis;
> }
> @@ -579,7 +581,7 @@ static int stm32_adc_hw_start(struct device *dev)
>
> /**
> * stm32f4_adc_start_conv() - Start conversions for regular channels.
> - * @adc: stm32 adc instance
> + * @indio_dev: IIO device instance
> * @dma: use dma to transfer conversion result
> *
> * Start conversions for regular channels.
> @@ -587,8 +589,10 @@ static int stm32_adc_hw_start(struct device *dev)
> * conversions, in IIO buffer modes. Otherwise, use ADC interrupt with direct
> * DR read instead (e.g. read_raw, or triggered buffer mode without DMA).
> */
> -static void stm32f4_adc_start_conv(struct stm32_adc *adc, bool dma)
> +static void stm32f4_adc_start_conv(struct iio_dev *indio_dev, bool dma)
> {
> + struct stm32_adc *adc = iio_priv(indio_dev);
> +
> stm32_adc_set_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN);
>
> if (dma)
> @@ -605,8 +609,10 @@ static void stm32f4_adc_start_conv(struct stm32_adc *adc, bool dma)
> stm32_adc_set_bits(adc, STM32F4_ADC_CR2, STM32F4_SWSTART);
> }
>
> -static void stm32f4_adc_stop_conv(struct stm32_adc *adc)
> +static void stm32f4_adc_stop_conv(struct iio_dev *indio_dev)
> {
> + struct stm32_adc *adc = iio_priv(indio_dev);
> +
> stm32_adc_clr_bits(adc, STM32F4_ADC_CR2, STM32F4_EXTEN_MASK);
> stm32_adc_clr_bits(adc, STM32F4_ADC_SR, STM32F4_STRT);
>
> @@ -615,8 +621,9 @@ static void stm32f4_adc_stop_conv(struct stm32_adc *adc)
> STM32F4_ADON | STM32F4_DMA | STM32F4_DDS);
> }
>
> -static void stm32h7_adc_start_conv(struct stm32_adc *adc, bool dma)
> +static void stm32h7_adc_start_conv(struct iio_dev *indio_dev, bool dma)
> {
> + struct stm32_adc *adc = iio_priv(indio_dev);
> enum stm32h7_adc_dmngt dmngt;
> unsigned long flags;
> u32 val;
> @@ -635,9 +642,9 @@ static void stm32h7_adc_start_conv(struct stm32_adc *adc, bool dma)
> stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_ADSTART);
> }
>
> -static void stm32h7_adc_stop_conv(struct stm32_adc *adc)
> +static void stm32h7_adc_stop_conv(struct iio_dev *indio_dev)
> {
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct stm32_adc *adc = iio_priv(indio_dev);
> int ret;
> u32 val;
>
> @@ -652,9 +659,9 @@ static void stm32h7_adc_stop_conv(struct stm32_adc *adc)
> stm32_adc_clr_bits(adc, STM32H7_ADC_CFGR, STM32H7_DMNGT_MASK);
> }
>
> -static int stm32h7_adc_exit_pwr_down(struct stm32_adc *adc)
> +static int stm32h7_adc_exit_pwr_down(struct iio_dev *indio_dev)
> {
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct stm32_adc *adc = iio_priv(indio_dev);
> int ret;
> u32 val;
>
> @@ -690,9 +697,9 @@ static void stm32h7_adc_enter_pwr_down(struct stm32_adc *adc)
> stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_DEEPPWD);
> }
>
> -static int stm32h7_adc_enable(struct stm32_adc *adc)
> +static int stm32h7_adc_enable(struct iio_dev *indio_dev)
> {
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct stm32_adc *adc = iio_priv(indio_dev);
> int ret;
> u32 val;
>
> @@ -713,9 +720,9 @@ static int stm32h7_adc_enable(struct stm32_adc *adc)
> return ret;
> }
>
> -static void stm32h7_adc_disable(struct stm32_adc *adc)
> +static void stm32h7_adc_disable(struct iio_dev *indio_dev)
> {
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct stm32_adc *adc = iio_priv(indio_dev);
> int ret;
> u32 val;
>
> @@ -730,12 +737,12 @@ static void stm32h7_adc_disable(struct stm32_adc *adc)
>
> /**
> * stm32h7_adc_read_selfcalib() - read calibration shadow regs, save result
> - * @adc: stm32 adc instance
> + * @indio_dev: IIO device instance
> * Note: Must be called once ADC is enabled, so LINCALRDYW[1..6] are writable
> */
> -static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc)
> +static int stm32h7_adc_read_selfcalib(struct iio_dev *indio_dev)
> {
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct stm32_adc *adc = iio_priv(indio_dev);
> int i, ret;
> u32 lincalrdyw_mask, val;
>
> @@ -774,12 +781,12 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc)
>
> /**
> * stm32h7_adc_restore_selfcalib() - Restore saved self-calibration result
> - * @adc: stm32 adc instance
> + * @indio_dev: IIO device instance
> * Note: ADC must be enabled, with no on-going conversions.
> */
> -static int stm32h7_adc_restore_selfcalib(struct stm32_adc *adc)
> +static int stm32h7_adc_restore_selfcalib(struct iio_dev *indio_dev)
> {
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct stm32_adc *adc = iio_priv(indio_dev);
> int i, ret;
> u32 lincalrdyw_mask, val;
>
> @@ -847,12 +854,12 @@ static int stm32h7_adc_restore_selfcalib(struct stm32_adc *adc)
>
> /**
> * stm32h7_adc_selfcalib() - Procedure to calibrate ADC
> - * @adc: stm32 adc instance
> + * @indio_dev: IIO device instance
> * Note: Must be called once ADC is out of power down.
> */
> -static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
> +static int stm32h7_adc_selfcalib(struct iio_dev *indio_dev)
> {
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct stm32_adc *adc = iio_priv(indio_dev);
> int ret;
> u32 val;
>
> @@ -903,7 +910,7 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
>
> /**
> * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
> - * @adc: stm32 adc instance
> + * @indio_dev: IIO device instance
> * Leave power down mode.
> * Configure channels as single ended or differential before enabling ADC.
> * Enable ADC.
> @@ -912,30 +919,31 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
> * - Only one input is selected for single ended (e.g. 'vinp')
> * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
> */
> -static int stm32h7_adc_prepare(struct stm32_adc *adc)
> +static int stm32h7_adc_prepare(struct iio_dev *indio_dev)
> {
> + struct stm32_adc *adc = iio_priv(indio_dev);
> int calib, ret;
>
> - ret = stm32h7_adc_exit_pwr_down(adc);
> + ret = stm32h7_adc_exit_pwr_down(indio_dev);
> if (ret)
> return ret;
>
> - ret = stm32h7_adc_selfcalib(adc);
> + ret = stm32h7_adc_selfcalib(indio_dev);
> if (ret < 0)
> goto pwr_dwn;
> calib = ret;
>
> stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
>
> - ret = stm32h7_adc_enable(adc);
> + ret = stm32h7_adc_enable(indio_dev);
> if (ret)
> goto pwr_dwn;
>
> /* Either restore or read calibration result for future reference */
> if (calib)
> - ret = stm32h7_adc_restore_selfcalib(adc);
> + ret = stm32h7_adc_restore_selfcalib(indio_dev);
> else
> - ret = stm32h7_adc_read_selfcalib(adc);
> + ret = stm32h7_adc_read_selfcalib(indio_dev);
> if (ret)
> goto disable;
>
> @@ -944,16 +952,18 @@ static int stm32h7_adc_prepare(struct stm32_adc *adc)
> return 0;
>
> disable:
> - stm32h7_adc_disable(adc);
> + stm32h7_adc_disable(indio_dev);
> pwr_dwn:
> stm32h7_adc_enter_pwr_down(adc);
>
> return ret;
> }
>
> -static void stm32h7_adc_unprepare(struct stm32_adc *adc)
> +static void stm32h7_adc_unprepare(struct iio_dev *indio_dev)
> {
> - stm32h7_adc_disable(adc);
> + struct stm32_adc *adc = iio_priv(indio_dev);
> +
> + stm32h7_adc_disable(indio_dev);
> stm32h7_adc_enter_pwr_down(adc);
> }
>
> @@ -1160,7 +1170,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>
> stm32_adc_conv_irq_enable(adc);
>
> - adc->cfg->start_conv(adc, false);
> + adc->cfg->start_conv(indio_dev, false);
>
> timeout = wait_for_completion_interruptible_timeout(
> &adc->completion, STM32_ADC_TIMEOUT);
> @@ -1173,7 +1183,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
> ret = IIO_VAL_INT;
> }
>
> - adc->cfg->stop_conv(adc);
> + adc->cfg->stop_conv(indio_dev);
>
> stm32_adc_conv_irq_disable(adc);
>
> @@ -1227,8 +1237,8 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>
> static irqreturn_t stm32_adc_threaded_isr(int irq, void *data)
> {
> - struct stm32_adc *adc = data;
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct iio_dev *indio_dev = data;
> + struct stm32_adc *adc = iio_priv(indio_dev);
> const struct stm32_adc_regspec *regs = adc->cfg->regs;
> u32 status = stm32_adc_readl(adc, regs->isr_eoc.reg);
>
> @@ -1240,8 +1250,8 @@ static irqreturn_t stm32_adc_threaded_isr(int irq, void *data)
>
> static irqreturn_t stm32_adc_isr(int irq, void *data)
> {
> - struct stm32_adc *adc = data;
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct iio_dev *indio_dev = data;
> + struct stm32_adc *adc = iio_priv(indio_dev);
> const struct stm32_adc_regspec *regs = adc->cfg->regs;
> u32 status = stm32_adc_readl(adc, regs->isr_eoc.reg);
>
> @@ -1514,7 +1524,7 @@ static int __stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
> if (!adc->dma_chan)
> stm32_adc_conv_irq_enable(adc);
>
> - adc->cfg->start_conv(adc, !!adc->dma_chan);
> + adc->cfg->start_conv(indio_dev, !!adc->dma_chan);
>
> return 0;
>
> @@ -1547,7 +1557,7 @@ static void __stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
> struct stm32_adc *adc = iio_priv(indio_dev);
> struct device *dev = indio_dev->dev.parent;
>
> - adc->cfg->stop_conv(adc);
> + adc->cfg->stop_conv(indio_dev);
> if (!adc->dma_chan)
> stm32_adc_conv_irq_disable(adc);
>
> @@ -1891,7 +1901,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
> indio_dev->info = &stm32_adc_iio_info;
> indio_dev->modes = INDIO_DIRECT_MODE | INDIO_HARDWARE_TRIGGERED;
>
> - platform_set_drvdata(pdev, adc);
> + platform_set_drvdata(pdev, indio_dev);
>
> ret = of_property_read_u32(pdev->dev.of_node, "reg", &adc->offset);
> if (ret != 0) {
> @@ -1905,7 +1915,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
>
> ret = devm_request_threaded_irq(&pdev->dev, adc->irq, stm32_adc_isr,
> stm32_adc_threaded_isr,
> - 0, pdev->name, adc);
> + 0, pdev->name, indio_dev);
> if (ret) {
> dev_err(&pdev->dev, "failed to request IRQ\n");
> return ret;
> @@ -1989,8 +1999,8 @@ static int stm32_adc_probe(struct platform_device *pdev)
>
> static int stm32_adc_remove(struct platform_device *pdev)
> {
> - struct stm32_adc *adc = platform_get_drvdata(pdev);
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct stm32_adc *adc = iio_priv(indio_dev);
>
> pm_runtime_get_sync(&pdev->dev);
> iio_device_unregister(indio_dev);
> @@ -2012,8 +2022,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> #if defined(CONFIG_PM_SLEEP)
> static int stm32_adc_suspend(struct device *dev)
> {
> - struct stm32_adc *adc = dev_get_drvdata(dev);
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>
> if (iio_buffer_enabled(indio_dev))
> __stm32_adc_buffer_predisable(indio_dev);
> @@ -2023,8 +2032,7 @@ static int stm32_adc_suspend(struct device *dev)
>
> static int stm32_adc_resume(struct device *dev)
> {
> - struct stm32_adc *adc = dev_get_drvdata(dev);
> - struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> int ret;
>
> ret = pm_runtime_force_resume(dev);
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 01/20] dt-bindings: arm: gic: Allow combining arm,gic-400 compatible strings
From: Rob Herring @ 2020-05-26 15:49 UTC (permalink / raw)
To: André Przywara
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Lorenzo Pieralisi, Marc Zyngier, Liviu Dudau, Geert Uytterhoeven,
Sudeep Holla, Linux ARM
In-Reply-To: <6e2cc3e9-b14f-2b50-0390-addcc82389e0@arm.com>
On Tue, May 19, 2020 at 3:20 AM André Przywara <andre.przywara@arm.com> wrote:
>
> On 19/05/2020 08:39, Geert Uytterhoeven wrote:
>
> Hi Geert,
>
>
> > On Wed, May 13, 2020 at 12:31 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >> The arm,gic-400 compatible is probably the best matching string for the
> >> GIC in most modern SoCs, but was only introduced later into the kernel.
> >> For historic reasons and to keep compatibility, some SoC DTs were thus
> >> using a combination of this name and one of the older strings, which
> >> currently the binding denies.
> >>
> >> Add a stanza to the DT binding to allow "arm,gic-400", followed by
> >> either "arm,cortex-a15-gic" or "arm,cortex-a7-gic". This fixes binding
> >> compliance for quite some SoC .dtsi files in the kernel tree.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >
> > Thanks for your patch, I was just looking into this issue ;-)
> >
> >> --- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
> >> +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
> >> @@ -39,6 +39,12 @@ properties:
> >> - qcom,msm-8660-qgic
> >> - qcom,msm-qgic2
> >>
> >> + - items:
> >> + - const: arm,gic-400
> >> + - enum:
> >> + - arm,cortex-a15-gic
> >> + - arm,cortex-a7-gic
> >> +
> >> - items:
> >> - const: arm,arm1176jzf-devchip-gic
> >> - const: arm,arm11mp-gic
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/r9a06g032.dtsi#n177
> > has them in the other order.
> >
> > What do you think is the preferred solution: reverting the order, or dropping
> > one or the other?
>
> Reverting the order would be the right thing. Theoretically this might
> change what the drivers match against, but there should be no difference
> between those strings anyway. And certainly Linux does not care which of
> the many strings it sees.
>
> The proper order is not really obvious here, but the cortex-a{15,7}-gic
> names serve as the missing "arm,gic-v2" generic fallback string here, I
> think just for historical reasons.
Really, they probably should have been mutually exclusive given an
implementation has one or the other. Since we have both, the order in
the schema is correct given gic-400 is a superset (multi-cluster) and
the compatible came later in time.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 0/2] mmc: mmci_sdmmc: fix dma api warnings
From: Ludovic Barre @ 2020-05-26 15:51 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel,
srinivas.kandagatla, Ludovic Barre, Maxime Coquelin, linux-stm32,
linux-arm-kernel
This patch series fixes warnings see with DMA_API_DEBUG_SG=y
Ludovic Barre (2):
mmc: mmci_sdmmc: fix DMA API warning overlapping mappings
mmc: mmci_sdmmc: fix DMA API warning max segment size
drivers/mmc/host/mmci_stm32_sdmmc.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 2/2] mmc: mmci_sdmmc: fix DMA API warning max segment size
From: Ludovic Barre @ 2020-05-26 15:51 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel,
srinivas.kandagatla, Ludovic Barre, Maxime Coquelin, linux-stm32,
linux-arm-kernel
In-Reply-To: <20200526155103.12514-1-ludovic.barre@st.com>
Turning on CONFIG_DMA_API_DEBUG_SG results in the following warning:
WARNING: CPU: 1 PID: 85 at kernel/dma/debug.c:1302 debug_dma_map_sg+0x2a0/0x3cc
mmci-pl18x 58005000.sdmmc: DMA-API: mapping sg segment longer than device claims to support [len=126976] [max=65536]
dma api debug checks and compares the segment size to
dma_get_max_seg_size (dev->dma_parms->max_segment_size),
the sdmmc variant has an internal DMA and should define
its max_segment_size constraint to avoid this warning.
This Patch defines the dev->dma_parms->max_segment_size
with the constraint already set for mmc core
(host->mmc->max_seg_size).
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
drivers/mmc/host/mmci_stm32_sdmmc.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 2965b1c062e1..51db30acf4dc 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -119,20 +119,19 @@ static void sdmmc_idma_unprep_data(struct mmci_host *host,
static int sdmmc_idma_setup(struct mmci_host *host)
{
struct sdmmc_idma *idma;
+ struct device *dev = mmc_dev(host->mmc);
- idma = devm_kzalloc(mmc_dev(host->mmc), sizeof(*idma), GFP_KERNEL);
+ idma = devm_kzalloc(dev, sizeof(*idma), GFP_KERNEL);
if (!idma)
return -ENOMEM;
host->dma_priv = idma;
if (host->variant->dma_lli) {
- idma->sg_cpu = dmam_alloc_coherent(mmc_dev(host->mmc),
- SDMMC_LLI_BUF_LEN,
+ idma->sg_cpu = dmam_alloc_coherent(dev, SDMMC_LLI_BUF_LEN,
&idma->sg_dma, GFP_KERNEL);
if (!idma->sg_cpu) {
- dev_err(mmc_dev(host->mmc),
- "Failed to alloc IDMA descriptor\n");
+ dev_err(dev, "Failed to alloc IDMA descriptor\n");
return -ENOMEM;
}
host->mmc->max_segs = SDMMC_LLI_BUF_LEN /
@@ -143,7 +142,7 @@ static int sdmmc_idma_setup(struct mmci_host *host)
host->mmc->max_seg_size = host->mmc->max_req_size;
}
- return 0;
+ return dma_set_max_seg_size(dev, host->mmc->max_seg_size);
}
static int sdmmc_idma_start(struct mmci_host *host, unsigned int *datactrl)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/2] mmc: mmci_sdmmc: fix DMA API warning overlapping mappings
From: Ludovic Barre @ 2020-05-26 15:51 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel,
srinivas.kandagatla, Ludovic Barre, Maxime Coquelin, linux-stm32,
linux-arm-kernel
In-Reply-To: <20200526155103.12514-1-ludovic.barre@st.com>
Turning on CONFIG_DMA_API_DEBUG_SG results in the following warning:
WARNING: CPU: 1 PID: 20 at kernel/dma/debug.c:500 add_dma_entry+0x16c/0x17c
DMA-API: exceeded 7 overlapping mappings of cacheline 0x031d2645
Modules linked in:
CPU: 1 PID: 20 Comm: kworker/1:1 Not tainted 5.5.0-rc2-00021-gdeda30999c2b-dirty #49
Hardware name: STM32 (Device Tree Support)
Workqueue: events_freezable mmc_rescan
[<c03138c0>] (unwind_backtrace) from [<c030d760>] (show_stack+0x10/0x14)
[<c030d760>] (show_stack) from [<c0f2eb28>] (dump_stack+0xc0/0xd4)
[<c0f2eb28>] (dump_stack) from [<c034a14c>] (__warn+0xd0/0xf8)
[<c034a14c>] (__warn) from [<c034a530>] (warn_slowpath_fmt+0x94/0xb8)
[<c034a530>] (warn_slowpath_fmt) from [<c03bca0c>] (add_dma_entry+0x16c/0x17c)
[<c03bca0c>] (add_dma_entry) from [<c03bdf54>] (debug_dma_map_sg+0xe4/0x3d4)
[<c03bdf54>] (debug_dma_map_sg) from [<c0d09244>] (sdmmc_idma_prep_data+0x94/0xf8)
[<c0d09244>] (sdmmc_idma_prep_data) from [<c0d05a2c>] (mmci_prep_data+0x2c/0xb0)
[<c0d05a2c>] (mmci_prep_data) from [<c0d073ec>] (mmci_start_data+0x134/0x2f0)
[<c0d073ec>] (mmci_start_data) from [<c0d078d0>] (mmci_request+0xe8/0x154)
[<c0d078d0>] (mmci_request) from [<c0cecb44>] (mmc_start_request+0x94/0xbc)
DMA api debug brings to light leaking dma-mappings, dma_map_sg and
dma_unmap_sg are not correctly balanced.
If a request is prepared, the dma_map/unmap are done in asynchronous
call pre_req (prep_data) and post_req (unprep_data). In this case
the dma-mapping is right balanced.
But if the request was not prepared, the data->host_cookie is
define to zero and the dma_map/unmap must be done in the request.
The dma_map is called by mmci_dma_start (prep_data), but there is
no dma_unmap in this case.
This patch adds dma_unmap_sg when the dma is finalized and
the data cookie is zero (request not prepared).
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
drivers/mmc/host/mmci_stm32_sdmmc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 14f99d8aa3f0..2965b1c062e1 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -188,6 +188,9 @@ static int sdmmc_idma_start(struct mmci_host *host, unsigned int *datactrl)
static void sdmmc_idma_finalize(struct mmci_host *host, struct mmc_data *data)
{
writel_relaxed(0, host->base + MMCI_STM32_IDMACTRLR);
+
+ if (!data->host_cookie)
+ sdmmc_idma_unprep_data(host, data, 0);
}
static void mmci_sdmmc_set_clkreg(struct mmci_host *host, unsigned int desired)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 03/16] mfd: mfd-core: match device tree node against reg property
From: Michael Walle @ 2020-05-26 15:54 UTC (permalink / raw)
To: Lee Jones
Cc: linux-pwm, Linus Walleij, Thierry Reding, linux-watchdog,
Andy Shevchenko, Marc Zyngier, Bartosz Golaszewski,
Uwe Kleine-König, Guenter Roeck, devicetree, Jean Delvare,
Jason Cooper, linux-gpio, Rob Herring, Thomas Gleixner,
Wim Van Sebroeck, linux-arm-kernel, linux-hwmon,
Greg Kroah-Hartman, linux-kernel, Li Yang, Mark Brown, Shawn Guo
In-Reply-To: <20200526072427.GC3628@dell>
Am 2020-05-26 09:24, schrieb Lee Jones:
> On Mon, 25 May 2020, Michael Walle wrote:
>
>> Am 2020-05-15 12:28, schrieb Lee Jones:
>> > On Thu, 30 Apr 2020, Michael Walle wrote:
>> >
>> > > Hi Lee,
>> > >
>> > > Am 2020-04-23 19:45, schrieb Michael Walle:
>> > > > There might be multiple children with the device tree compatible, for
>> > > > example if a MFD has multiple instances of the same function. In this
>> > > > case only the first is matched and the other children get a wrong
>> > > > of_node reference.
>> > > > Add a new option to match also against the unit address of the child
>> > > > node. Additonally, a new helper OF_MFD_CELL_REG is added.
>> > >
>> > >
>> > > Do you think this is feasible? I guess this is the biggest uncertainty
>> > > for me at the moment in this patch series.
>> >
>> > I think it sounds fine in principle. So long as it doesn't change the
>> > existing behaviour when of_reg isn't set.
>> >
>> > > > Signed-off-by: Michael Walle <michael@walle.cc>
>> > > > ---
>> > > > drivers/mfd/mfd-core.c | 29 ++++++++++++++++++++---------
>> > > > include/linux/mfd/core.h | 26 ++++++++++++++++++++------
>> > > > 2 files changed, 40 insertions(+), 15 deletions(-)
>
> [...]
>
>> > > > diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
>> > > > index d01d1299e49d..c2c0ad6b14f3 100644
>> > > > --- a/include/linux/mfd/core.h
>> > > > +++ b/include/linux/mfd/core.h
>> > > > @@ -13,8 +13,11 @@
>> > > > #include <linux/platform_device.h>
>> > > >
>> > > > #define MFD_RES_SIZE(arr) (sizeof(arr) / sizeof(struct resource))
>> > > > +#define MFD_OF_REG_VALID BIT(31)
>> >
>> > What about 64bit platforms?
>>
>> The idea was to have this as a logical number. I.e. for now you may
>> only
>> have one subdevice per unique compatible string. In fact, if you have
>> a
>> look at the ab8500.c, there are multiple "stericsson,ab8500-pwm"
>> subdevices. But there is only one DT node for all three of it. I guess
>> this works as long as you don't use phandles to reference the pwm node
>> in the device tree. Or you don't want to use device tree properties
>> per subdevice (for example the "timeout-sec" of a watchdog device).
>>
>> So to circumvent this, I thought of having the unit-address (and thus
>> the "reg" property) to differentiate between multiple subdevices. Now
>> there is one special case for me: this board management controller
>> might be upgradable and it might change internally. Thus I came up
>> with that logical numbering of subdevices. Rob doesn't seem to be a
>> fan of that, though. Therefore, having bit 31 as a valid indicator
>> leaves you with 2^31 logical devices, which should be enough ;)
>>
>> Rob proposed to have the internal offset as the unit-address. But
>> in that case I can also use devm_of_platform_populate() and don't
>> need the OF_MFD_CELL_REG; I'd just parse the reg offset in each
>> individual subdevice driver. But like I said, I wanted to keep the
>> internal offsets out of the device tree.
>
> Oh, I see what you're doing.
>
> So you're adding an arbitrary ID to the device's reg property in DT?
Yes.
> How is this not a hack?
Well IMHO this is not more or less a hack as the current of_node
handling of MFD devices, which happens to work only because there
is only one device per compatible string (or it doesn't really work,
like in the stericsson,ab8500-pwm case). The of_node is assigned
according to the compatible string, just like in my case, only that
I have two subdevices with the same compatible string.
> Why don't you use the full address for identification?
Like I said, in the long term I would like to have support for
different versions of the board management controller without having
to change the device tree and have device tree bindings for the
subdevices at the same time. But it seems, that this is not possible
and I guess I have to bite the bullet and may need to provide another
device tree if the controller might be updated.
-michael
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v4 2/3] serial: 8250_dw: Simplify the ref clock rate setting procedure
From: Serge Semin @ 2020-05-26 16:03 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby,
Andy Shevchenko
Cc: Arnd Bergmann, linux-kernel, Russell King, Maxime Ripard,
Alexey Malahov, Serge Semin, Serge Semin, linux-serial,
linux-mips, Will Deacon, linux-arm-kernel
In-Reply-To: <20200526160316.26136-1-Sergey.Semin@baikalelectronics.ru>
Really instead of twice checking the clk_round_rate() return value
we could do it once, and if it isn't error the clock rate can be changed.
By doing so we decrease a number of ret-value tests and remove a weird
goto-based construction implemented in the dw8250_set_termios() method.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Will Deacon <will@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-mips@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
Changelog v3:
- This is a new patch.
---
drivers/tty/serial/8250/8250_dw.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index aab3cccc6789..12866083731d 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -282,20 +282,13 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
clk_disable_unprepare(d->clk);
rate = clk_round_rate(d->clk, baud * 16);
- if (rate < 0)
- ret = rate;
- else if (rate == 0)
- ret = -ENOENT;
- else
+ if (rate > 0) {
ret = clk_set_rate(d->clk, rate);
+ if (!ret)
+ p->uartclk = rate;
+ }
clk_prepare_enable(d->clk);
- if (ret)
- goto out;
-
- p->uartclk = rate;
-
-out:
p->status &= ~UPSTAT_AUTOCTS;
if (termios->c_cflag & CRTSCTS)
p->status |= UPSTAT_AUTOCTS;
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v4 1/3] serial: 8250: Add 8250 port clock update method
From: Serge Semin @ 2020-05-26 16:03 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, Arnd Bergmann, linux-mips, Russell King,
Maxime Ripard, Alexey Malahov, Serge Semin, Serge Semin,
linux-serial, Andy Shevchenko, Will Deacon, linux-arm-kernel
In-Reply-To: <20200526160316.26136-1-Sergey.Semin@baikalelectronics.ru>
Some platforms can be designed in a way so the UART port reference clock
might be asynchronously changed at some point. In Baikal-T1 SoC this may
happen due to the reference clock being shared between two UART ports, on
the Allwinner SoC the reference clock is derived from the CPU clock, so
any CPU frequency change should get to be known/reflected by/in the UART
controller as well. But it's not enough to just update the
uart_port->uartclk field of the corresponding UART port, the 8250
controller reference clock divisor should be altered so to preserve
current baud rate setting. All of these things is done in a coherent
way by calling the serial8250_update_uartclk() method provided in this
patch. Though note that it isn't supposed to be called from within the
UART port callbacks because the locks using to the protect the UART port
data are already taken in there.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Will Deacon <will@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-mips@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
Changelog v4:
- Export serial8250_update_uartclk() symbol for GPL modules only.
---
drivers/tty/serial/8250/8250_port.c | 38 +++++++++++++++++++++++++++++
include/linux/serial_8250.h | 2 ++
2 files changed, 40 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 4d83c85a7389..5596868c8832 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2628,6 +2628,44 @@ static unsigned int serial8250_get_baud_rate(struct uart_port *port,
(port->uartclk + tolerance) / 16);
}
+/*
+ * Note in order to avoid the tty port mutex deadlock don't use the next method
+ * within the uart port callbacks. Primarily it's supposed to be utilized to
+ * handle a sudden reference clock rate change.
+ */
+void serial8250_update_uartclk(struct uart_port *port, unsigned int uartclk)
+{
+ struct uart_8250_port *up = up_to_u8250p(port);
+ unsigned int baud, quot, frac = 0;
+ struct ktermios *termios;
+ unsigned long flags;
+
+ mutex_lock(&port->state->port.mutex);
+
+ if (port->uartclk == uartclk)
+ goto out_lock;
+
+ port->uartclk = uartclk;
+ termios = &port->state->port.tty->termios;
+
+ baud = serial8250_get_baud_rate(port, termios, NULL);
+ quot = serial8250_get_divisor(port, baud, &frac);
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ uart_update_timeout(port, termios->c_cflag, baud);
+
+ serial8250_set_divisor(port, baud, quot, frac);
+ serial_port_out(port, UART_LCR, up->lcr);
+ serial8250_out_MCR(up, UART_MCR_DTR | UART_MCR_RTS);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+
+out_lock:
+ mutex_unlock(&port->state->port.mutex);
+}
+EXPORT_SYMBOL_GPL(serial8250_update_uartclk);
+
void
serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
struct ktermios *old)
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 6545f8cfc8fa..2b70f736b091 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -155,6 +155,8 @@ extern int early_serial_setup(struct uart_port *port);
extern int early_serial8250_setup(struct earlycon_device *device,
const char *options);
+extern void serial8250_update_uartclk(struct uart_port *port,
+ unsigned int uartclk);
extern void serial8250_do_set_termios(struct uart_port *port,
struct ktermios *termios, struct ktermios *old);
extern void serial8250_do_set_ldisc(struct uart_port *port,
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 03/16] mfd: mfd-core: match device tree node against reg property
From: Andy Shevchenko @ 2020-05-26 16:03 UTC (permalink / raw)
To: Michael Walle
Cc: devicetree, Linus Walleij, Thierry Reding, Lee Jones,
linux-watchdog, Marc Zyngier, Bartosz Golaszewski,
Uwe Kleine-König, Guenter Roeck, linux-pwm, Jean Delvare,
Jason Cooper, linux-gpio, Rob Herring, Thomas Gleixner,
Wim Van Sebroeck, linux-arm-kernel, linux-hwmon,
Greg Kroah-Hartman, linux-kernel, Li Yang, Mark Brown, Shawn Guo
In-Reply-To: <f5704ce5a3e280f63c81fe35efb08234@walle.cc>
On Tue, May 26, 2020 at 05:54:38PM +0200, Michael Walle wrote:
> Am 2020-05-26 09:24, schrieb Lee Jones:
...
> Like I said, in the long term I would like to have support for
> different versions of the board management controller
> without having to change the device tree and have device tree bindings for the
> subdevices at the same time.
But isn't device tree to describe *very specific platform* rather than *class
of platforms*?
> But it seems, that this is not possible
> and I guess I have to bite the bullet and may need to provide another
> device tree if the controller might be updated.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v4 0/3] serial: 8250_dw: Fix ref clock usage
From: Serge Semin @ 2020-05-26 16:03 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby
Cc: Maxime Ripard, Alexey Kolotnikov, Vadim Vlasov, Maxim Kaurkin,
Ramil Zaripov, linux-mips, linux-kernel, Russell King,
Serge Semin, Alexey Malahov, Serge Semin, Arnd Bergmann,
Pavel Parkhomenko, Ekaterina Skachko, linux-serial,
Andy Shevchenko, Will Deacon, linux-arm-kernel
Greg, Jiri, the merge window is upon us, please review/merge in/whatever
the rest of the patches.
It might be dangerous if an UART port reference clock rate is suddenly
changed. In particular the 8250 port drivers (and AFAICS most of the tty
drivers using common clock framework clocks) rely either on the
exclusive reference clock utilization or on the ref clock rate being
always constant. Needless to say that it turns out not true and if some
other service suddenly changes the clock rate behind an UART port driver
back it's no good. So the port might not only end up with an invalid
uartclk value saved, but may also experience a distorted output/input
data since such action will effectively update the programmed baud-clock.
We discovered such problem on Baikal-T1 SoC where two DW 8250 ports have
got a shared reference clock. Allwinner SoC is equipped with an UART,
which clock is derived from the CPU PLL clock source, so the CPU frequency
change might be propagated down up to the serial port reference clock.
This patchset provides a way to fix the problem to the 8250 serial port
controllers and mostly fixes it for the DW 8250-compatible UART. I say
mostly because due to not having a facility to pause/stop and resume/
restart on-going transfers we implemented the UART clock rate update
procedure executed post factum of the actual reference clock rate change.
In addition the patchset includes a few fixes we discovered when were
working the issue. First one concerns the maximum baud rate setting used
to determine a serial port baud based on the current UART port clock rate.
Another one simplifies the ref clock rate setting procedure a bit.
This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4
Changelog v3:
- Refactor the original patch to adjust the UART port divisor instead of
requesting an exclusive ref clock utilization.
Changelog v4:
- Discard commit b426bf0fb085 ("serial: 8250: Fix max baud limit in generic
8250 port") since Greg has already merged it into the tty-next branch.
- Use EXPORT_SYMBOL_GPL() for the serial8250_update_uartclk() method.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Will Deacon <will@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-mips@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Serge Semin (3):
serial: 8250: Add 8250 port clock update method
serial: 8250_dw: Simplify the ref clock rate setting procedure
serial: 8250_dw: Fix common clocks usage race condition
drivers/tty/serial/8250/8250_dw.c | 125 +++++++++++++++++++++++++---
drivers/tty/serial/8250/8250_port.c | 38 +++++++++
include/linux/serial_8250.h | 2 +
3 files changed, 153 insertions(+), 12 deletions(-)
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox