* Re: [PATCH v4 5/5] coresight: sysfs: Allow select default sink on source enable.
From: Suzuki K Poulose @ 2020-06-02 11:51 UTC (permalink / raw)
To: mike.leach, linux-arm-kernel, coresight, mathieu.poirier; +Cc: acme
In-Reply-To: <20200526104642.9526-6-mike.leach@linaro.org>
Hi Mike,
On 05/26/2020 11:46 AM, Mike Leach wrote:
> When enabling a trace source using sysfs, allow the CoreSight system to
> auto-select a default sink if none has been enabled by the user.
>
> Uses the sink select algorithm that uses the default select priorities
> set when sinks are registered with the system. At present this will
> prefer ETR over ETB / ETF.
>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> index 7632d060e25d..bd1a52a65d00 100644
> --- a/drivers/hwtracing/coresight/coresight.c
> +++ b/drivers/hwtracing/coresight/coresight.c
> @@ -965,8 +965,15 @@ int coresight_enable(struct coresight_device *csdev)
> */
> sink = coresight_get_enabled_sink(false);
> if (!sink) {
> - ret = -EINVAL;
> - goto out;
> + /* look for a default sink if nothing enabled */
> + sink = coresight_find_default_sink(csdev);
> + if (!sink) {
> + ret = -EINVAL;
> + goto out;
> + }
> + /* mark the default as enabled */
> + sink->activated = true;
> + dev_info(&sink->dev, "Enabled default sink.");
> }
To be honest, I would drop this change and mandate that the
user enables the sink(s). There is no way to reliably match
which ETM is tracing to the sink above in case multiple ETMs
are being enabled, even with the above message. It is important
for sysfs mode, as the user must collect the trace data manually,
unlike the perf mode where the race data is collected and presented to
the user via memory buffers.
Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] usb: host: ehci-exynos: Fix error check in exynos_ehci_probe()
From: Tang Bin @ 2020-06-02 11:47 UTC (permalink / raw)
To: gregkh, stern, kgene, krzk
Cc: linux-samsung-soc, Tang Bin, linux-usb, linux-kernel,
Zhang Shengju, linux-arm-kernel
If the function platform_get_irq() failed, the negative value
returned will not be detected here. So fix error handling in
exynos_ehci_probe(). And when get irq failed, the function
platform_get_irq() logs an error message, so remove redundant
message here.
Fixes: 1bcc5aa87f04 ("USB: Add initial S5P EHCI driver")
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
---
drivers/usb/host/ehci-exynos.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index a4e9abcbd..1a9b7572e 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -203,9 +203,8 @@ static int exynos_ehci_probe(struct platform_device *pdev)
hcd->rsrc_len = resource_size(res);
irq = platform_get_irq(pdev, 0);
- if (!irq) {
- dev_err(&pdev->dev, "Failed to get IRQ\n");
- err = -ENODEV;
+ if (irq < 0) {
+ err = irq;
goto fail_io;
}
--
2.20.1.windows.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] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend
From: Macpaul Lin @ 2020-06-02 11:53 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Matthias Brugger, Alexander Tsoy,
Johan Hovold, Hui Wang, Szabolcs Szőke, Macpaul Lin,
alsa-devel, linux-usb
Cc: linux-arm-kernel, Macpaul Lin, linux-mediatek, linux-kernel,
Mediatek WSD Upstream
In-Reply-To: <linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org>
This patch fix incorrect power state changed by usb_audio_suspend()
when CONFIG_PM is enabled.
After receiving suspend PM message with auto flag, usb_audio_suspend()
change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other
resume PM message with auto flag can change power state to
SNDRV_CTL_POWER_D0 in __usb_audio_resume().
However, when system is not under auto suspend, resume PM message with
auto flag might not be able to receive on time which cause the power
state was incorrect. At this time, if a player starts to play sound,
will cause snd_usb_pcm_open() to access the card and setup_hw_info() will
resume the card.
But even the card is back to work and all function normal, the power
state is still in SNDRV_CTL_POWER_D3hot. Which cause the infinite loop
happened in snd_power_wait() to check the power state. Thus the
successive setting ioctl cannot be passed to card.
Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card
has been resumed successfully.
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
sound/usb/pcm.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index a4e4064..d667ecb 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1322,6 +1322,17 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre
if (err < 0)
return err;
+ /* fix incorrect power state when resuming by open and later ioctls */
+ if (IS_ENABLED(CONFIG_PM) &&
+ snd_power_get_state(subs->stream->chip->card)
+ == SNDRV_CTL_POWER_D3hot) {
+ /* set these variables for power state correction */
+ subs->stream->chip->autosuspended = 0;
+ subs->stream->chip->num_suspended_intf = 1;
+ dev_info(&subs->dev->dev,
+ "change power state from D3hot to D0\n");
+ }
+
return snd_usb_autoresume(subs->stream->chip);
}
--
1.7.9.5
_______________________________________________
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: [RFC PATCH v4 2/2] arm64: tlb: Use the TLBI RANGE feature in arm64
From: Zhenyu Ye @ 2020-06-02 12:06 UTC (permalink / raw)
To: catalin.marinas, will, suzuki.poulose, maz, steven.price,
guohanjun, olof
Cc: linux-arch, linux-kernel, xiexiangyou, zhangshaokun, linux-mm,
arm, prime.zeng, kuhn.chenqun, linux-arm-kernel
In-Reply-To: <20200601144713.2222-3-yezhenyu2@huawei.com>
Hi all,
Some optimizations to the codes:
On 2020/6/1 22:47, Zhenyu Ye wrote:
> - start = __TLBI_VADDR(start, asid);
> - end = __TLBI_VADDR(end, asid);
> + /*
> + * The minimum size of TLB RANGE is 2 pages;
> + * Use normal TLB instruction to handle odd pages.
> + * If the stride != PAGE_SIZE, this will never happen.
> + */
> + if (range_pages % 2 == 1) {
> + addr = __TLBI_VADDR(start, asid);
> + __tlbi_last_level(vale1is, vae1is, addr, last_level);
> + start += 1 << PAGE_SHIFT;
> + range_pages >>= 1;
> + }
>
We flush a single page here, and below loop does the same thing
if cpu not support TLB RANGE feature. So may we use a goto statement
to simplify the code.
> + while (range_pages > 0) {
> + if (cpus_have_const_cap(ARM64_HAS_TLBI_RANGE) &&
> + stride == PAGE_SIZE) {
> + num = (range_pages & TLB_RANGE_MASK) - 1;
> + if (num >= 0) {
> + addr = __TLBI_VADDR_RANGE(start, asid, scale,
> + num, 0);
> + __tlbi_last_level(rvale1is, rvae1is, addr,
> + last_level);
> + start += __TLBI_RANGE_SIZES(num, scale);
> + }
> + scale++;
> + range_pages >>= TLB_RANGE_MASK_SHIFT;
> + continue;
> }
> +
> + addr = __TLBI_VADDR(start, asid);
> + __tlbi_last_level(vale1is, vae1is, addr, last_level);
> + start += stride;
> + range_pages -= stride >> 12;
> }
> dsb(ish);
> }
>
Just like:
--8<---
if (range_pages %2 == 1)
goto flush_single_tlb;
while (range_pages > 0) {
if (cpus_have_const_cap(ARM64_HAS_TLBI_RANGE) &&
stride == PAGE_SIZE) {
num = ((range_pages >> 1) & TLB_RANGE_MASK) - 1;
if (num >= 0) {
addr = __TLBI_VADDR_RANGE(start, asid, scale,
num, 0);
__tlbi_last_level(rvale1is, rvae1is, addr,
last_level);
start += __TLBI_RANGE_SIZES(num, scale);
}
scale++;
range_pages >>= TLB_RANGE_MASK_SHIFT;
continue;
}
flush_single_tlb:
addr = __TLBI_VADDR(start, asid);
__tlbi_last_level(vale1is, vae1is, addr, last_level);
start += stride;
range_pages -= stride >> PAGE_SHIFT;
}
--8<---
_______________________________________________
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 v7 21/24] iommu/arm-smmu-v3: Add stall support for platform devices
From: Shameerali Kolothum Thodi @ 2020-06-02 12:12 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: devicetree@vger.kernel.org, kevin.tian@intel.com, will@kernel.org,
fenghua.yu@intel.com, linux-pci@vger.kernel.org,
felix.kuehling@amd.com, hch@infradead.org, jgg@ziepe.ca,
iommu@lists.linux-foundation.org, linux-mm@kvack.org,
catalin.marinas@arm.com, zhangfei.gao@linaro.org,
robin.murphy@arm.com, christian.koenig@amd.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20200602114611.GB1029680@myrica>
> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org]
> On Behalf Of Jean-Philippe Brucker
> Sent: 02 June 2020 12:46
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: devicetree@vger.kernel.org; kevin.tian@intel.com; fenghua.yu@intel.com;
> linux-pci@vger.kernel.org; felix.kuehling@amd.com; robin.murphy@arm.com;
> christian.koenig@amd.com; hch@infradead.org; jgg@ziepe.ca;
> iommu@lists.linux-foundation.org; catalin.marinas@arm.com;
> zhangfei.gao@linaro.org; will@kernel.org; linux-mm@kvack.org;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v7 21/24] iommu/arm-smmu-v3: Add stall support for
> platform devices
>
> On Tue, Jun 02, 2020 at 10:31:29AM +0000, Shameerali Kolothum Thodi wrote:
> > Hi Jean,
> >
> > > -----Original Message-----
> > > From: linux-arm-kernel
> > > [mailto:linux-arm-kernel-bounces@lists.infradead.org]
> > > On Behalf Of Jean-Philippe Brucker
> > > Sent: 02 June 2020 10:39
> > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> > > Cc: devicetree@vger.kernel.org; kevin.tian@intel.com;
> > > will@kernel.org; fenghua.yu@intel.com; jgg@ziepe.ca;
> > > linux-pci@vger.kernel.org; felix.kuehling@amd.com;
> > > hch@infradead.org; linux-mm@kvack.org;
> > > iommu@lists.linux-foundation.org; catalin.marinas@arm.com;
> > > zhangfei.gao@linaro.org; robin.murphy@arm.com;
> > > christian.koenig@amd.com; linux-arm-kernel@lists.infradead.org
> > > Subject: Re: [PATCH v7 21/24] iommu/arm-smmu-v3: Add stall support
> > > for platform devices
> > >
> > > Hi Shameer,
> > >
> > > On Mon, Jun 01, 2020 at 12:42:15PM +0000, Shameerali Kolothum Thodi
> > > wrote:
> > > > > /* IRQ and event handlers */
> > > > > +static int arm_smmu_handle_evt(struct arm_smmu_device *smmu,
> > > > > +u64
> > > > > +*evt) {
> > > > > + int ret;
> > > > > + u32 perm = 0;
> > > > > + struct arm_smmu_master *master;
> > > > > + bool ssid_valid = evt[0] & EVTQ_0_SSV;
> > > > > + u8 type = FIELD_GET(EVTQ_0_ID, evt[0]);
> > > > > + u32 sid = FIELD_GET(EVTQ_0_SID, evt[0]);
> > > > > + struct iommu_fault_event fault_evt = { };
> > > > > + struct iommu_fault *flt = &fault_evt.fault;
> > > > > +
> > > > > + /* Stage-2 is always pinned at the moment */
> > > > > + if (evt[1] & EVTQ_1_S2)
> > > > > + return -EFAULT;
> > > > > +
> > > > > + master = arm_smmu_find_master(smmu, sid);
> > > > > + if (!master)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + if (evt[1] & EVTQ_1_READ)
> > > > > + perm |= IOMMU_FAULT_PERM_READ;
> > > > > + else
> > > > > + perm |= IOMMU_FAULT_PERM_WRITE;
> > > > > +
> > > > > + if (evt[1] & EVTQ_1_EXEC)
> > > > > + perm |= IOMMU_FAULT_PERM_EXEC;
> > > > > +
> > > > > + if (evt[1] & EVTQ_1_PRIV)
> > > > > + perm |= IOMMU_FAULT_PERM_PRIV;
> > > > > +
> > > > > + if (evt[1] & EVTQ_1_STALL) {
> > > > > + flt->type = IOMMU_FAULT_PAGE_REQ;
> > > > > + flt->prm = (struct iommu_fault_page_request) {
> > > > > + .flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE,
> > > > > + .pasid = FIELD_GET(EVTQ_0_SSID, evt[0]),
> > > > > + .grpid = FIELD_GET(EVTQ_1_STAG, evt[1]),
> > > > > + .perm = perm,
> > > > > + .addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
> > > > > + };
> > > > > +
> > > >
> > > > > + if (ssid_valid)
> > > > > + flt->prm.flags |=
> > > IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> > > >
> > > > Do we need to set this for STALL mode only support? I had an issue
> > > > with this being set on a vSVA POC based on our D06 zip
> > > > device(which is a "fake " pci dev that supports STALL mode but no
> > > > PRI). The issue is, CMDQ_OP_RESUME doesn't have any ssid or SSV
> > > > params and works on sid
> > > and stag only.
> > >
> > > I don't understand the problem, arm_smmu_page_response() doesn't set
> > > SSID or SSV when sending a CMDQ_OP_RESUME. Could you detail the flow
> > > of a stall event and RESUME command in your prototype? Are you
> > > getting issues with the host driver or the guest driver?
> >
> > The issue is on the host side iommu_page_response(). The flow is
> > something like below.
> >
> > Stall: Host:-
> >
> > arm_smmu_handle_evt()
> > iommu_report_device_fault()
> > vfio_pci_iommu_dev_fault_handler()
> >
> > Stall: Qemu:-
> >
> > vfio_dma_fault_notifier_handler()
> > inject_faults()
> > smmuv3_inject_faults()
> >
> > Stall: Guest:-
> >
> > arm_smmu_handle_evt()
> > iommu_report_device_fault()
> > iommu_queue_iopf
> > ...
> > iopf_handle_group()
> > iopf_handle_single()
> > handle_mm_fault()
> > iopf_complete()
> > iommu_page_response()
> > arm_smmu_page_response()
> > arm_smmu_cmdq_issue_cmd(CMDQ_OP_RESUME)
> >
> > Resume: Qemu:-
> >
> > smmuv3_cmdq_consume(SMMU_CMD_RESUME)
> > smmuv3_notify_page_resp()
> > vfio:ioctl(page_response) --> struct iommu_page_response is filled
> > with only version, grpid and code.
> >
> > Resume: Host:-
> > ioctl(page_response)
> > iommu_page_response() --> fails as the pending req has PASID_VALID
> flag
> > set and it checks for a match.
>
> I believe the fix needs to be here. It's also wrong for PRI since not all PCIe
> endpoint require a PASID in the page response. Could you try the attached
> patch?
Going through the patch, yes, that will definitely fix the issue. But isn't it better if
the request itself indicate whether it expects a response msg with a valid pasid or
not? The response msg can come from userspace as well(vSVA) and if for some reason
doesn't set it for a req that expects pasid then it should be an error, right? In the temp
fix I had, I introduced another flag to indicate the endpoint has PRI support or not and
used that to verify the pasid requirement. But for the PRI case you mentioned
above, not sure it is easy to get that information or not. May be I am complicating things
here :)
Thanks,
Shameer
> Thanks,
> Jean
>
> > arm_smmu_page_response()
> >
> > Hope the above is clear.
> >
> > > We do need to forward the SSV flag all the way to the guest driver,
> > > so the guest can find the faulting address space using the SSID.
> > > Once the guest handled the fault, then we don't send the SSID back
> > > to the host as part of the RESUME command.
> >
> > True, the guest requires SSV flag to handle the page fault. But, as
> > shown in the flow above, the issue is on the host side
> > iommu_page_response() where it searches for a matching pending req
> > based on pasid. Not sure we can bypass that and call
> > arm_smmu_page_response() directly but then have to delete the pending req
> from the list as well.
> >
> > Please let me know if there is a better way to handle the host side
> > page response.
> >
> > Thanks,
> > Shameer
> >
> > > Thanks,
> > > Jean
> > >
> > > > Hence, it is difficult for
> > > > Qemu SMMUv3 to populate this fields while preparing a page
> > > > response. I can see that this flag is being checked in
> > > > iopf_handle_single() (patch
> > > > 04/24) as well. For POC, I used a temp fix[1] to work around this.
> > > > Please let
> > > me know your thoughts.
> > > >
> > > > Thanks,
> > > > Shameer
> > > >
> > > > 1.
> > > > https://github.com/hisilicon/kernel-dev/commit/99ff96146e924055f38
> > > > d97a
> > > > 5897e4becfa378d15
> > > >
> > >
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-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: Security Random Number Generator support
From: Ard Biesheuvel @ 2020-06-02 12:14 UTC (permalink / raw)
To: Neal Liu
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman, Sean Wang, lkml,
wsd_upstream, Rob Herring, linux-mediatek,
Linux Crypto Mailing List, Matt Mackall, Matthias Brugger,
Crystal Guo, Linux ARM
In-Reply-To: <1591085678-22764-1-git-send-email-neal.liu@mediatek.com>
On Tue, 2 Jun 2020 at 10:15, Neal Liu <neal.liu@mediatek.com> wrote:
>
> These patch series introduce a security random number generator
> which provides a generic interface to get hardware rnd from Secure
> state. The Secure state can be Arm Trusted Firmware(ATF), Trusted
> Execution Environment(TEE), or even EL2 hypervisor.
>
> Patch #1..2 adds sec-rng kernel driver for Trustzone based SoCs.
> For security awareness SoCs on ARMv8 with TrustZone enabled,
> peripherals like entropy sources is not accessible from normal world
> (linux) and rather accessible from secure world (HYP/ATF/TEE) only.
> This driver aims to provide a generic interface to Arm Trusted
> Firmware or Hypervisor rng service.
>
>
> changes since v1:
> - rename mt67xx-rng to mtk-sec-rng since all MediaTek ARMv8 SoCs can reuse
> this driver.
> - refine coding style and unnecessary check.
>
> changes since v2:
> - remove unused comments.
> - remove redundant variable.
>
> changes since v3:
> - add dt-bindings for MediaTek rng with TrustZone enabled.
> - revise HWRNG SMC call fid.
>
> changes since v4:
> - move bindings to the arm/firmware directory.
> - revise driver init flow to check more property.
>
> changes since v5:
> - refactor to more generic security rng driver which
> is not platform specific.
>
> *** BLURB HERE ***
>
> Neal Liu (2):
> dt-bindings: rng: add bindings for sec-rng
> hwrng: add sec-rng driver
>
There is no reason to model a SMC call as a driver, and represent it
via a DT node like this.
It would be much better if this SMC interface is made truly generic,
and wired into the arch_get_random() interface, which can be used much
earlier.
> .../devicetree/bindings/rng/sec-rng.yaml | 53 ++++++
> drivers/char/hw_random/Kconfig | 13 ++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/sec-rng.c | 155 ++++++++++++++++++
> 4 files changed, 222 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rng/sec-rng.yaml
> create mode 100644 drivers/char/hw_random/sec-rng.c
>
> --
> 2.18.0
_______________________________________________
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 0/2] ARM: Allow either FLATMEM or SPARSEMEM on the multiplatform build
From: Geert Uytterhoeven @ 2020-06-02 12:18 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Florian Fainelli, Arnd Bergmann, Stephen Boyd, Kevin Cernekee,
Mike Rapoport, Doug Berger, Gregory Fong, Linux ARM
In-Reply-To: <20200521145020.GS1551@shell.armlinux.org.uk>
Hi Russell,
On Thu, May 21, 2020 at 4:50 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
> On Thu, May 21, 2020 at 05:07:45PM +0300, Mike Rapoport wrote:
> > Ah, when either of these patforms will become a part of the
> > multiplatform build, the only option for multiplatform build will be
> > sparsemem.
> > So it would be nice if somebody could check the cost of using sparsemem
> > vs flatmem, espessially on low end machines.
>
> Do you think they will become part of multiplatform?
>
> If they're low-end machines, then adding:
>
> (a) the additional memory overhead of a multiplatform kernel
> (b) the additional runtime overhead of the complexities of multiplatform
> kernels
>
> is surely an odd thing to do, especially when few really care about
> these platforms becoming part of a multiplatform kernel, except those
> who like the idea of multiplat.
The fallacy of "multi-platform", again?
Isn't it a requirement for any new ARM v7-A platforms to be part of
ARCH_MULTI_V7, even if it's a low-end machine?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
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] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend
From: Macpaul Lin @ 2020-06-02 12:19 UTC (permalink / raw)
To: Jaroslav Kysela, Alexander Tsoy, Johan Hovold, Hui Wang,
Szabolcs Szőke, alsa-devel, linux-usb, Mediatek WSD Upstream,
Macpaul Lin, linux-kernel, linux-arm-kernel, linux-mediatek,
Matthias Brugger, Takashi Iwai
In-Reply-To: <1591098821-17910-1-git-send-email-macpaul.lin@mediatek.com>
On Tue, 2020-06-02 at 19:53 +0800, Macpaul Lin wrote:
> This patch fix incorrect power state changed by usb_audio_suspend()
> when CONFIG_PM is enabled.
>
> After receiving suspend PM message with auto flag, usb_audio_suspend()
> change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other
> resume PM message with auto flag can change power state to
> SNDRV_CTL_POWER_D0 in __usb_audio_resume().
>
> However, when system is not under auto suspend, resume PM message with
> auto flag might not be able to receive on time which cause the power
> state was incorrect. At this time, if a player starts to play sound,
> will cause snd_usb_pcm_open() to access the card and setup_hw_info() will
> resume the card.
>
> But even the card is back to work and all function normal, the power
> state is still in SNDRV_CTL_POWER_D3hot. Which cause the infinite loop
> happened in snd_power_wait() to check the power state. Thus the
> successive setting ioctl cannot be passed to card.
>
> Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card
> has been resumed successfully.
>
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
> sound/usb/pcm.c | 11 +++++++++++linux-usb@vger.kernel.org,
> 1 file changed, 11 insertions(+)
>
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index a4e4064..d667ecb 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -1322,6 +1322,17 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre
> if (err < 0)
> return err;
>
> + /* fix incorrect power state when resuming by open and later ioctls */
> + if (IS_ENABLED(CONFIG_PM) &&
> + snd_power_get_state(subs->stream->chip->card)
> + == SNDRV_CTL_POWER_D3hot) {
> + /* set these variables for power state correction */
> + subs->stream->chip->autosuspended = 0;
> + subs->stream->chip->num_suspended_intf = 1;
> + dev_info(&subs->dev->dev,
> + "change power state from D3hot to D0\n");
> + }
> +
> return snd_usb_autoresume(subs->stream->chip);
> }
>
The issue was found on kernel 4.14 (android tree). The test is to add
debug log in sound/core/init.c to check if the power state is
SNDRV_CTL_POWER_D3hot.
diff --git a/sound/core/init.c b/sound/core/init.c
index b02a997..a0bee76 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -1011,6 +1011,8 @@ int snd_power_wait(struct snd_card *card, unsigned
int power_state)
if (snd_power_get_state(card) == power_state)
break;
set_current_state(TASK_UNINTERRUPTIBLE);
+ pr_info("%s snd_power_get_state[%x]\n", __func__,
+ snd_power_get_state(card));
schedule_timeout(30 * HZ);
}
remove_wait_queue(&card->power_sleep, &wait);
After applied a work around by forcing the power state, pcm related
ioctl and parameter settings can be set to usb sound card correctly.
Otherwise a infinite loop will happened in snd_power_wait().
Here is the origin work around for verifying this power state issue on
kernel 4.14.
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 933adcd7af81..9acd50dd7155 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1274,6 +1274,16 @@ static int setup_hw_info(struct snd_pcm_runtime
*runtime, struct snd_usb_substre
if (err < 0)
return err;
+ /* avoid incorrect power state when executing IOCTL */
+ if (IS_ENABLED(CONFIG_PM) &&
+ snd_power_get_state(subs->stream->chip->card)
+ == SNDRV_CTL_POWER_D3hot) {
+ dev_info(&subs->dev->dev,
+ "change power state from D3hot to D0\n");
+ snd_power_change_state(subs->stream->chip->card,
+ SNDRV_CTL_POWER_D0);
+ }
+
param_period_time_if_needed = SNDRV_PCM_HW_PARAM_PERIOD_TIME;
if (subs->speed == USB_SPEED_FULL)
/* full speed devices have fixed data packet interval */
However, the patch I've send is meant to make sure the power state will
be corrected before snd_usb_autoresume(), It should be adapt to kernel
4.14 and later.
Thanks.
Macpaul Lin
_______________________________________________
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 0/2] ARM: Allow either FLATMEM or SPARSEMEM on the multiplatform build
From: Russell King - ARM Linux admin @ 2020-06-02 12:22 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Florian Fainelli, Arnd Bergmann, Stephen Boyd, Kevin Cernekee,
Mike Rapoport, Doug Berger, Gregory Fong, Linux ARM
In-Reply-To: <CAMuHMdViAcnaPzeD6cS8U-uzjZARGyf0dqaiehevn+DwCNid5Q@mail.gmail.com>
On Tue, Jun 02, 2020 at 02:18:48PM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
>
> On Thu, May 21, 2020 at 4:50 PM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> > On Thu, May 21, 2020 at 05:07:45PM +0300, Mike Rapoport wrote:
> > > Ah, when either of these patforms will become a part of the
> > > multiplatform build, the only option for multiplatform build will be
> > > sparsemem.
> > > So it would be nice if somebody could check the cost of using sparsemem
> > > vs flatmem, espessially on low end machines.
> >
> > Do you think they will become part of multiplatform?
> >
> > If they're low-end machines, then adding:
> >
> > (a) the additional memory overhead of a multiplatform kernel
> > (b) the additional runtime overhead of the complexities of multiplatform
> > kernels
> >
> > is surely an odd thing to do, especially when few really care about
> > these platforms becoming part of a multiplatform kernel, except those
> > who like the idea of multiplat.
>
> The fallacy of "multi-platform", again?
>
> Isn't it a requirement for any new ARM v7-A platforms to be part of
> ARCH_MULTI_V7, even if it's a low-end machine?
What does ARMv7 have to do with this thread, that is discussing the
older ARMv4 platforms? I find your comments above irrelevent to
this discussion, and seems to be worded to provoke a reaction.
Not playing.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] pinctrl: sirf: add missing put_device() in error handling path in sirfsoc_gpio_probe()
From: yu kuai @ 2020-06-02 12:29 UTC (permalink / raw)
To: baohua; +Cc: linux-gpio, yi.zhang, linux-kernel, linux-arm-kernel, yukuai3
in sirfsoc_gpio_probe(), if of_find_device_by_node() succeed,
put_device() is missing in the error handling patch.
Signed-off-by: yu kuai <yukuai3@huawei.com>
---
drivers/pinctrl/sirf/pinctrl-sirf.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
index 1ebcb957c654..63a287d5795f 100644
--- a/drivers/pinctrl/sirf/pinctrl-sirf.c
+++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
@@ -794,13 +794,17 @@ static int sirfsoc_gpio_probe(struct device_node *np)
return -ENODEV;
sgpio = devm_kzalloc(&pdev->dev, sizeof(*sgpio), GFP_KERNEL);
- if (!sgpio)
- return -ENOMEM;
+ if (!sgpio) {
+ err = -ENOMEM;
+ goto out_put_device;
+ }
spin_lock_init(&sgpio->lock);
regs = of_iomap(np, 0);
- if (!regs)
- return -ENOMEM;
+ if (!regs) {
+ err = -ENOMEM;
+ goto out_put_device;
+ }
sgpio->chip.gc.request = sirfsoc_gpio_request;
sgpio->chip.gc.free = sirfsoc_gpio_free;
@@ -824,8 +828,10 @@ static int sirfsoc_gpio_probe(struct device_node *np)
girq->parents = devm_kcalloc(&pdev->dev, SIRFSOC_GPIO_NO_OF_BANKS,
sizeof(*girq->parents),
GFP_KERNEL);
- if (!girq->parents)
- return -ENOMEM;
+ if (!girq->parents) {
+ err = -ENOMEM;
+ goto out_put_device;
+ }
for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
bank = &sgpio->sgpio_bank[i];
spin_lock_init(&bank->lock);
@@ -868,6 +874,8 @@ static int sirfsoc_gpio_probe(struct device_node *np)
gpiochip_remove(&sgpio->chip.gc);
out:
iounmap(regs);
+out_put_device:
+ put_device(&pdev->dev);
return err;
}
--
2.25.4
_______________________________________________
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 06/12] PM / devfreq: Add cpu based scaling support to passive_governor
From: andrew-sh.cheng @ 2020-06-02 12:23 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Mark Rutland, Nishanth Menon, srv_heupstream, devicetree,
Stephen Boyd, Viresh Kumar, Mark Brown, linux-pm,
Rafael J . Wysocki, Liam Girdwood, Rob Herring, linux-kernel,
Kyungmin Park, MyungJoo Ham, linux-mediatek, Sibi Sankar,
Matthias Brugger, linux-arm-kernel
In-Reply-To: <7ad35770-9327-084a-d2ca-1646cabd0a4d@samsung.com>
On Thu, 2020-05-28 at 16:17 +0900, Chanwoo Choi wrote:
> Hi Andrew-sh.Cheng,
>
> The exynos-bus.c used the passive governor.
> Even if don't make the problem because DEVFREQ_PARENT_DEV is zero,
> you need to initialize the parent_type with DEVFREQ_PARENT_DEV as following:
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 8fa8eb541373..1c71c47bc2ac 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -369,6 +369,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
> return -ENOMEM;
>
> passive_data->parent = parent_devfreq;
> + passive_data->parent_type = DEVFREQ_PARENT_DEV;
>
> /* Add devfreq device for exynos bus with passive governor */
> bus->devfreq = devm_devfreq_add_device(dev, profile, DEVFREQ_GOV_PASSIVE,
Hi Chanwoo Choi,
Do you just remind me to initialize it to DEVFREQ_PARENT_DEV whn use
this governor?
I will do it and thank you for reminding.
>
>
> On 5/28/20 3:14 PM, Chanwoo Choi wrote:
> > Hi Andrew-sh.Cheng,
> >
> > Thanks for your posting. I like this approach absolutely.
> > I think that it is necessary. When I developed the embedded product,
> > I needed this feature always.
> >
> > I add the comments on below.
> >
> >
> > And the following email is not valid. So, I dropped this email
> > from Cc list.
> > Saravana Kannan <skannan@codeaurora.org>
> >
> >
> > On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
> >> From: Saravana Kannan <skannan@codeaurora.org>
> >>
> >> Many CPU architectures have caches that can scale independent of the
> >> CPUs. Frequency scaling of the caches is necessary to make sure that the
> >> cache is not a performance bottleneck that leads to poor performance and
> >> power. The same idea applies for RAM/DDR.
> >>
> >> To achieve this, this patch adds support for cpu based scaling to the
> >> passive governor. This is accomplished by taking the current frequency
> >> of each CPU frequency domain and then adjust the frequency of the cache
> >> (or any devfreq device) based on the frequency of the CPUs. It listens
> >> to CPU frequency transition notifiers to keep itself up to date on the
> >> current CPU frequency.
> >>
> >> To decide the frequency of the device, the governor does one of the
> >> following:
> >> * Derives the optimal devfreq device opp from required-opps property of
> >> the parent cpu opp_table.
> >>
> >> * Scales the device frequency in proportion to the CPU frequency. So, if
> >> the CPUs are running at their max frequency, the device runs at its
> >> max frequency. If the CPUs are running at their min frequency, the
> >> device runs at its min frequency. It is interpolated for frequencies
> >> in between.
> >>
> >> Andrew-sh.Cheng change
> >> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
> >> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
> >> for kernel-5.7
> >>
> >> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> >> [Sibi: Integrated cpu-freqmap governor into passive_governor]
> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> >> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> >> ---
> >> drivers/devfreq/Kconfig | 2 +
> >> drivers/devfreq/governor_passive.c | 278 ++++++++++++++++++++++++++++++++++---
> >> include/linux/devfreq.h | 40 +++++-
> >> 3 files changed, 299 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> >> index 0b1df12e0f21..d9067950af6a 100644
> >> --- a/drivers/devfreq/Kconfig
> >> +++ b/drivers/devfreq/Kconfig
> >> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
> >> device. This governor does not change the frequency by itself
> >> through sysfs entries. The passive governor recommends that
> >> devfreq device uses the OPP table to get the frequency/voltage.
> >> + Alternatively the governor can also be chosen to scale based on
> >> + the online CPUs current frequency.
> >>
> >> comment "DEVFREQ Drivers"
> >>
> >> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
> >> index 2d67d6c12dce..7dcda02a5bb7 100644
> >> --- a/drivers/devfreq/governor_passive.c
> >> +++ b/drivers/devfreq/governor_passive.c
> >> @@ -8,11 +8,89 @@
> >> */
> >>
> >> #include <linux/module.h>
> >> +#include <linux/cpu.h>
> >> +#include <linux/cpufreq.h>
> >> +#include <linux/cpumask.h>
> >> #include <linux/device.h>
> >> #include <linux/devfreq.h>
> >> +#include <linux/slab.h>
> >> #include "governor.h"
> >>
> >> -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> >> +static unsigned int xlate_cpufreq_to_devfreq(struct devfreq_passive_data *data,
> >
> > Need to change 'unsigned int' to 'unsigned long'.
> >
> >> + unsigned int cpu)
> >> +{
> >> + unsigned int cpu_min, cpu_max, dev_min, dev_max, cpu_percent, max_state;
> >
> > Better to define them separately as following and then need to rename
> > the variable. Usually, use the 'min_freq' and 'max_freq' word for
> > the minimum/maximum frequency.
> >
> > unsigned int cpu_min_freq, cpu_max_freq, cpu_curr_freq, cpu_percent;
> > unsigned long dev_min_freq, dev_max_freq, dev_max_state,
> >
> > The devfreq used 'unsigned long'. The cpufreq used 'unsigned long'
> > and 'unsigned int'. You need to handle them properly.
> >
> >
> >> + struct devfreq_cpu_state *cpu_state = data->cpu_state[cpu];
> >> + struct devfreq *devfreq = (struct devfreq *)data->this;
> >> + unsigned long *freq_table = devfreq->profile->freq_table;
> >
> > In this function, use 'cpu' work for cpufreq and use 'dev' for devfreq.
> > So, I think 'dev_freq_table' is proper name instead of 'freq_table'
> > for the readability.
> >
> > freq_table -> dev_freq_table
> >
> >> + struct dev_pm_opp *opp = NULL, *cpu_opp = NULL;
> >
> > In the get_target_freq_with_devfreq(), use 'p_opp' indicating
> > the OPP of parent device. For the consistency, I think that
> > use 'p_opp' instead of 'cpu_opp'.
> >
> >> + unsigned long cpu_freq, freq;
> >
> > Define the 'cpu_freq' on above with cpu_min_freq/cpu_max_freq definition.
> > cpu_freq -> cpu_curr_freq.
> >
> >> +
> >> + if (!cpu_state || cpu_state->first_cpu != cpu ||
> >> + !cpu_state->opp_table || !devfreq->opp_table)
> >> + return 0;
> >> +
> >> + cpu_freq = cpu_state->freq * 1000;
> >> + cpu_opp = devfreq_recommended_opp(cpu_state->dev, &cpu_freq, 0);
> >> + if (IS_ERR(cpu_opp))
> >> + return 0;
> >> +
> >> + opp = dev_pm_opp_xlate_required_opp(cpu_state->opp_table,
> >> + devfreq->opp_table, cpu_opp);
> >> + dev_pm_opp_put(cpu_opp);
> >> +
> >> + if (!IS_ERR(opp)) {
> >> + freq = dev_pm_opp_get_freq(opp);
> >> + dev_pm_opp_put(opp);
> >
> > Better to add the 'out' goto statement.
> > If you use 'goto out', you can reduce the one indentation
> > without 'else' statement.
> >
> >
> >> + } else {
> >
> > As I commented, when dev_pm_opp_xlate_required_opp() return successfully
> > , use 'goto out'. We can remove 'else' and then reduce the unneeded indentation.
> >
> >
> >> + /* Use Interpolation if required opps is not available */
> >> + cpu_min = cpu_state->min_freq;
> >> + cpu_max = cpu_state->max_freq;
> >> + cpu_freq = cpu_state->freq;
> >> +
> >> + if (freq_table) {
> >> + /* Get minimum frequency according to sorting order */
> >> + max_state = freq_table[devfreq->profile->max_state - 1];
> >> + if (freq_table[0] < max_state) {
> >> + dev_min = freq_table[0];
> >> + dev_max = max_state;
> >> + } else {
> >> + dev_min = max_state;
> >> + dev_max = freq_table[0];
> >> + }
> >> + } else {
> >> + if (devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value
> >> + <= devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value)
> >> + return 0;
> >> + dev_min =
> >> + devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value;
> >> + dev_max =
> >> + devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value;
> >
> > I think it is not proper to access the variable of pm_qos structure directly.
> > Instead of direct access, you have to use the exported PM QoS function such as
> > - pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY);
> > - pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY);
> >
> >> + }
> >> + cpu_percent = ((cpu_freq - cpu_min) * 100) / cpu_max - cpu_min;
> >> + freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
> >> + }
> >
> >
> > I think that you better to add 'out' jump label as following:
> >
> > out:
> >
> >> +
> >> + return freq;
> >> +}
> >> +
> >> +static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
> >> + unsigned long *freq)
> >> +{
> >> + struct devfreq_passive_data *p_data =
> >> + (struct devfreq_passive_data *)devfreq->data;
> >> + unsigned int cpu, target_freq = 0;
> >
> > Need to define 'target_freq' with 'unsigned long' type.
> >
> >> +
> >> + for_each_online_cpu(cpu)
> >> + target_freq = max(target_freq,
> >> + xlate_cpufreq_to_devfreq(p_data, cpu));
> >> +
> >> + *freq = target_freq;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int get_target_freq_with_devfreq(struct devfreq *devfreq,
> >> unsigned long *freq)
> >> {
> >> struct devfreq_passive_data *p_data
> >> @@ -23,16 +101,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> >> int i, count, ret = 0;
> >>
> >> /*
> >> - * If the devfreq device with passive governor has the specific method
> >> - * to determine the next frequency, should use the get_target_freq()
> >> - * of struct devfreq_passive_data.
> >> - */
> >> - if (p_data->get_target_freq) {
> >> - ret = p_data->get_target_freq(devfreq, freq);
> >> - goto out;
> >> - }
> >> -
> >> - /*
> >> * If the parent and passive devfreq device uses the OPP table,
> >> * get the next frequency by using the OPP table.
> >> */
> >> @@ -102,6 +170,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> >> return ret;
> >> }
> >>
> >> +static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> >> + unsigned long *freq)
> >> +{
> >> + struct devfreq_passive_data *p_data =
> >> + (struct devfreq_passive_data *)devfreq->data;
> >> + int ret;
> >> +
> >> + /*
> >> + * If the devfreq device with passive governor has the specific method
> >> + * to determine the next frequency, should use the get_target_freq()
> >> + * of struct devfreq_passive_data.
> >> + */
> >> + if (p_data->get_target_freq)
> >> + return p_data->get_target_freq(devfreq, freq);
> >> +
> >> + switch (p_data->parent_type) {
> >> + case DEVFREQ_PARENT_DEV:
> >> + ret = get_target_freq_with_devfreq(devfreq, freq);
> >> + break;
> >> + case CPUFREQ_PARENT_DEV:
> >> + ret = get_target_freq_with_cpufreq(devfreq, freq);
> >> + break;
> >> + default:
> >> + ret = -EINVAL;
> >> + dev_err(&devfreq->dev, "Invalid parent type\n");
> >> + break;
> >> + }
> >> +
> >> + return ret;
> >> +}
> >> +
> >> static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq)
> >> {
> >> int ret;
> >> @@ -156,6 +255,140 @@ static int devfreq_passive_notifier_call(struct notifier_block *nb,
> >> return NOTIFY_DONE;
> >> }
> >>
> >> +static int cpufreq_passive_notifier_call(struct notifier_block *nb,
> >> + unsigned long event, void *ptr)
> >> +{
> >> + struct devfreq_passive_data *data =
> >> + container_of(nb, struct devfreq_passive_data, nb);
> >> + struct devfreq *devfreq = (struct devfreq *)data->this;
> >> + struct devfreq_cpu_state *cpu_state;
> >> + struct cpufreq_freqs *freq = ptr;
> >
> > How about changing 'freq' to 'cpu_freqs'?
> >
> > In the drivers/cpufreq/cpufreq.c, use 'freqs' name indicating
> > the instance of 'struct cpufreq_freqs'. And in order to
> > identfy, how about adding 'cpu_' prefix for variable name?
> >
> >> + unsigned int current_freq;
> >
> > Need to define curr_freq with 'unsigned long' type
> > and better to use 'curr_freq' variable name.
> >
> >> + int ret;
> >> +
> >> + if (event != CPUFREQ_POSTCHANGE || !freq ||
> >> + !data->cpu_state[freq->policy->cpu])
> >> + return 0;
> >> +
> >> + cpu_state = data->cpu_state[freq->policy->cpu];
> >> + if (cpu_state->freq == freq->new)
> >> + return 0;
> >> +
> >> + /* Backup current freq and pre-update cpu state freq*/
> >> + current_freq = cpu_state->freq;
> >> + cpu_state->freq = freq->new;
> >> +
> >> + mutex_lock(&devfreq->lock);
> >> + ret = update_devfreq(devfreq);
> >> + mutex_unlock(&devfreq->lock);
> >> + if (ret) {
> >> + cpu_state->freq = current_freq;
> >> + dev_err(&devfreq->dev, "Couldn't update the frequency.\n");
> >> + return ret;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int cpufreq_passive_register(struct devfreq_passive_data **p_data)
> >> +{
> >> + struct devfreq_passive_data *data = *p_data;
> >> + struct devfreq *devfreq = (struct devfreq *)data->this;
> >> + struct device *dev = devfreq->dev.parent;
> >> + struct opp_table *opp_table = NULL;
> >> + struct devfreq_cpu_state *state;
> >
> > For the readability, I thinkt 'cpu_state' is proper instead of 'state'.
> >
> >> + struct cpufreq_policy *policy;
> >> + struct device *cpu_dev;
> >> + unsigned int cpu;
> >> + int ret;
> >> +
> >> + get_online_cpus();
> >
> > Add blank line.
> >
> >> + data->nb.notifier_call = cpufreq_passive_notifier_call;
> >> + ret = cpufreq_register_notifier(&data->nb,
> >> + CPUFREQ_TRANSITION_NOTIFIER);
> >> + if (ret) {
> >> + dev_err(dev, "Couldn't register cpufreq notifier.\n");
> >> + data->nb.notifier_call = NULL;
> >> + goto out;
> >> + }
> >> +
> >> + /* Populate devfreq_cpu_state */
> >> + for_each_online_cpu(cpu) {
> >> + if (data->cpu_state[cpu])
> >> + continue;
> >> +
> >> + policy = cpufreq_cpu_get(cpu);
> >
> > cpufreq_cpu_get() might return 'NULL'. I think you need to handle
> > return value as following:
> >
> > if (!policy) {
> > ret = -EINVAL;
> > goto out;
> > } else if (PTR_ERR(policy) == -EPROBE_DEFER) {
> > goto out;
> > } else if (IS_ERR(policy) {
> > ret = PTR_ERR(policy);
> > dev_err(dev, "Couldn't get the cpufreq_poliy.\n");
> > goto out;
> > }
> >
> > If cpufreq_cpu_get() return successfully, to do next.
> > It reduces the one indentaion.
> >
> >
> >
> >> + if (policy) {
> >> + state = kzalloc(sizeof(*state), GFP_KERNEL);
> >> + if (!state) {
> >> + ret = -ENOMEM;
> >> + goto out;
> >> + }
> >> +
> >> + cpu_dev = get_cpu_device(cpu);
> >> + if (!cpu_dev) {
> >> + dev_err(dev, "Couldn't get cpu device.\n");
> >> + ret = -ENODEV;
> >> + goto out;
> >> + }
> >> +
> >> + opp_table = dev_pm_opp_get_opp_table(cpu_dev);
> >> + if (IS_ERR(devfreq->opp_table)) {
> >> + ret = PTR_ERR(opp_table);
> >> + goto out;
> >> + }
> >> +
> >> + state->dev = cpu_dev;
> >> + state->opp_table = opp_table;
> >> + state->first_cpu = cpumask_first(policy->related_cpus);
> >> + state->freq = policy->cur;
> >> + state->min_freq = policy->cpuinfo.min_freq;
> >> + state->max_freq = policy->cpuinfo.max_freq;
> >> + data->cpu_state[cpu] = state;
> >
> > Add blank line.
> >
> >> + cpufreq_cpu_put(policy);
> >> + } else {
> >> + ret = -EPROBE_DEFER;
> >> + goto out;
> >> + }
> >> + }
> >
> > Add blank line.
> >
> >> +out:
> >> + put_online_cpus();
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* Update devfreq */
> >> + mutex_lock(&devfreq->lock);
> >> + ret = update_devfreq(devfreq);
> >> + mutex_unlock(&devfreq->lock);
> >> + if (ret)
> >> + dev_err(dev, "Couldn't update the frequency.\n");
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int cpufreq_passive_unregister(struct devfreq_passive_data **p_data)
> >> +{
> >> + struct devfreq_passive_data *data = *p_data;
> >> + struct devfreq_cpu_state *cpu_state;
> >> + int cpu;
> >> +
> >> + if (data->nb.notifier_call)
> >> + cpufreq_unregister_notifier(&data->nb,
> >> + CPUFREQ_TRANSITION_NOTIFIER);
> >> +
> >> + for_each_possible_cpu(cpu) {
> >> + cpu_state = data->cpu_state[cpu];
> >> + if (cpu_state) {
> >> + if (cpu_state->opp_table)
> >> + dev_pm_opp_put_opp_table(cpu_state->opp_table);
> >> + kfree(cpu_state);
> >> + cpu_state = NULL;
> >> + }
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static int devfreq_passive_event_handler(struct devfreq *devfreq,
> >> unsigned int event, void *data)
> >> {
> >> @@ -165,7 +398,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
> >> struct notifier_block *nb = &p_data->nb;
> >> int ret = 0;
> >>
> >> - if (!parent)
> >> + if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
> >> return -EPROBE_DEFER;
> >
> > If you modify the devfreq_passive_event_handler() as following,
> > you can move this condition for DEVFREQ_PARENT_DEV into
> > (register|unregister)_parent_dev_notifier.
> >
> > switch (event) {
> > case DEVFREQ_GOV_START:
> > ret = register_parent_dev_notifier(p_data);
> > break;
> > case DEVFREQ_GOV_STOP:
> > ret = unregister_parent_dev_notifier(p_data);
> > break;
> > default:
> > ret = -EINVAL;
> > break;
> > }
> >
> > return ret;
> >
> >>
> >> switch (event) {
> >> @@ -173,13 +406,24 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
> >> if (!p_data->this)
> >> p_data->this = devfreq;
> >>
> >> - nb->notifier_call = devfreq_passive_notifier_call;
> >> - ret = devfreq_register_notifier(parent, nb,
> >> - DEVFREQ_TRANSITION_NOTIFIER);
> >> + if (p_data->parent_type == DEVFREQ_PARENT_DEV) {
> >> + nb->notifier_call = devfreq_passive_notifier_call;
> >> + ret = devfreq_register_notifier(parent, nb,
> >> + DEVFREQ_TRANSITION_NOTIFIER);
> >> + } else if (p_data->parent_type == CPUFREQ_PARENT_DEV) {
> >> + ret = cpufreq_passive_register(&p_data);
> >
> > I think that we better to collect the code related to notifier registration
> > into one function like devfreq_pass_register_notifier() instead of
> > cpufreq_passive_register() as following: I think it is more simple and readable.
> >
> > If you have more proper function name of register_parent_dev_notifier,
> > please give your opinion.
> >
> >
> > int register_parent_dev_notifier(struct devfreq_passive_data **p_data)
> > switch (p_data->parent_type) {
> > case DEVFREQ_PARENT_DEV:
> > nb->notifier_call = devfreq_passive_notifier_call;
> > ret = devfreq_register_notifier(parent, nb,
> > break;
> > case CPUFREQ_PARENT_DEV:
> > cpufreq_register_notifier(...)
> > ...
> > break;
> > }
> >
> >
> >> + } else {
> >> + ret = -EINVAL;
> >> + }
> >> break;
> >> case DEVFREQ_GOV_STOP:
> >> - WARN_ON(devfreq_unregister_notifier(parent, nb,
> >> - DEVFREQ_TRANSITION_NOTIFIER));
> >> + if (p_data->parent_type == DEVFREQ_PARENT_DEV)
> >> + WARN_ON(devfreq_unregister_notifier(parent, nb,
> >> + DEVFREQ_TRANSITION_NOTIFIER));
> >> + else if (p_data->parent_type == CPUFREQ_PARENT_DEV)
> >> + cpufreq_passive_unregister(&p_data);
> >> + else
> >> + ret = -EINVAL;
> >
> > ditto. unregister_parent_dev_notifier(struct devfreq_passive_data **p_data)
> >
> >> break;
> >> default:
> >> break;
> >> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> >> index a4b19d593151..04ce576fd6f1 100644
> >> --- a/include/linux/devfreq.h
> >> +++ b/include/linux/devfreq.h
> >> @@ -278,6 +278,32 @@ struct devfreq_simple_ondemand_data {
> >>
> >> #if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE)
> >> /**
> >> + * struct devfreq_cpu_state - holds the per-cpu state
> >> + * @freq: the current frequency of the cpu.
> >> + * @min_freq: the min frequency of the cpu.
> >> + * @max_freq: the max frequency of the cpu.
> >> + * @first_cpu: the cpumask of the first cpu of a policy.
> >> + * @dev: reference to cpu device.
> >> + * @opp_table: reference to cpu opp table.
> >> + *
> >> + * This structure stores the required cpu_state of a cpu.
> >> + * This is auto-populated by the governor.
> >> + */
> >> +struct devfreq_cpu_state {> + unsigned int freq;
> >
> > It is better to change from 'freq' to 'curr_freq'
> > for more correct expression.
> >
> >> + unsigned int min_freq;
> >> + unsigned int max_freq;
> >> + unsigned int first_cpu;
> >> + struct device *dev;
> >
> > How about changing the name 'dev' to 'cpu_dev'?
> >
> >
> >> + struct opp_table *opp_table;
> >> +};
> >
> > devfreq_cpu_state is only handled by within driver/devfreq/governor_passive.c.
> >
> > So, you can move it into drivers/devfreq/governor_passive.c
> > and just add the definition into include/linux/devfreq.h as following:
> > It is able to prevent the access of variable of 'struct devfreq_cpu_state'
> > outside.
> >
> > struct devfreq_cpu_state;
> >
> >> +
> >> +enum devfreq_parent_dev_type {
> >> + DEVFREQ_PARENT_DEV,
> >> + CPUFREQ_PARENT_DEV,
> >> +};
> >> +
> >> +/**
> >> * struct devfreq_passive_data - ``void *data`` fed to struct devfreq
> >> * and devfreq_add_device
> >> * @parent: the devfreq instance of parent device.
> >> @@ -288,13 +314,15 @@ struct devfreq_simple_ondemand_data {
> >> * using governors except for passive governor.
> >> * If the devfreq device has the specific method to decide
> >> * the next frequency, should use this callback.
> >> - * @this: the devfreq instance of own device.
> >> - * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
> >> + * @parent_type parent type of the device
> >
> > Need to add ':' at the end of word. -> "parent_type:".
> >
> >> + * @this: the devfreq instance of own device.
> >> + * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
> >
> > I knew that you make them with same indentation.
> > But, actually, it is not related to this patch like clean-up code.
> > Even if it is not pretty, you better to don't touch 'this' and 'nb' indentaion.
> >
> >> + * @cpu_state: the state min/max/current frequency of all online cpu's
> >> *
> >> * The devfreq_passive_data have to set the devfreq instance of parent
> >> * device with governors except for the passive governor. But, don't need to
> >> - * initialize the 'this' and 'nb' field because the devfreq core will handle
> >> - * them.
> >> + * initialize the 'this', 'nb' and 'cpu_state' field because the devfreq core
> >> + * will handle them.
> >> */
> >> struct devfreq_passive_data {
> >> /* Should set the devfreq instance of parent device */
> >> @@ -303,9 +331,13 @@ struct devfreq_passive_data {
> >> /* Optional callback to decide the next frequency of passvice device */
> >> int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
> >>
> >> + /* Should set the type of parent device */
> >> + enum devfreq_parent_dev_type parent_type;
> >> +
> >> /* For passive governor's internal use. Don't need to set them */
> >> struct devfreq *this;
> >> struct notifier_block nb;
> >> + struct devfreq_cpu_state *cpu_state[NR_CPUS];
> >> };
> >> #endif
> >>
> >>
> >
> >
>
>
_______________________________________________
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 0/2] ARM: Allow either FLATMEM or SPARSEMEM on the multiplatform build
From: Geert Uytterhoeven @ 2020-06-02 12:37 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Florian Fainelli, Arnd Bergmann, Stephen Boyd, Kevin Cernekee,
Mike Rapoport, Doug Berger, Gregory Fong, Linux ARM
In-Reply-To: <20200602122247.GV1551@shell.armlinux.org.uk>
Hi Russell,
On Tue, Jun 2, 2020 at 2:22 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
> On Tue, Jun 02, 2020 at 02:18:48PM +0200, Geert Uytterhoeven wrote:
> > On Thu, May 21, 2020 at 4:50 PM Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > > On Thu, May 21, 2020 at 05:07:45PM +0300, Mike Rapoport wrote:
> > > > Ah, when either of these patforms will become a part of the
> > > > multiplatform build, the only option for multiplatform build will be
> > > > sparsemem.
> > > > So it would be nice if somebody could check the cost of using sparsemem
> > > > vs flatmem, espessially on low end machines.
> > >
> > > Do you think they will become part of multiplatform?
> > >
> > > If they're low-end machines, then adding:
> > >
> > > (a) the additional memory overhead of a multiplatform kernel
> > > (b) the additional runtime overhead of the complexities of multiplatform
> > > kernels
> > >
> > > is surely an odd thing to do, especially when few really care about
> > > these platforms becoming part of a multiplatform kernel, except those
> > > who like the idea of multiplat.
> >
> > The fallacy of "multi-platform", again?
> >
> > Isn't it a requirement for any new ARM v7-A platforms to be part of
> > ARCH_MULTI_V7, even if it's a low-end machine?
>
> What does ARMv7 have to do with this thread, that is discussing the
> older ARMv4 platforms? I find your comments above irrelevent to
> this discussion, and seems to be worded to provoke a reaction.
Sorry, I used ARMv7 as an example, as it's rare for any new ARMv4
platforms to show up. But the recently introduced sam9x60 (ARMv5) is
part of ARCH_MULTI_V5, and I expect it will be used with less than the
256 MiB provided on the dev board.
Nevertheless, there's a movement to convert everything to ARCH_MULTI_V*
where possible. So it matters for all variants.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
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] pwm: rockchip: simplify rockchip_pwm_get_state()
From: Thierry Reding @ 2020-06-02 12:39 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: linux-pwm, Heiko Stuebner, linux-kernel, linux-rockchip, David Wu,
linux-arm-kernel
In-Reply-To: <20190919091728.24756-1-linux@rasmusvillemoes.dk>
[-- Attachment #1.1: Type: text/plain, Size: 896 bytes --]
On Thu, Sep 19, 2019 at 11:17:27AM +0200, Rasmus Villemoes wrote:
> The way state->enabled is computed is rather convoluted and hard to
> read - both branches of the if() actually do the exact same thing. So
> remove the if(), and further simplify "<boolean condition> ? true :
> false" to "<boolean condition>".
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> I stumbled on this while trying to understand how the pwm subsystem
> works. This patch is a semantic no-op, but it's also possible that,
> say, the first branch simply contains a "double negative" so either
> the != should be == or the "false : true" should be "true : false".
>
> drivers/pwm/pwm-rockchip.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
I've applied this. Irrespective of any feedback David would have this is
correct and a nice simplification.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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] pwm: sun4i: direct clock output support for Allwinner A64
From: Thierry Reding @ 2020-06-02 12:40 UTC (permalink / raw)
To: Peter Vasil
Cc: linux-pwm, nightwolf, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
Uwe Kleine-König, linux-arm-kernel
In-Reply-To: <20200428164150.366966-1-peter.vasil@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 384 bytes --]
On Tue, Apr 28, 2020 at 06:41:50PM +0200, Peter Vasil wrote:
> Allwinner A64 is capable of a direct clock output on PWM (see A64
> User Manual chapter 3.10). Add support for this in the sun4i PWM
> driver.
>
> Signed-off-by: Peter Vasil <peter.vasil@gmail.com>
> ---
> drivers/pwm/pwm-sun4i.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
Applied, thanks.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend
From: Takashi Iwai @ 2020-06-02 12:46 UTC (permalink / raw)
To: Macpaul Lin
Cc: alsa-devel, Mediatek WSD Upstream, linux-kernel, linux-usb,
Johan Hovold, Takashi Iwai, Hui Wang, Alexander Tsoy,
linux-mediatek, Matthias Brugger, Macpaul Lin, Jaroslav Kysela,
Szabolcs Szőke, linux-arm-kernel
In-Reply-To: <1591098821-17910-1-git-send-email-macpaul.lin@mediatek.com>
On Tue, 02 Jun 2020 13:53:41 +0200,
Macpaul Lin wrote:
>
> This patch fix incorrect power state changed by usb_audio_suspend()
> when CONFIG_PM is enabled.
>
> After receiving suspend PM message with auto flag, usb_audio_suspend()
> change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other
> resume PM message with auto flag can change power state to
> SNDRV_CTL_POWER_D0 in __usb_audio_resume().
>
> However, when system is not under auto suspend, resume PM message with
> auto flag might not be able to receive on time which cause the power
> state was incorrect. At this time, if a player starts to play sound,
> will cause snd_usb_pcm_open() to access the card and setup_hw_info() will
> resume the card.
>
> But even the card is back to work and all function normal, the power
> state is still in SNDRV_CTL_POWER_D3hot.
Hm, in exactly which situation does this happen? I still don't get
it. Could you elaborate how to trigger this?
> Which cause the infinite loop
> happened in snd_power_wait() to check the power state. Thus the
> successive setting ioctl cannot be passed to card.
>
> Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card
> has been resumed successfully.
This doesn't look like a right solution for the problem, sorry.
The card PM status must be recovered to D0 when the autoresume
succeeds. If not, something is broken there, and it must be fixed
instead of fiddling the status flag externally.
thanks,
Takashi
>
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
> sound/usb/pcm.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index a4e4064..d667ecb 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -1322,6 +1322,17 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre
> if (err < 0)
> return err;
>
> + /* fix incorrect power state when resuming by open and later ioctls */
> + if (IS_ENABLED(CONFIG_PM) &&
> + snd_power_get_state(subs->stream->chip->card)
> + == SNDRV_CTL_POWER_D3hot) {
> + /* set these variables for power state correction */
> + subs->stream->chip->autosuspended = 0;
> + subs->stream->chip->num_suspended_intf = 1;
> + dev_info(&subs->dev->dev,
> + "change power state from D3hot to D0\n");
> + }
> +
> return snd_usb_autoresume(subs->stream->chip);
> }
>
> --
> 1.7.9.5
_______________________________________________
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] pwm: imx27: Fix rounding behavior
From: Thierry Reding @ 2020-06-02 12:48 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-pwm, Shawn Guo, Sascha Hauer, NXP Linux Team,
Pengutronix Kernel Team, Fabio Estevam, linux-arm-kernel
In-Reply-To: <20200416080245.3203-1-u.kleine-koenig@pengutronix.de>
[-- Attachment #1.1: Type: text/plain, Size: 556 bytes --]
On Thu, Apr 16, 2020 at 10:02:45AM +0200, Uwe Kleine-König wrote:
> To not trigger the warnings provided by CONFIG_PWM_DEBUG
>
> - use up-rounding in .get_state()
> - don't divide by the result of a division
> - don't use the rounded counter value for the period length to calculate
> the counter value for the duty cycle
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> drivers/pwm/pwm-imx27.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
Applied, thanks.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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 015/105] drm/vc4: hvs: Boost the core clock during modeset
From: Maxime Ripard @ 2020-06-02 12:52 UTC (permalink / raw)
To: Eric Anholt
Cc: Tim Gover, Dave Stevenson, linux-kernel, DRI Development,
bcm-kernel-feedback-list, Nicolas Saenz Julienne, Phil Elwell,
linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <CADaigPWQdeTd2CGCK-yxq+TAU6xKMVsdZfhSVptn4RSENxpdxg@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 956 bytes --]
Hi Eric,
On Wed, May 27, 2020 at 09:33:44AM -0700, Eric Anholt wrote:
> On Wed, May 27, 2020 at 8:49 AM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > In order to prevent timeouts and stalls in the pipeline, the core clock
> > needs to be maxed at 500MHz during a modeset on the BCM2711.
>
> Like, the whole system's core clock?
Yep, unfortunately...
> How is it reasonable for some device driver to crank the system's core
> clock up and back down to some fixed-in-the-driver frequency? Sounds
> like you need some sort of opp thing here.
That frequency is the minimum rate of that clock. However, since other
devices have similar requirements (unicam in particular) with different
minimum requirements, we will switch to setting a minimum rate instead
of enforcing a particular rate, so that patch would be essentially
s/clk_set_rate/clk_set_min_rate/.
Would that work for you?
>
> Patch 13,14 r-b.
Thanks!
Maxime
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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 0/2] ARM: Allow either FLATMEM or SPARSEMEM on the multiplatform build
From: Russell King - ARM Linux admin @ 2020-06-02 12:56 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Florian Fainelli, Arnd Bergmann, Stephen Boyd, Kevin Cernekee,
Mike Rapoport, Doug Berger, Gregory Fong, Linux ARM
In-Reply-To: <CAMuHMdUxdPML=eS59M_KRvpbAiKmLptf3ngfhhiKRyNYgpKt2Q@mail.gmail.com>
On Tue, Jun 02, 2020 at 02:37:45PM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
>
> On Tue, Jun 2, 2020 at 2:22 PM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> > On Tue, Jun 02, 2020 at 02:18:48PM +0200, Geert Uytterhoeven wrote:
> > > On Thu, May 21, 2020 at 4:50 PM Russell King - ARM Linux admin
> > > <linux@armlinux.org.uk> wrote:
> > > > On Thu, May 21, 2020 at 05:07:45PM +0300, Mike Rapoport wrote:
> > > > > Ah, when either of these patforms will become a part of the
> > > > > multiplatform build, the only option for multiplatform build will be
> > > > > sparsemem.
> > > > > So it would be nice if somebody could check the cost of using sparsemem
> > > > > vs flatmem, espessially on low end machines.
> > > >
> > > > Do you think they will become part of multiplatform?
> > > >
> > > > If they're low-end machines, then adding:
> > > >
> > > > (a) the additional memory overhead of a multiplatform kernel
> > > > (b) the additional runtime overhead of the complexities of multiplatform
> > > > kernels
> > > >
> > > > is surely an odd thing to do, especially when few really care about
> > > > these platforms becoming part of a multiplatform kernel, except those
> > > > who like the idea of multiplat.
> > >
> > > The fallacy of "multi-platform", again?
> > >
> > > Isn't it a requirement for any new ARM v7-A platforms to be part of
> > > ARCH_MULTI_V7, even if it's a low-end machine?
> >
> > What does ARMv7 have to do with this thread, that is discussing the
> > older ARMv4 platforms? I find your comments above irrelevent to
> > this discussion, and seems to be worded to provoke a reaction.
>
> Sorry, I used ARMv7 as an example, as it's rare for any new ARMv4
> platforms to show up. But the recently introduced sam9x60 (ARMv5) is
> part of ARCH_MULTI_V5, and I expect it will be used with less than the
> 256 MiB provided on the dev board.
>
> Nevertheless, there's a movement to convert everything to ARCH_MULTI_V*
> where possible. So it matters for all variants.
First, please understand that I have nothing against multiplatform.
What I do have problems with is the "lets move everything to
multiplatform" without fully reasoning it out, especially when it
comes to older platforms that have limited attraction for the whole
motivation behind multiplatform, which is the convenience of
distributions.
multiplatform is the current fad-de-jour, and everyone is in a "you
MUST convert EVERYTHING to it, because it's the best thing since
sliced bread".
multiplatform comes with it a load of forced-enabled options that
are force-enabled. Should RiscPC be forced to have the common clock
support built into the kernel, when it doesn't' have _any_ controllable
clocks, when it's likely that the machines have limited memory
available? Yes, I have recently booted the RiscPC with a 5.x kernel,
and no, it didn't need very much fixing, the code is quite *stable*
(oh, I blaspheme, we can't have stable code in Linux, we must change
it every five minutes.)
I think, maybe, you should leave these decisions to people who actually
have the platforms, know them inside out, and can assess whether
conversion really makes sense or not, rather than blindly joining the
zombie march repeating "multiplatform is great, everything must be
multiplatform".
What is patently obvious is that in Linux, there is no tolerance of
someone who has a differing view, and they end up receiving reaction
provoking comments designed to inflame the situation. Difference of
opinion must be suppressed at all costs!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [Linux-stm32] [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check
From: Emil Velikov @ 2020-06-02 12:53 UTC (permalink / raw)
To: Adrian Ratiu
Cc: devicetree@vger.kernel.org, Jernej Skrabec, Benjamin GAIGNARD,
Adrian Pop, Jonas Karlman, Philippe CORNU,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Yannick FERTRE, linux-rockchip@lists.infradead.org, Andrzej Hajda,
Laurent Pinchart, Arnaud Ferraris, kernel@collabora.com,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, linux-imx@nxp.com
In-Reply-To: <87blm387vt.fsf@iwork.i-did-not-set--mail-host-address--so-tickle-me>
Hi Adrian,
On Mon, 1 Jun 2020 at 10:14, Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
>
> On Fri, 29 May 2020, Philippe CORNU <philippe.cornu@st.com> wrote:
> > Hi Adrian, and thank you very much for the patchset. Thank you
> > also for having tested it on STM32F769 and STM32MP1. Sorry for
> > the late response, Yannick and I will review it as soon as
> > possible and we will keep you posted. Note: Do not hesitate to
> > put us in copy for the next version (philippe.cornu@st.com,
> > yannick.fertre@st.com) Regards, Philippe :-)
>
> Hi Philippe,
>
> Thank you very much for your previous and future STM testing,
> really appreciate it! I've CC'd Yannick until now but I'll also CC
> you sure :)
>
> It's been over a month since I posted v8 and I was just gearing up
> to address all feedback, rebase & retest to prepare v9 but I'll
> wait a little longer, no problem, it's no rush.
>
Small idea, pardon for joining so late:
Might be a good idea to add inline comment, why the clocks are disabled so late.
Effectively a 2 line version of the commit summary.
Feel free to make that a separate/follow-up patch.
-Emil
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: Security Random Number Generator support
From: Marc Zyngier @ 2020-06-02 13:02 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman, Sean Wang, lkml,
wsd_upstream, Crystal Guo, Rob Herring, Neal Liu,
Linux Crypto Mailing List, Matt Mackall, Matthias Brugger,
linux-mediatek, Linux ARM
In-Reply-To: <CAMj1kXHjAdk5=-uSh_=S9j5cz42zr3h6t+YYGy+obevuQDp0fg@mail.gmail.com>
On 2020-06-02 13:14, Ard Biesheuvel wrote:
> On Tue, 2 Jun 2020 at 10:15, Neal Liu <neal.liu@mediatek.com> wrote:
>>
>> These patch series introduce a security random number generator
>> which provides a generic interface to get hardware rnd from Secure
>> state. The Secure state can be Arm Trusted Firmware(ATF), Trusted
>> Execution Environment(TEE), or even EL2 hypervisor.
>>
>> Patch #1..2 adds sec-rng kernel driver for Trustzone based SoCs.
>> For security awareness SoCs on ARMv8 with TrustZone enabled,
>> peripherals like entropy sources is not accessible from normal world
>> (linux) and rather accessible from secure world (HYP/ATF/TEE) only.
>> This driver aims to provide a generic interface to Arm Trusted
>> Firmware or Hypervisor rng service.
>>
>>
>> changes since v1:
>> - rename mt67xx-rng to mtk-sec-rng since all MediaTek ARMv8 SoCs can
>> reuse
>> this driver.
>> - refine coding style and unnecessary check.
>>
>> changes since v2:
>> - remove unused comments.
>> - remove redundant variable.
>>
>> changes since v3:
>> - add dt-bindings for MediaTek rng with TrustZone enabled.
>> - revise HWRNG SMC call fid.
>>
>> changes since v4:
>> - move bindings to the arm/firmware directory.
>> - revise driver init flow to check more property.
>>
>> changes since v5:
>> - refactor to more generic security rng driver which
>> is not platform specific.
>>
>> *** BLURB HERE ***
>>
>> Neal Liu (2):
>> dt-bindings: rng: add bindings for sec-rng
>> hwrng: add sec-rng driver
>>
>
> There is no reason to model a SMC call as a driver, and represent it
> via a DT node like this.
+1.
> It would be much better if this SMC interface is made truly generic,
> and wired into the arch_get_random() interface, which can be used much
> earlier.
Wasn't there a plan to standardize a SMC call to rule them all?
M.
--
Who you jivin' with that Cosmik Debris?
_______________________________________________
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 v4 4/5] coresight: etm: perf: Add default sink selection to etm perf
From: Mike Leach @ 2020-06-02 13:12 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: Coresight ML, Arnaldo Carvalho de Melo, Mathieu Poirier,
linux-arm-kernel
In-Reply-To: <51fcc1b5-a4ab-04d1-e395-95df9f4745f7@arm.com>
Hi Suzuki,
On Tue, 2 Jun 2020 at 12:40, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> On 05/26/2020 11:46 AM, Mike Leach wrote:
> > Add default sink selection to the perf trace handling in the etm driver.
> > Uses the select default sink infrastructure to select a sink for the perf
> > session, if no other sink is specified.
> >
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
>
> This patch looks fine to me as such. But please see below for some
> discussion on the future support for other configurations.
>
>
> > ---
> > .../hwtracing/coresight/coresight-etm-perf.c | 17 ++++++++++++++---
> > 1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> > index 84f1dcb69827..1a3169e69bb1 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> > @@ -226,9 +226,6 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
> > sink = coresight_get_enabled_sink(true);
> > }
> >
> > - if (!sink)
> > - goto err;
> > -
> > mask = &event_data->mask;
> >
> > /*
> > @@ -253,6 +250,16 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
> > continue;
> > }
> >
> > + /*
> > + * No sink provided - look for a default sink for one of the
> > + * devices. At present we only support topology where all CPUs
> > + * use the same sink [N:1], so only need to find one sink. The
> > + * coresight_build_path later will remove any CPU that does not
> > + * attach to the sink, or if we have not found a sink.
> > + */
> > + if (!sink)
> > + sink = coresight_find_default_sink(csdev);
> > +
>
> While we are here, should we remove the "find enabled sink" if the csink
> is not specified via config. ? That step is problematic, as the user may
> not remember which sinks were enabled. Also, we can't hit that with
> perf tool as it prevents any invocation without sink (until this change).
>
> So may be this is a good time to get rid of that ?
>
You are correct - the 'sink = coresight_get_enabled_sink(true);' was
dead code until this patch.
However - if someone has set up their system using sysfs to enable
sinks, then should we not respect that rather than assume they made a
mistake?
Thinking about N:M topologies mentioned below - one method of handling
this is to enable relevant sinks and then let perf trace on any cores
that might use them.
> Also, we may need to do special handling for cases where there multiple
> sinks (ETRS) and the cpus in the event mask has different preferred
> sink. We can defer it for now as we don't claim to support such
> configurations yet.
Yes - the newer topologies will need some changes - beyond what we are
handling here.
However - especially for 1:1 - the best way may be to always use the
default sink - as specifying multiple sinks on the perf command line
may be problematical.
> When we do, we could either :
>
> 1) Make sure the event is bound to a single CPU, in which case
> the sink remains the same for the event.
>
> OR
>
> 2) All the different "preferred" sinks (ETRs selected by the ETM) have
> the same capabilitiy. i.e, we can move around the "sink" specific
> buffers and use them where we end up using.
If here by "capabilities" we are talking about buffer vs system memory
type sinks then I agree. We may need in future to limit the search
algorithm to ensure that only the best system memory type sinks are
found and eliminate cores attached to only buffer type sinks.
>
> We can defer all of this, until we get platforms which ned the support.
>
> Suzuki
Thanks for the review.
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
_______________________________________________
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 v4 4/5] coresight: etm: perf: Add default sink selection to etm perf
From: Suzuki K Poulose @ 2020-06-02 13:29 UTC (permalink / raw)
To: mike.leach; +Cc: coresight, acme, mathieu.poirier, linux-arm-kernel
In-Reply-To: <CAJ9a7ViUVAttf3-Mb4zVeJ6+Ty=4yxg3MZeGPcGDc0UMVY_Xtg@mail.gmail.com>
On 06/02/2020 02:12 PM, Mike Leach wrote:
> Hi Suzuki,
>
> On Tue, 2 Jun 2020 at 12:40, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>>
>> On 05/26/2020 11:46 AM, Mike Leach wrote:
>>> Add default sink selection to the perf trace handling in the etm driver.
>>> Uses the select default sink infrastructure to select a sink for the perf
>>> session, if no other sink is specified.
>>>
>>> Signed-off-by: Mike Leach <mike.leach@linaro.org>
>>
>> This patch looks fine to me as such. But please see below for some
>> discussion on the future support for other configurations.
>>
>>
>>> ---
>>> .../hwtracing/coresight/coresight-etm-perf.c | 17 ++++++++++++++---
>>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> index 84f1dcb69827..1a3169e69bb1 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> @@ -226,9 +226,6 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
>>> sink = coresight_get_enabled_sink(true);
>>> }
>>>
>>> - if (!sink)
>>> - goto err;
>>> -
>>> mask = &event_data->mask;
>>>
>>> /*
>>> @@ -253,6 +250,16 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
>>> continue;
>>> }
>>>
>>> + /*
>>> + * No sink provided - look for a default sink for one of the
>>> + * devices. At present we only support topology where all CPUs
>>> + * use the same sink [N:1], so only need to find one sink. The
>>> + * coresight_build_path later will remove any CPU that does not
>>> + * attach to the sink, or if we have not found a sink.
>>> + */
>>> + if (!sink)
>>> + sink = coresight_find_default_sink(csdev);
>>> +
>>
>> While we are here, should we remove the "find enabled sink" if the csink
>> is not specified via config. ? That step is problematic, as the user may
>> not remember which sinks were enabled. Also, we can't hit that with
>> perf tool as it prevents any invocation without sink (until this change).
>>
>> So may be this is a good time to get rid of that ?
>>
>
> You are correct - the 'sink = coresight_get_enabled_sink(true);' was
> dead code until this patch.
> However - if someone has set up their system using sysfs to enable
> sinks, then should we not respect that rather than assume they made a
> mistake?
If someone really wants to use a specific sink, then they could always
specify it via the config attribute and it will be honoured. We need not
carry along this non-intuitive hinting.
>
> Thinking about N:M topologies mentioned below - one method of handling
> this is to enable relevant sinks and then let perf trace on any cores
> that might use them.
>
>> Also, we may need to do special handling for cases where there multiple
>> sinks (ETRS) and the cpus in the event mask has different preferred
>> sink. We can defer it for now as we don't claim to support such
>> configurations yet.
>
> Yes - the newer topologies will need some changes - beyond what we are
> handling here.
> However - especially for 1:1 - the best way may be to always use the
> default sink - as specifying multiple sinks on the perf command line
> may be problematical.
>
>> When we do, we could either :
>>
>> 1) Make sure the event is bound to a single CPU, in which case
>> the sink remains the same for the event.
>>
>> OR
>>
>> 2) All the different "preferred" sinks (ETRs selected by the ETM) have
>> the same capabilitiy. i.e, we can move around the "sink" specific
>> buffers and use them where we end up using.
>
> If here by "capabilities" we are talking about buffer vs system memory
> type sinks then I agree. We may need in future to limit the search
Not necessarily. e.g, if we ever get two different types of system
memory sinks, (e.g, a global ETR and a dedicate "new" sink for a
cluster), we can't keep switching between the two sinks depending on how
they use the buffers. (i.e, direct buffer vs double copy)
Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] dt-bindings: clock: Convert imx7ulp clock to json-schema
From: Anson Huang @ 2020-06-02 13:16 UTC (permalink / raw)
To: mturquette, sboyd, robh+dt, shawnguo, s.hauer, kernel, festevam,
aisheng.dong, linux-clk, devicetree, linux-arm-kernel,
linux-kernel
Cc: Linux-imx
Convert the i.MX7ULP clock binding to DT schema format using json-schema,
the original binding doc is actually for two clock modules(SCG and PCC),
so split it to two binding docs, and the MPLL(mipi PLL) is NOT supposed
to be in clock module, so remove it from binding doc as well.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
.../devicetree/bindings/clock/imx7ulp-clock.txt | 103 ------------------
.../bindings/clock/imx7ulp-pcc-clock.yaml | 119 +++++++++++++++++++++
.../bindings/clock/imx7ulp-scg-clock.yaml | 97 +++++++++++++++++
3 files changed, 216 insertions(+), 103 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-pcc-clock.yaml
create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-scg-clock.yaml
diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
deleted file mode 100644
index 93d89ad..0000000
--- a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
+++ /dev/null
@@ -1,103 +0,0 @@
-* Clock bindings for Freescale i.MX7ULP
-
-i.MX7ULP Clock functions are under joint control of the System
-Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
-modules, and Core Mode Controller (CMC)1 blocks
-
-The clocking scheme provides clear separation between M4 domain
-and A7 domain. Except for a few clock sources shared between two
-domains, such as the System Oscillator clock, the Slow IRC (SIRC),
-and and the Fast IRC clock (FIRCLK), clock sources and clock
-management are separated and contained within each domain.
-
-M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
-A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
-
-Note: this binding doc is only for A7 clock domain.
-
-System Clock Generation (SCG) modules:
----------------------------------------------------------------------
-The System Clock Generation (SCG) is responsible for clock generation
-and distribution across this device. Functions performed by the SCG
-include: clock reference selection, generation of clock used to derive
-processor, system, peripheral bus and external memory interface clocks,
-source selection for peripheral clocks and control of power saving
-clock gating mode.
-
-Required properties:
-
-- compatible: Should be "fsl,imx7ulp-scg1".
-- reg : Should contain registers location and length.
-- #clock-cells: Should be <1>.
-- clocks: Should contain the fixed input clocks.
-- clock-names: Should contain the following clock names:
- "rosc", "sosc", "sirc", "firc", "upll", "mpll".
-
-Peripheral Clock Control (PCC) modules:
----------------------------------------------------------------------
-The Peripheral Clock Control (PCC) is responsible for clock selection,
-optional division and clock gating mode for peripherals in their
-respected power domain
-
-Required properties:
-- compatible: Should be one of:
- "fsl,imx7ulp-pcc2",
- "fsl,imx7ulp-pcc3".
-- reg : Should contain registers location and length.
-- #clock-cells: Should be <1>.
-- clocks: Should contain the fixed input clocks.
-- clock-names: Should contain the following clock names:
- "nic1_bus_clk", "nic1_clk", "ddr_clk", "apll_pfd2",
- "apll_pfd1", "apll_pfd0", "upll", "sosc_bus_clk",
- "mpll", "firc_bus_clk", "rosc", "spll_bus_clk";
-
-The clock consumer should specify the desired clock by having the clock
-ID in its "clocks" phandle cell.
-See include/dt-bindings/clock/imx7ulp-clock.h
-for the full list of i.MX7ULP clock IDs of each module.
-
-Examples:
-
-#include <dt-bindings/clock/imx7ulp-clock.h>
-
-scg1: scg1@403e0000 {
- compatible = "fsl,imx7ulp-scg1;
- reg = <0x403e0000 0x10000>;
- clocks = <&rosc>, <&sosc>, <&sirc>,
- <&firc>, <&upll>, <&mpll>;
- clock-names = "rosc", "sosc", "sirc",
- "firc", "upll", "mpll";
- #clock-cells = <1>;
-};
-
-pcc2: pcc2@403f0000 {
- compatible = "fsl,imx7ulp-pcc2";
- reg = <0x403f0000 0x10000>;
- #clock-cells = <1>;
- clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
- <&scg1 IMX7ULP_CLK_NIC1_DIV>,
- <&scg1 IMX7ULP_CLK_DDR_DIV>,
- <&scg1 IMX7ULP_CLK_APLL_PFD2>,
- <&scg1 IMX7ULP_CLK_APLL_PFD1>,
- <&scg1 IMX7ULP_CLK_APLL_PFD0>,
- <&scg1 IMX7ULP_CLK_UPLL>,
- <&scg1 IMX7ULP_CLK_SOSC_BUS_CLK>,
- <&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>,
- <&scg1 IMX7ULP_CLK_ROSC>,
- <&scg1 IMX7ULP_CLK_SPLL_BUS_CLK>;
- clock-names = "nic1_bus_clk", "nic1_clk", "ddr_clk",
- "apll_pfd2", "apll_pfd1", "apll_pfd0",
- "upll", "sosc_bus_clk", "mpll",
- "firc_bus_clk", "rosc", "spll_bus_clk";
-};
-
-usdhc1: usdhc@40380000 {
- compatible = "fsl,imx7ulp-usdhc";
- reg = <0x40380000 0x10000>;
- interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
- <&scg1 IMX7ULP_CLK_NIC1_DIV>,
- <&pcc2 IMX7ULP_CLK_USDHC1>;
- clock-names ="ipg", "ahb", "per";
- bus-width = <4>;
-};
diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-pcc-clock.yaml b/Documentation/devicetree/bindings/clock/imx7ulp-pcc-clock.yaml
new file mode 100644
index 0000000..d5fd286
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx7ulp-pcc-clock.yaml
@@ -0,0 +1,119 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx7ulp-pcc-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX7ULP Peripheral Clock Control (PCC) modules
+
+maintainers:
+ - A.s. Dong <aisheng.dong@nxp.com>
+
+description: |
+ i.MX7ULP Clock functions are under joint control of the System
+ Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
+ modules, and Core Mode Controller (CMC)1 blocks
+
+ The clocking scheme provides clear separation between M4 domain
+ and A7 domain. Except for a few clock sources shared between two
+ domains, such as the System Oscillator clock, the Slow IRC (SIRC),
+ and and the Fast IRC clock (FIRCLK), clock sources and clock
+ management are separated and contained within each domain.
+
+ M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
+ A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
+
+ Note: this binding doc is only for A7 clock domain.
+
+ The Peripheral Clock Control (PCC) is responsible for clock selection,
+ optional division and clock gating mode for peripherals in their
+ respected power domain.
+
+ The clock consumer should specify the desired clock by having the clock
+ ID in its "clocks" phandle cell.
+ See include/dt-bindings/clock/imx7ulp-clock.h for the full list of
+ i.MX7ULP clock IDs of each module.
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx7ulp-pcc2
+ - fsl,imx7ulp-pcc3
+
+ reg:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+
+ clocks:
+ items:
+ - description: nic1 bus clock
+ - description: nic1 clock
+ - description: ddr clock
+ - description: apll pfd2
+ - description: apll pfd1
+ - description: apll pfd0
+ - description: usb pll
+ - description: system osc bus clock
+ - description: fast internal reference clock bus
+ - description: rtc osc
+ - description: system pll bus clock
+
+ clock-names:
+ items:
+ - const: nic1_bus_clk
+ - const: nic1_clk
+ - const: ddr_clk
+ - const: apll_pfd2
+ - const: apll_pfd1
+ - const: apll_pfd0
+ - const: upll
+ - const: sosc_bus_clk
+ - const: firc_bus_clk
+ - const: rosc
+ - const: spll_bus_clk
+
+required:
+ - compatible
+ - reg
+ - '#clock-cells'
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx7ulp-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ clock-controller@403f0000 {
+ compatible = "fsl,imx7ulp-pcc2";
+ reg = <0x403f0000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
+ <&scg1 IMX7ULP_CLK_NIC1_DIV>,
+ <&scg1 IMX7ULP_CLK_DDR_DIV>,
+ <&scg1 IMX7ULP_CLK_APLL_PFD2>,
+ <&scg1 IMX7ULP_CLK_APLL_PFD1>,
+ <&scg1 IMX7ULP_CLK_APLL_PFD0>,
+ <&scg1 IMX7ULP_CLK_UPLL>,
+ <&scg1 IMX7ULP_CLK_SOSC_BUS_CLK>,
+ <&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>,
+ <&scg1 IMX7ULP_CLK_ROSC>,
+ <&scg1 IMX7ULP_CLK_SPLL_BUS_CLK>;
+ clock-names = "nic1_bus_clk", "nic1_clk", "ddr_clk",
+ "apll_pfd2", "apll_pfd1", "apll_pfd0",
+ "upll", "sosc_bus_clk", "firc_bus_clk",
+ "rosc", "spll_bus_clk";
+ };
+
+ mmc@40380000 {
+ compatible = "fsl,imx7ulp-usdhc";
+ reg = <0x40380000 0x10000>;
+ interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
+ <&scg1 IMX7ULP_CLK_NIC1_DIV>,
+ <&pcc2 IMX7ULP_CLK_USDHC1>;
+ clock-names ="ipg", "ahb", "per";
+ bus-width = <4>;
+ };
diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-scg-clock.yaml b/Documentation/devicetree/bindings/clock/imx7ulp-scg-clock.yaml
new file mode 100644
index 0000000..ae2a648
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx7ulp-scg-clock.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx7ulp-scg-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX7ULP System Clock Generation (SCG) modules
+
+maintainers:
+ - A.s. Dong <aisheng.dong@nxp.com>
+
+description: |
+ i.MX7ULP Clock functions are under joint control of the System
+ Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
+ modules, and Core Mode Controller (CMC)1 blocks
+
+ The clocking scheme provides clear separation between M4 domain
+ and A7 domain. Except for a few clock sources shared between two
+ domains, such as the System Oscillator clock, the Slow IRC (SIRC),
+ and and the Fast IRC clock (FIRCLK), clock sources and clock
+ management are separated and contained within each domain.
+
+ M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
+ A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
+
+ Note: this binding doc is only for A7 clock domain.
+
+ The System Clock Generation (SCG) is responsible for clock generation
+ and distribution across this device. Functions performed by the SCG
+ include: clock reference selection, generation of clock used to derive
+ processor, system, peripheral bus and external memory interface clocks,
+ source selection for peripheral clocks and control of power saving
+ clock gating mode.
+
+ The clock consumer should specify the desired clock by having the clock
+ ID in its "clocks" phandle cell.
+ See include/dt-bindings/clock/imx7ulp-clock.h for the full list of
+ i.MX7ULP clock IDs of each module.
+
+properties:
+ compatible:
+ const: fsl,imx7ulp-scg1
+
+ reg:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+
+ clocks:
+ items:
+ - description: rtc osc
+ - description: system osc
+ - description: slow internal reference clock
+ - description: fast internal reference clock
+ - description: usb PLL
+
+ clock-names:
+ items:
+ - const: rosc
+ - const: sosc
+ - const: sirc
+ - const: firc
+ - const: upll
+
+required:
+ - compatible
+ - reg
+ - '#clock-cells'
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx7ulp-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ clock-controller@403e0000 {
+ compatible = "fsl,imx7ulp-scg1";
+ reg = <0x403e0000 0x10000>;
+ clocks = <&rosc>, <&sosc>, <&sirc>,
+ <&firc>, <&upll>;
+ clock-names = "rosc", "sosc", "sirc",
+ "firc", "upll";
+ #clock-cells = <1>;
+ };
+
+ mmc@40380000 {
+ compatible = "fsl,imx7ulp-usdhc";
+ reg = <0x40380000 0x10000>;
+ interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
+ <&scg1 IMX7ULP_CLK_NIC1_DIV>,
+ <&pcc2 IMX7ULP_CLK_USDHC1>;
+ clock-names ="ipg", "ahb", "per";
+ bus-width = <4>;
+ };
--
2.7.4
_______________________________________________
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] media: stm32-dcmi: Set minimum cpufreq requirement
From: Valentin Schneider @ 2020-06-02 13:35 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: Alexandre TORGUE, rjw@rjwysocki.net, linux-kernel@vger.kernel.org,
mcoquelin.stm32@gmail.com, Hugues FRUCHET, mchehab@kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <f95ce45f-7a1c-0feb-afa8-203ddb500f2f@st.com>
On 02/06/20 12:37, Benjamin GAIGNARD wrote:
> On 6/2/20 11:31 AM, Valentin Schneider wrote:
>>> @@ -99,6 +100,8 @@ enum state {
>>>
>>> #define OVERRUN_ERROR_THRESHOLD 3
>>>
>>> +#define DCMI_MIN_FREQ 650000 /* in KHz */
>>> +
>> This assumes the handling part is guaranteed to always run on the same CPU
>> with the same performance profile (regardless of the platform). If that's
>> not guaranteed, it feels like you'd want this to be configurable in some
>> way.
> Yes I could add a st,stm32-dcmi-min-frequency (in KHz) parameter the
> device tree node.
>
Something like that - I'm not sure how well this fits with the DT
landscape, as you could argue it isn't really a description of the
hardware, more of a description of the performance expectations of the
software. I won't really argue here.
>>
>>> struct dcmi_graph_entity {
>>> struct v4l2_async_subdev asd;
>>>
>> [...]
>>> @@ -2020,6 +2042,8 @@ static int dcmi_probe(struct platform_device *pdev)
>>> goto err_cleanup;
>>> }
>>>
>>> + dcmi->policy = cpufreq_cpu_get(0);
>>> +
>> Ideally you'd want to fetch the policy of the CPU your IRQ (and handling
>> thread) is affined to; The only compatible DTS I found describes a single
>> A7, which is somewhat limited in the affinity area...
> If I move this code just before start streaming and use get_cpu(), would
> it works ?
>
AFAIA streaming_start() is not necessarily executing on the same CPU as the
one that will handle the interrupt. I was thinking you could use the IRQ's
effective affinity as a hint of which CPU(s) to boost, i.e. something like:
---
struct cpumask_var_t visited;
struct irq_data *d = irq_get_irq_data(irq);
err = alloc_cpumask_var(visited, GFP_KERNEL);
/* ... */
for_each_cpu(cpu, irq_data_get_effective_affinity_mask(d)) {
/* check if not already spanned */
if (cpumask_test_cpu(cpu, visited))
continue;
policy = cpufreq_cpu_get(cpu);
cpumask_or(visited, visited, policy->cpus);
/* do the boost for that policy here */
/* ... */
cpufreq_cpu_put(policy);
}
---
That of course falls apart when hotplug gets involved, and the effective
affinity changes... There's irq_set_affinity_notifier() out there, but it
seems it's only about the affinity, not the effective_affinity, I'm not
sure how valid it would be to query the effective_affinity in that
notifier.
> Benjamin
>>
>>> dev_info(&pdev->dev, "Probe done\n");
>>>
>>> platform_set_drvdata(pdev, dcmi);
>>> @@ -2049,6 +2073,9 @@ static int dcmi_remove(struct platform_device *pdev)
>>>
>>> pm_runtime_disable(&pdev->dev);
>>>
>>> + if (dcmi->policy)
>>> + cpufreq_cpu_put(dcmi->policy);
>>> +
>>> v4l2_async_notifier_unregister(&dcmi->notifier);
>>> v4l2_async_notifier_cleanup(&dcmi->notifier);
>>> media_entity_cleanup(&dcmi->vdev->entity);
_______________________________________________
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/7] drm/rockchip: prepare common code for cdns and rk dpi/dp driver
From: Emil Velikov @ 2020-06-02 13:55 UTC (permalink / raw)
To: sandor.yu
Cc: Jernej Skrabec, Heiko Stübner, Jonas Karlman, Neil Armstrong,
Sandy Huang, ML dri-devel, Linux-Kernel@Vger. Kernel. Org,
Andrzej Hajda, Laurent Pinchart, linux-rockchip, dkos, LAKML,
NXP Linux Team
In-Reply-To: <d3d707cf37e7928a839071242c752779061cc094.1590982881.git.Sandor.yu@nxp.com>
HI Sandor Yu
On Mon, 1 Jun 2020 at 07:29, <sandor.yu@nxp.com> wrote:
>
> From: Sandor Yu <Sandor.yu@nxp.com>
>
> - Extracted common fields from cdn_dp_device to a new cdns_mhdp_device
> structure which will be used by two separate drivers later on.
> - Moved some datatypes (audio_format, audio_info, vic_pxl_encoding_format,
> video_info) from cdn-dp-core.c to cdn-dp-reg.h.
> - Changed prefixes from cdn_dp to cdns_mhdp
> cdn -> cdns to match the other Cadence's drivers
> dp -> mhdp to distinguish it from a "just a DP" as the IP underneath
> this registers map can be a HDMI (which is internally different,
> but the interface for commands, events is pretty much the same).
> - Modified cdn-dp-core.c to use the new driver structure and new function
> names.
> - writel and readl are replaced by cdns_mhdp_bus_write and
> cdns_mhdp_bus_read.
>
The high-level idea is great - split, refactor and reuse the existing drivers.
Although looking at the patches themselves - they seems to be doing
multiple things at once.
As indicated by the extensive list in the commit log.
I would suggest splitting those up a bit, roughly in line of the
itemisation as per the commit message.
Here is one hand wavy way to chunk this patch:
1) use put_unalligned*
2) 'use local variable dev' style of changes (as seem in cdn_dp_clk_enable)
3) add writel/readl wrappers
4) hookup struct cdns_mhdp_device, keep dp->mhdp detail internal.
The cdn-dp-reg.h function names/signatures will stay the same.
5) finalize the helpers - use mhdp directly, rename
HTH
Emil
Examples:
4)
static int cdn_dp_mailbox_read(struct cdn_dp_device *dp)
{
+" struct cdns_mhdp_device *mhdp = dp->mhdp;
int val, ret;
- ret = readx_poll_timeout(readl, dp->regs + MAILBOX_EMPTY_ADDR,
+ ret = readx_poll_timeout(readl, mhdp->regs_base + MAILBOX_EMPTY_ADDR,
...
return fancy_readl(dp, MAILBOX0_RD_DATA) & 0xff;
}
5)
-static int cdn_dp_mailbox_read(struct cdn_dp_device *dp)
+static int mhdp_mailbox_read(struct cdns_mhdp_device *mhdp)
{
- struct cdns_mhdp_device *mhdp = dp->mhdp;
int val, ret;
...
- return fancy_readl(dp, MAILBOX0_RD_DATA) & 0xff;
+ return cdns_mhdp_bus_read(mhdp, MAILBOX0_RD_DATA) & 0xff;
}
_______________________________________________
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