* Constantly map and unmap of streaming DMA buffers with IOMMU backend might cause serious performance problem
From: Song Bao Hua @ 2020-05-15 8:19 UTC (permalink / raw)
To: linux@armlinux.org.uk, hch@lst.de, m.szyprowski@samsung.com,
robin.murphy@arm.com, dagum@barrel.engr.sgi.com, ralf@oss.sgi.com,
grundler@cup.hp.com, Jay.Estabrook@compaq.com,
sailer@ife.ee.ethz.ch, andrea@suse.de, jens.axboe@oracle.com,
davidm@hpl.hp.com
Cc: iommu@lists.linux-foundation.org, Linuxarm,
linux-arm-kernel@lists.infradead.org
Hi Russell & All,
In many DMA streaming map/unmap use cases, lower-layer device drivers completely have no idea how and when single/sg buffers are allocated and freed by upper-layer filesystem, network protocol, mm management system etc. So the only thing device drivers can do is constantly mapping the buffer before DMA begins and unmapping the buffer when DMA is done.
This will dramatically increase the latency of dma_map_single/sg and dma_unmap_single/sg when these APIs are bound with the IOMMU backend. As for each map, iommu driver needs to allocate iova and do the map in iommu. And for each unmap, it needs to free iova and unmap the buffer in iommu hardware. When devices performing DMA are super-fast, for example, on 100GbE networks, the DMA streaming map/unmap latency might become a critical system bottleneck.
In comparison to DMA streaming APIs, DMA consistent APIs using IOMMU backend may show much better performance as the map is done when the buffer is allocated and unmap is done when the buffer is freed. DMA can be done multiple times before the buffers are freed by dma_free_coherent(). There is no such map and unmap overhead for each separate DMA transfer as streaming APIs. The typical work flow is like
dma_alloc_coherent->
doing DMA ->
doing DMA ->
doing DMA ->
.... /* DMA many times */
dma_free_coherent
However, the typical work flow for streaming DMA is like
dma_map_sg -> doing DMA -> dma_unmap_sg ->
dma_map_sg -> doing DMA -> dma_unmap_sg ->
dma_map_sg -> doing DMA -> dma_unmap_sg ->
.... /* map, DMA transfer, unmap many times */
Even though upper-layer software might use the same buffers multiple times, for each single DMA transmission, map and unmap still need to be done by lower-level drivers as lower-layer drivers don't know this fact.
A possible routine to improve the performance of stream APIs is like:
dma_map_sg ->
dma_sync_sg_for_device -> doing DMA ->
dma_sync_sg_for_device -> doing DMA ->
dma_sync_sg_for_device -> doing DMA ->
... -> /* sync between DMA and CPU many times */
dma_unmap_sg
For every single DMA, software only needs to do sync operations which are much lighter that map and unmap. But this case is often not applicable to device drivers as the buffers usually come from the upper-layer filesystem, network protocol, mm management system etc. Device drivers have to work with the assumption that the buffer will be freed immediately after DMA is done. However, for those device drivers which are able to allocate and free the DMA stream buffers by themselves, they will get benefits of reusing the same buffers for doing DMA multiple times without map/unmap overhead.
I collected some latency data for iommu_dma_map_sg and iommu_dma_unmap_sg. In the test case, zswap is calling acomp APIs to compress/decompress pages, and comp/decomp is done by lower-level hardware ZIP driver.
root@ubuntu:/usr/share/bcc/tools# ./funclatency iommu_dma_map_sg
Tracing 1 functions for "iommu_dma_map_sg"... Hit Ctrl-C to end.
^C
nsecs : count distribution
0 -> 1 : 0 | |
2 -> 3 : 0 | |
4 -> 7 : 0 | |
8 -> 15 : 0 | |
16 -> 31 : 0 | |
32 -> 63 : 0 | |
64 -> 127 : 0 | |
128 -> 255 : 0 | |
256 -> 511 : 0 | |
512 -> 1023 : 0 | |
1024 -> 2047 : 2274570 |*********************** |
2048 -> 4095 : 3896310 |****************************************|
4096 -> 8191 : 74499 | |
8192 -> 16383 : 4475 | |
16384 -> 32767 : 1519 | |
32768 -> 65535 : 480 | |
65536 -> 131071 : 286 | |
131072 -> 262143 : 18 | |
262144 -> 524287 : 2 | |
root@ubuntu:/usr/share/bcc/tools# ./funclatency iommu_dma_unmap_sg
Tracing 1 functions for "iommu_dma_unmap_sg"... Hit Ctrl-C to end.
^C
nsecs : count distribution
0 -> 1 : 0 | |
2 -> 3 : 0 | |
4 -> 7 : 0 | |
8 -> 15 : 0 | |
16 -> 31 : 0 | |
32 -> 63 : 0 | |
64 -> 127 : 0 | |
128 -> 255 : 0 | |
256 -> 511 : 0 | |
512 -> 1023 : 0 | |
1024 -> 2047 : 0 | |
2048 -> 4095 : 56083 | |
4096 -> 8191 : 5232036 |****************************************|
8192 -> 16383 : 7723 | |
16384 -> 32767 : 1277 | |
32768 -> 65535 : 32 | |
65536 -> 131071 : 12 | |
131072 -> 262143 : 41 | |
In contrast, if we set iommu passthrough, the latency will be much better:
root@ubuntu:/usr/share/bcc/tools# ./funclatency dma_direct_map_sg
Tracing 1 functions for "dma_direct_map_sg"... Hit Ctrl-C to end.
^C
nsecs : count distribution
0 -> 1 : 0 | |
2 -> 3 : 0 | |
4 -> 7 : 0 | |
8 -> 15 : 0 | |
16 -> 31 : 0 | |
32 -> 63 : 0 | |
64 -> 127 : 0 | |
128 -> 255 : 0 | |
256 -> 511 : 0 | |
512 -> 1023 : 10798 | |
1024 -> 2047 : 1435035 |****************************************|
2048 -> 4095 : 13879 | |
4096 -> 8191 : 485 | |
8192 -> 16383 : 791 | |
16384 -> 32767 : 418 | |
32768 -> 65535 : 55 | |
65536 -> 131071 : 67 | |
131072 -> 262143 : 8 | |
root@ubuntu:/usr/share/bcc/tools# ./funclatency dma_direct_unmap_sg
Tracing 1 functions for "dma_direct_unmap_sg"... Hit Ctrl-C to end.
^C
nsecs : count distribution
0 -> 1 : 0 | |
2 -> 3 : 0 | |
4 -> 7 : 0 | |
8 -> 15 : 0 | |
16 -> 31 : 0 | |
32 -> 63 : 0 | |
64 -> 127 : 0 | |
128 -> 255 : 0 | |
256 -> 511 : 0 | |
512 -> 1023 : 216 | |
1024 -> 2047 : 250849 |****************************************|
2048 -> 4095 : 54341 |******** |
4096 -> 8191 : 80 | |
8192 -> 16383 : 191 | |
16384 -> 32767 : 65 | |
In summary, the comparison is as below:
(1)map
iommu passthrough mainly 1-2us
iommu non-passthrough mainly 2-4us
(2)unmap
iommu passthrough mainly 1-2us
iommu non-passthrough mainly 4-8us
The below is the long function trace for each dma_map/unmap_sg while iommu is enabled:
507.520069 | 53) | iommu_dma_map_sg() {
507.520070 | 53) 0.670 us | iommu_get_dma_domain();
507.520071 | 53) 0.610 us | iommu_dma_deferred_attach();
507.520072 | 53) | iommu_dma_alloc_iova.isra.26() {
507.520073 | 53) | alloc_iova_fast() {
507.520074 | 53) | _raw_spin_lock_irqsave() {
507.520074 | 53) 0.570 us | preempt_count_add();
507.520076 | 53) 2.060 us | }
507.520077 | 53) | _raw_spin_unlock_irqrestore() {
507.520077 | 53) 0.790 us | preempt_count_sub();
507.520079 | 53) 2.090 us | }
507.520079 | 53) 6.260 us | }
507.520080 | 53) 7.470 us | }
507.520081 | 53) | iommu_map_sg_atomic() {
507.520081 | 53) | __iommu_map_sg() {
507.520082 | 53) | __iommu_map() {
507.520082 | 53) 0.630 us | iommu_pgsize.isra.14();
507.520084 | 53) | arm_smmu_map() {
507.520084 | 53) | arm_lpae_map() {
507.520085 | 53) | __arm_lpae_map() {
507.520086 | 53) | __arm_lpae_map() {
507.520086 | 53) | __arm_lpae_map() {
507.520087 | 53) 0.930 us | __arm_lpae_map();
507.520089 | 53) 2.170 us | }
507.520089 | 53) 3.490 us | }
507.520090 | 53) 4.730 us | }
507.520090 | 53) 5.980 us | }
507.520091 | 53) 7.250 us | }
507.520092 | 53) 0.650 us | iommu_pgsize.isra.14();
507.520093 | 53) | arm_smmu_map() {
507.520093 | 53) | arm_lpae_map() {
507.520094 | 53) | __arm_lpae_map() {
507.520095 | 53) | __arm_lpae_map() {
507.520096 | 53) | __arm_lpae_map() {
507.520096 | 53) 0.630 us | __arm_lpae_map();
507.520098 | 53) 1.860 us | }
507.520098 | 53) 3.210 us | }
507.520099 | 53) 4.610 us | }
507.520099 | 53) 5.860 us | }
507.520100 | 53) 7.110 us | }
507.520101 | 53) + 18.740 us | }
507.520101 | 53) + 20.080 us | }
507.520102 | 53) + 21.320 us | }
507.520102 | 53) + 33.200 us | }
783.039976 | 48) | iommu_dma_unmap_sg() {
783.039977 | 48) | __iommu_dma_unmap() {
783.039978 | 48) 0.720 us | iommu_get_dma_domain();
783.039979 | 48) | iommu_unmap_fast() {
783.039980 | 48) | __iommu_unmap() {
783.039981 | 48) 0.740 us | iommu_pgsize.isra.14();
783.039982 | 48) | arm_smmu_unmap() {
783.039983 | 48) | arm_lpae_unmap() {
783.039984 | 48) | __arm_lpae_unmap() {
783.039985 | 48) | __arm_lpae_unmap() {
783.039985 | 48) | __arm_lpae_unmap() {
783.039986 | 48) | __arm_lpae_unmap() {
783.039988 | 48) 0.730 us | arm_smmu_tlb_inv_page_nosync();
783.039989 | 48) 3.010 us | }
783.039990 | 48) 4.490 us | }
783.039991 | 48) 5.950 us | }
783.039991 | 48) 7.460 us | }
783.039992 | 48) 8.920 us | }
783.039993 | 48) + 10.380 us | }
783.039993 | 48) + 13.350 us | }
783.039994 | 48) + 14.820 us | }
783.039995 | 48) | arm_smmu_iotlb_sync() {
783.039996 | 48) | arm_smmu_tlb_inv_range() {
783.039996 | 48) | arm_smmu_cmdq_batch_add() {
783.039997 | 48) 0.760 us | arm_smmu_cmdq_build_cmd();
783.039999 | 48) 2.220 us | }
783.039999 | 48) | arm_smmu_cmdq_issue_cmdlist() {
783.040000 | 48) 0.530 us | arm_smmu_cmdq_build_cmd();
783.040001 | 48) 0.530 us | __arm_smmu_cmdq_poll_set_valid_map.isra.40();
783.040002 | 48) 0.540 us | __arm_smmu_cmdq_poll_set_valid_map.isra.40();
783.040004 | 48) | ktime_get() {
783.040004 | 48) 0.540 us | arch_counter_read();
783.040005 | 48) 1.570 us | }
783.040006 | 48) 6.880 us | }
783.040007 | 48) 0.830 us | arm_smmu_atc_inv_domain.constprop.48();
783.040008 | 48) + 12.910 us | }
783.040009 | 48) + 14.370 us | }
783.040010 | 48) | iommu_dma_free_iova() {
783.040011 | 48) | free_iova_fast() {
783.040011 | 48) | _raw_spin_lock_irqsave() {
783.040012 | 48) 0.600 us | preempt_count_add();
783.040013 | 48) 2.000 us | }
783.040014 | 48) | _raw_spin_unlock_irqrestore() {
783.040015 | 48) 0.820 us | preempt_count_sub();
783.040016 | 48) 2.220 us | }
783.040018 | 48) 6.200 us | }
783.040019 | 48) 8.880 us | }
783.040020 | 48) + 42.540 us | }
783.040020 | 48) + 44.030 us | }
I am thinking several possible ways on decreasing or removing the latency of DMA map/unmap for every single DMA transfer. Meanwhile, "non-strict" as an existing option with possible safety issues, I won't discuss it in this mail.
1. provide bounce coherent buffers for streaming buffers.
As the coherent buffers keep the status of mapping, we can remove the overhead of map and unmap for each single DMA operations. However, this solution requires memory copy between stream buffers and bounce buffers. Thus it will work only if copy is faster than map/unmap. Meanwhile, it will consume much more memory bandwidth.
2.make upper-layer kernel components aware of the pain of iommu map/unmap
upper-layer fs, mm, networks can somehow let the lower-layer drivers know the end of the life cycle of sg buffers. In zswap case, I have seen zswap always use the same 2 pages as the destination buffers to save compressed page, but the compressor driver still has to constantly map and unmap those same two pages for every single compression since zswap and zip drivers are working in two completely different software layers.
I am thinking some way as below, upper-layer kernel code can call:
sg_init_table(&sg...);
sg_mark_reusable(&sg....);
.... /* use the buffer many times */
....
sg_mark_stop_reuse(&sg);
After that, if low level drivers see "reusable" flag, it will realize the buffer can be used multiple times and will not do map/unmap every time. it means upper-layer components will further use the buffers and the same buffers will probably be given to lower-layer drivers for new DMA transfer later. When upper-layer code sets " stop_reuse", lower-layer driver will unmap the sg buffers, possibly by providing a unmap-callback to upper-layer components. For zswap case, I have seen the same buffers are always re-used and zip driver maps and unmaps it again and again. Shortly after the buffer is unmapped, it will be mapped in the next transmission, almost without any time gap between unmap and map. In case zswap can set the "reusable" flag, zip driver will save a lot of time.
Meanwhile, for the safety of buffers, lower-layer drivers need to make certain the buffers have already been unmapped in iommu before those buffers go back to buddy for other users.
I don't think letting upper-layer components aware of the overhead of map and unmap is elegant. But it might be something which deserves to be done for performance reason. Upper-layer software which is friendly to lower-layer driver might call sg_mark_reusable(&sg....). But it is not enforced, if upper-layer components don't call the API, the current lower-level driver won't be affected.
Please kindly give your comments on this proposal and provide your suggestions on any possible way to improve the performance of DMA stream APIs with iommu backend. I am glad to send a draft patch for "reusable" buffers if you think it is not bad.
Best Regards
Barry
_______________________________________________
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 20/91] clk: bcm: rpi: Discover the firmware clocks
From: Maxime Ripard @ 2020-05-15 8:19 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Tim Gover, Dave Stevenson, Stephen Boyd, Michael Turquette,
linux-kernel, dri-devel, Phil Elwell, Eric Anholt,
bcm-kernel-feedback-list, linux-rpi-kernel, linux-clk,
linux-arm-kernel
In-Reply-To: <c9a7e50f88022179ef913fc6dfd066ec44b27988.camel@suse.de>
[-- Attachment #1.1: Type: text/plain, Size: 5543 bytes --]
Hi Nicolas,
On Mon, May 04, 2020 at 02:05:47PM +0200, Nicolas Saenz Julienne wrote:
> Hi Maxime, as always, thanks for the series!
> Some extra context, and comments below.
>
> On Fri, 2020-04-24 at 17:34 +0200, Maxime Ripard wrote:
> > The RaspberryPi4 firmware actually exposes more clocks than are currently
> > handled by the driver and we will need to change some of them directly
> > based on the pixel rate for the display related clocks, or the load for the
> > GPU.
> >
> > This rate change can have a number of side-effects, including adjusting the
> > various PLL voltages or the PLL parents. The firmware will also update
> > those clocks by itself for example if the SoC runs too hot.
>
> To complete this:
>
> RPi4's firmware implements DVFS. Clock frequency and SoC voltage are
> correlated, if you can determine all clocks are running below a maximum then it
> should be safe to lower the voltage. Currently, firmware actively monitors arm,
> core, h264, v3d, isp and hevc to determine what voltage can be reduced to, and
> if arm increases any of those clocks behind the firmware's back, when it has a
> lowered voltage, a crash is highly likely.
>
> As pointed out to me by RPi foundation engineers hsm/pixel aren't currently
> being monitored, as driving a high resolution display also requires a high core
> clock, which already sets an acceptable min voltage threshold. But that might
> change if needed.
>
> Ultimately, from the DVFS, the safe way to change clocks from arm would be to
> always use the firmware interface, which we're far from doing right now. On the
> other hand, AFAIK not all clocks have a firmware counterpart.
>
> Note that we could also have a word with the RPi foundation and see if
> disabling DVFS is possible (AFAIK it's not an option right now). Although I
> personally prefer to support this upstream, aside from the obvious benefits to
> battery powered use cases, not consuming power unnecessarily is always big
> plus.
>
> > In order to make Linux play as nice as possible with those constraints, it
> > makes sense to rely on the firmware clocks as much as possible.
>
> As I comment above, not as simple as it seems. I suggest, for now, we only
> register the clocks we're going to use and that are likely to be affected by
> DVFS. hsm being a contender here.
>
> As we'd be settling on a hybrid solution here, which isn't ideal to say the
> least, I'd like to gather some opinions on whether pushing towards a fully
> firmware based solution is something you'd like to see.
Thanks for the summary above, I'll try to adjust that commit log to reflect
this. As for my opinion, I don't really think that an hybrid approach is
practical, since that would leave us with weird interactions between the
firmware (and possibly multiple versinos of it) and the linux driver, and this
would be pretty hard to maintain in the long run.
That leaves us either the MMIO-based driver or the firmware-based one, and here
with what you said above, at the moment, the firmware based one is a clear
winner.
> > Fortunately,t he firmware has an interface to discover the clocks it
> > exposes.
>
> nit: wrongly placed space
>
> > Let's use it to discover, register the clocks in the clocks framework and
> > then expose them through the device tree for consumers to use them.
> >
> > Cc: Michael Turquette <mturquette@baylibre.com>
> > Cc: Stephen Boyd <sboyd@kernel.org>
> > Cc: linux-clk@vger.kernel.org
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> > drivers/clk/bcm/clk-raspberrypi.c | 104 +++++++++++++++++++---
>
> [...]
>
> > +static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
> > + unsigned int parent,
> > + unsigned int id)
> > +{
> > + struct raspberrypi_clk_data *data;
> > + struct clk_init_data init = {};
> > + int ret;
> > +
> > + if (id == RPI_FIRMWARE_ARM_CLK_ID) {
> > + struct clk_hw *hw;
> > +
> > + hw = raspberrypi_register_pllb(rpi);
> > + if (IS_ERR(hw)) {
> > + dev_err(rpi->dev, "Failed to initialize pllb, %ld\n",
> > + PTR_ERR(hw));
> > + return hw;
> > + }
> > +
> > + return raspberrypi_register_pllb_arm(rpi);
> > + }
>
> We shouldn't create a special case for pllb. My suggestion here is that we
> revert the commit that removed pllb from the mmio driver, in order to maintain
> a nice view of the clock tree, and register this as a regular fw-clk.
>
> The only user to this is RPi's the cpufreq driver, which shouldn't mind the
> change as long as you maintain the clk lookup creation.
Ok, I'll change that.
> On that topic, now that the clocks are available in DT we could even move
> raspberrypi-cpufreq's registration there. But that's out of the scope of this
> series.
>
> > +
> > + data = devm_kzalloc(rpi->dev, sizeof(*data), GFP_KERNEL);
> > + if (!data)
> > + return ERR_PTR(-ENOMEM);
> > + data->rpi = rpi;
> > + data->id = id;
> > +
> > + init.name = devm_kasprintf(rpi->dev, GFP_KERNEL, "fw-clk-%u", id);
>
> I'd really like more descriptive names here, sadly the firmware interface
> doesn't provide them, but they are set in stone nonetheless. Something like
> 'fw-clk-arm' and 'fw-clk-hsm' comes to mind (SCMI does provide naming BTW).
>
> See here for a list of all clocks:
> https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface#clocks
That's a good idea, I'll add that too.
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: [Linux-stm32] [PATCH v3 3/5] ARM: dts: stm32: enable ltdc binding with ili9341 on stm32429-disco board
From: Benjamin GAIGNARD @ 2020-05-15 8:31 UTC (permalink / raw)
To: dillon min, Alexandre TORGUE
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Maxime Coquelin, Michael Turquette, Dave Airlie, Linus Walleij,
linux-kernel@vger.kernel.org, open list:DRM PANEL DRIVERS,
linux-stm32@st-md-mailman.stormreply.com, Stephen Boyd,
Rob Herring, thierry.reding@gmail.com, Daniel Vetter,
Sam Ravnborg, linux-clk, Linux ARM
In-Reply-To: <CAL9mu0Jt_xwo5pJfcx6G3grBuOaxLXvakpEjiB4gV3=bkiq2fg@mail.gmail.com>
On 5/14/20 3:07 PM, dillon min wrote:
> Hi Alexandre,
>
> On Thu, May 14, 2020 at 8:53 PM Alexandre Torgue
> <alexandre.torgue@st.com> wrote:
>>
>>
>> On 5/14/20 10:24 AM, Linus Walleij wrote:
>>> On Tue, May 12, 2020 at 9:04 AM <dillon.minfei@gmail.com> wrote:
>>>
>>>> From: dillon min <dillon.minfei@gmail.com>
>>>>
>>>> Enable the ltdc & ili9341 on stm32429-disco board.
>>>>
>>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>
>>> This mostly looks good but...
>>>
>>>> +&spi5 {
>>>> + status = "okay";
>>>> + pinctrl-0 = <&spi5_pins>;
>>>> + pinctrl-names = "default";
>>>> + #address-cells = <1>;
>>>> + #size-cells = <0>;
>>>> + cs-gpios = <&gpioc 2 GPIO_ACTIVE_LOW>;
>>>> + dmas = <&dma2 3 2 0x400 0x0>,
>>>> + <&dma2 4 2 0x400 0x0>;
>>>> + dma-names = "rx", "tx";
>>> These DMA assignments seem to be SoC things and should
>>> rather be in the DTS(I) file where &spi5 is defined, right?
>>> stm32f429.dtsi I suppose?
>> I agree with Linus, DMA have to be defined in SoC dtsi. And if a board
>> doesn't want to use it, we use the "delete-property".
> Yes, will move to Soc dtsi in next submits.
>
> i'm working on write a v4l2-m2m driver for dma2d of stm32 to support
> pixel conversion
> alpha blending between foreground and background graphics.
>
> as you know, some soc's engineer trying to add this function to drm system.
>
> do you know st's planning about soc's hardware accelerator driver on stm32mp?
> such as chrom-art, will add to drm subsystem via ioctl to access, or to v4l2,
On stm32mp we do not plan to use chrom-art in drm or v4l2 because it
does fit
with userland way of working. We use the GPU to do conversion, scaling,
blending
and composition in only one go.
As explain here [1] DRM subsytem it isn't a solution and v4l2-m2m isn't
used in any
mainline compositors like Weston or android surfaceflinger.
Benjamin
[1]
https://www.phoronix.com/scan.php?page=news_item&px=Linux-DRM-No-2D-Accel-API
>
> thanks.
>
>>> It is likely the same no matter which device is using spi5.
>>>
>>> Yours,
>>> Linus Walleij
>>>
> _______________________________________________
> Linux-stm32 mailing list
> Linux-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/2] hwrng: iproc-rng200 - Set the quality value
From: Stephan Mueller @ 2020-05-15 8:32 UTC (permalink / raw)
To: Lukasz Stelmach
Cc: Florian Fainelli, Herbert Xu, Scott Branden, Matthias Brugger,
Greg Kroah-Hartman, Matt Mackall, linux-kernel,
Krzysztof Kozlowski, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
Kukjin Kim, Arnd Bergmann, Stefan Wahren, Ray Jui,
bcm-kernel-feedback-list, Markus Elfring, linux-arm-kernel,
linux-crypto
In-Reply-To: <dleftjtv0i88ji.fsf%l.stelmach@samsung.com>
Am Freitag, 15. Mai 2020, 00:18:41 CEST schrieb Lukasz Stelmach:
Hi Lukasz,
>
> I am running tests using SP800-90B tools and the first issue I can see
> is the warning that samples contain less than 1e6 bytes of data. I know
> little about maths behind random number generators, but I have noticed
> that the bigger chunk of data from an RNG I feed into either ent or ea_iid
> the higher entropy they report. That is why I divided the data into 1024
> bit chunks in the first place. To get worse results. With ea_iid they
> get even worse (128 bytes of random data)
I read that you seem to just take the output data from the RNG. If this is
correct, I think we can stop right here. The output of an RNG is usually after
post-processing commonly provided by a cryptographic function.
Thus, when processing the output of the RNG all what we measure here is the
quality of the cryptographic post-processing and not the entropy that may be
present in the data.
What we need is to access the noise source and analyze this with the given
tool set. And yes, the analysis may require adjusting the data to a format
that can be consumed and analyzed by the statistical tests.
Ciao
Stephan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC] arm64: Enable perf events based hard lockup detector
From: Sumit Garg @ 2020-05-15 8:49 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, Sumit Garg, daniel.thompson, peterz,
catalin.marinas, jolsa, dianders, acme, alexander.shishkin, mingo,
namhyung, tglx, will, julien.thierry.kdev
With the recent feature added to enable perf events to use pseudo NMIs
as interrupts on platforms which support GICv3 or later, its now been
possible to enable hard lockup detector (or NMI watchdog) on arm64
platforms. So enable corresponding support.
One thing to note here is that normally lockup detector is initialized
just after the early initcalls but PMU on arm64 comes up much later as
device_initcall(). So we need to re-initialize lockup detection once
PMU has been initialized.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
This patch is dependent on perf NMI patch-set [1].
[1] https://patchwork.kernel.org/cover/11047407/
arch/arm64/Kconfig | 2 ++
arch/arm64/kernel/perf_event.c | 32 ++++++++++++++++++++++++++++++--
drivers/perf/arm_pmu.c | 11 +++++++++++
include/linux/perf/arm_pmu.h | 2 ++
4 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 40fb05d..36f75c2 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -160,6 +160,8 @@ config ARM64
select HAVE_NMI
select HAVE_PATA_PLATFORM
select HAVE_PERF_EVENTS
+ select HAVE_PERF_EVENTS_NMI if ARM64_PSEUDO_NMI
+ select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
select HAVE_PERF_REGS
select HAVE_PERF_USER_STACK_DUMP
select HAVE_REGS_AND_STACK_ACCESS_API
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 3ad5c8f..df57360 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -20,6 +20,8 @@
#include <linux/perf/arm_pmu.h>
#include <linux/platform_device.h>
#include <linux/smp.h>
+#include <linux/nmi.h>
+#include <linux/cpufreq.h>
/* ARMv8 Cortex-A53 specific event types. */
#define ARMV8_A53_PERFCTR_PREF_LINEFILL 0xC2
@@ -1190,10 +1192,21 @@ static struct platform_driver armv8_pmu_driver = {
static int __init armv8_pmu_driver_init(void)
{
+ int ret;
+
if (acpi_disabled)
- return platform_driver_register(&armv8_pmu_driver);
+ ret = platform_driver_register(&armv8_pmu_driver);
else
- return arm_pmu_acpi_probe(armv8_pmuv3_init);
+ ret = arm_pmu_acpi_probe(armv8_pmuv3_init);
+
+ /*
+ * Try to re-initialize lockup detector after PMU init in
+ * case PMU events are triggered via NMIs.
+ */
+ if (arm_pmu_irq_is_nmi())
+ lockup_detector_init();
+
+ return ret;
}
device_initcall(armv8_pmu_driver_init)
@@ -1225,3 +1238,18 @@ void arch_perf_update_userpage(struct perf_event *event,
userpg->time_shift = (u16)shift;
userpg->time_offset = -now;
}
+
+#ifdef CONFIG_HARDLOCKUP_DETECTOR_PERF
+#define SAFE_MAX_CPU_FREQ 4000000000UL // 4 GHz
+u64 hw_nmi_get_sample_period(int watchdog_thresh)
+{
+ unsigned int cpu = smp_processor_id();
+ unsigned int max_cpu_freq;
+
+ max_cpu_freq = cpufreq_get_hw_max_freq(cpu);
+ if (max_cpu_freq)
+ return (u64)max_cpu_freq * 1000 * watchdog_thresh;
+ else
+ return (u64)SAFE_MAX_CPU_FREQ * watchdog_thresh;
+}
+#endif
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index f96cfc4..691dfc9 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -718,6 +718,17 @@ static int armpmu_get_cpu_irq(struct arm_pmu *pmu, int cpu)
return per_cpu(hw_events->irq, cpu);
}
+bool arm_pmu_irq_is_nmi(void)
+{
+ const struct pmu_irq_ops *irq_ops;
+
+ irq_ops = per_cpu(cpu_irq_ops, smp_processor_id());
+ if (irq_ops == &pmunmi_ops || irq_ops == &percpu_pmunmi_ops)
+ return true;
+ else
+ return false;
+}
+
/*
* PMU hardware loses all context when a CPU goes offline.
* When a CPU is hotplugged back in, since some hardware registers are
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index d9b8b76..a71f029 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -155,6 +155,8 @@ int arm_pmu_acpi_probe(armpmu_init_fn init_fn);
static inline int arm_pmu_acpi_probe(armpmu_init_fn init_fn) { return 0; }
#endif
+bool arm_pmu_irq_is_nmi(void);
+
/* Internal functions only for core arm_pmu code */
struct arm_pmu *armpmu_alloc(void);
struct arm_pmu *armpmu_alloc_atomic(void);
--
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 1/2] hwrng: iproc-rng200 - Set the quality value
From: Lukasz Stelmach @ 2020-05-15 9:01 UTC (permalink / raw)
To: Stephan Mueller
Cc: Florian Fainelli, Herbert Xu, Scott Branden, Matthias Brugger,
Greg Kroah-Hartman, Matt Mackall, linux-kernel,
Krzysztof Kozlowski, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
Kukjin Kim, Arnd Bergmann, Stefan Wahren, Ray Jui,
bcm-kernel-feedback-list, Markus Elfring, linux-arm-kernel,
linux-crypto
In-Reply-To: <dleftjtv0i88ji.fsf%l.stelmach@samsung.com>
[-- Attachment #1.1: Type: text/plain, Size: 1999 bytes --]
It was <2020-05-15 pią 00:18>, when Lukasz Stelmach wrote:
> It was <2020-05-14 czw 22:20>, when Stephan Mueller wrote:
>> Am Donnerstag, 14. Mai 2020, 21:07:33 CEST schrieb Łukasz Stelmach:
>>
>> Hi Łukasz,
>>
>>> The value has been estimaded by obtainig 1024 chunks of data 128 bytes
>>> (1024 bits) each from the generator and finding chunk with minimal
>>> entropy using the ent(1) tool. The value was 6.327820 bits of entropy
>>> in each 8 bits of data.
>>
>> I am not sure we should use the ent tool to define the entropy
>> level. Ent seems to use a very coarse entropy estimation.
>>
>> I would feel more comfortable when using other measures like SP800-90B
>> which even provides a tool for the analysis.
>>
>> I understand that entropy estimates, well, are estimates. But the ent
>> data is commonly not very conservative.
>>
>> [1] https://github.com/usnistgov/SP800-90B_EntropyAssessment
[...]
> Anyway. I collected 1024 files 1024 bits each once again and ran the
> following tests
>
> for f in exynos-trng/random*; do ./ea_iid "$f" | grep ^min; done | sort | head -1
> for f in rng200/random*; do ./ea_iid "$f" | grep ^min; done | sort | head -1
>
> For both RNGs I got the same
>
> min(H_original, 8 X H_bitstring): 3.393082
Oddly enough I've got the same number for other random sources on my x86
| Source | ea_iid -i | ea_iid -c (h') | ent |
|--------------+-----------+----------------+----------|
| /dev/random | 3.393082 | 0.768654 | 6.300399 |
| /dev/urandom | 3.393082 | 0.759161 | 6.348562 |
| tpm-rng | 3.393082 | 0.735722 | 6.323990 |
| exynos-trng | 3.393082 | 0.687825 | 6.327820 |
| rng200 | 3.393082 | 0.740376 | 6.291959 |
I suspect 1024 bits is too little for ea_iid to give a meaningfull
result. BTW ent results also seem a little oddly low for crng. Any
thoughs?
--
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 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 1/2] hwrng: iproc-rng200 - Set the quality value
From: Lukasz Stelmach @ 2020-05-15 9:06 UTC (permalink / raw)
To: Stephan Mueller
Cc: Florian Fainelli, Herbert Xu, Scott Branden, Matthias Brugger,
Greg Kroah-Hartman, Matt Mackall, linux-kernel,
Krzysztof Kozlowski, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
Kukjin Kim, Arnd Bergmann, Stefan Wahren, Ray Jui,
bcm-kernel-feedback-list, Markus Elfring, linux-arm-kernel,
linux-crypto
In-Reply-To: <2080864.23lDWg4Bvs@tauon.chronox.de>
[-- Attachment #1.1: Type: text/plain, Size: 1483 bytes --]
It was <2020-05-15 pią 10:32>, when Stephan Mueller wrote:
> Am Freitag, 15. Mai 2020, 00:18:41 CEST schrieb Lukasz Stelmach:
>
>> I am running tests using SP800-90B tools and the first issue I can see
>> is the warning that samples contain less than 1e6 bytes of data. I know
>> little about maths behind random number generators, but I have noticed
>> that the bigger chunk of data from an RNG I feed into either ent or ea_iid
>> the higher entropy they report. That is why I divided the data into 1024
>> bit chunks in the first place. To get worse results. With ea_iid they
>> get even worse (128 bytes of random data)
>
> I read that you seem to just take the output data from the RNG. If this is
> correct, I think we can stop right here. The output of an RNG is usually after
> post-processing commonly provided by a cryptographic function.
>
> Thus, when processing the output of the RNG all what we measure here is the
> quality of the cryptographic post-processing and not the entropy that may be
> present in the data.
>
> What we need is to access the noise source and analyze this with the given
> tool set. And yes, the analysis may require adjusting the data to a format
> that can be consumed and analyzed by the statistical tests.
I took data from /dev/hwrng which is directly connected to the
hardware. See rng_dev_read() in drivers/char/hw_random/core.c.
--
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 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 1/2] hwrng: iproc-rng200 - Set the quality value
From: Stephan Mueller @ 2020-05-15 9:10 UTC (permalink / raw)
To: Lukasz Stelmach
Cc: Florian Fainelli, Herbert Xu, Scott Branden, Matthias Brugger,
Greg Kroah-Hartman, Matt Mackall, linux-kernel,
Krzysztof Kozlowski, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
Kukjin Kim, Arnd Bergmann, Stefan Wahren, Ray Jui,
bcm-kernel-feedback-list, Markus Elfring, linux-arm-kernel,
linux-crypto
In-Reply-To: <dleftjimgx8tc3.fsf%l.stelmach@samsung.com>
Am Freitag, 15. Mai 2020, 11:01:48 CEST schrieb Lukasz Stelmach:
Hi Lukasz,
As I mentioned, all that is or seems to be analyzed here is the quality of the
cryptographic post-processing. Thus none of the data can be used for getting
an idea of the entropy content.
That said, the ent value indeed looks too low which seems to be an issue in
the tool itself.
Note, for an entropy assessment commonly at least 1 million traces from the
raw noise source are needed.
See for examples on how such entropy assessments are conducted in the LRNG
documentation [1] or the Linux /dev/random implementation in [2]
[1] appendix C of https://www.chronox.de/lrng/doc/lrng.pdf
[2] chapter 6 of https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/
Publications/Studies/LinuxRNG/LinuxRNG_EN.pdf
Ciao
Stephan
_______________________________________________
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 01/10] spi: dw: Add support for polled operation via no IRQ specified in DT
From: Lars Povlsen @ 2020-05-15 9:11 UTC (permalink / raw)
To: Serge Semin
Cc: devicetree, Alexandre Belloni, Andy Shevchenko, linux-kernel,
Serge Semin, linux-spi, SoC Team, Mark Brown, linux-arm-kernel,
Microchip Linux Driver Support, Lars Povlsen
In-Reply-To: <20200514130407.guyk3r4ltjvszy5s@mobilestation>
Serge Semin writes:
> Hi Mark
>
> On Wed, May 13, 2020 at 03:20:50PM +0100, Mark Brown wrote:
>> On Wed, May 13, 2020 at 04:00:22PM +0200, Lars Povlsen wrote:
>> > With this change a SPI controller can be added without having a IRQ
>> > associated, and causing all transfers to be polled. For SPI controllers
>> > without DMA, this can significantly improve performance by less
>> > interrupt handling overhead.
>>
>> This overlaps substantially with some work that Serge Semin (CCed) has
>> in progress, please coordinate with him.
>
> Thanks for copying me these mails. I haven't been Cc'ed in the series and
> hasn't been subscribed to the SPI mailing list, so I would have definitely
> missed that.
>
> I would like to coordinate my efforts with Lars. I'll have the patchset reviewed
> soon in addition providing my comments/suggestions of how to make it useful for
> both mine and Lars solution.
Serge - thanks for taking on this.
Note that my primary concern now is to get Sparx5 upstreamed. The
mem_ops (or dirmap) and polled mode are both performance enhancements,
which can be pulled from my series if it creates too much noise. I can
then add the necessary on top of your work/current kernel at a later
time.
> One thing I can tell about the mem_ops he implemented, is that they aren't
> mem_ops, but dirmap (as you remember it's also implemented in my code, but with
> alignment specific), and the exec_mem_op partly consists of a code, which belong
> to the supports_op() callback. The rest of my comments will be inlined in the
> patches.
>
> -Sergey
--
Lars Povlsen,
Microchip
_______________________________________________
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] arm64/defconfig: Enable CONFIG_KEXEC_FILE
From: Bhupesh Sharma @ 2020-05-15 9:14 UTC (permalink / raw)
To: Catalin Marinas, Arnd Bergmann
Cc: Mark Rutland, kexec mailing list, AKASHI Takahiro, arm,
James Morse, Bhupesh SHARMA, Will Deacon, linux-arm-kernel
In-Reply-To: <CACi5LpPW2zmq0-UDnU_115ePxXKWG+1i6UciVWPpq=PzQHrkOw@mail.gmail.com>
Hi Arnd,
On Thu, Apr 30, 2020 at 10:05 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
>
> On Tue, Apr 28, 2020 at 3:37 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > On Tue, Apr 28, 2020 at 01:55:58PM +0530, Bhupesh Sharma wrote:
> > > On Wed, Apr 8, 2020 at 4:17 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > > > On Tue, Apr 07, 2020 at 04:01:40AM +0530, Bhupesh Sharma wrote:
> > > > > arch/arm64/configs/defconfig | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > > > > index 24e534d85045..fa122f4341a2 100644
> > > > > --- a/arch/arm64/configs/defconfig
> > > > > +++ b/arch/arm64/configs/defconfig
> > > > > @@ -66,6 +66,7 @@ CONFIG_SCHED_SMT=y
> > > > > CONFIG_NUMA=y
> > > > > CONFIG_SECCOMP=y
> > > > > CONFIG_KEXEC=y
> > > > > +CONFIG_KEXEC_FILE=y
> > > > > CONFIG_CRASH_DUMP=y
> > > > > CONFIG_XEN=y
> > > > > CONFIG_COMPAT=y
> > > > > --
> > > > > 2.7.4
> > >
> > > Thanks a lot Mark.
> > >
> > > Hi Catalin, Will,
> > >
> > > Can you please help pick this patch in the arm tree. We have an
> > > increasing number of user-cases from distro users
> > > who want to use kexec_file_load() as the default interface for
> > > kexec/kdump on arm64.
> >
> > We could pick it up if it doesn't conflict with the arm-soc tree. They
> > tend to pick most of the defconfig changes these days (and could as well
> > pick this one).
>
> Thanks Catalin.
> (+Cc Arnd)
>
> Hi Arnd,
>
> Can you please help pick this change via the arm-soc tree?
Ping. Any updates on this defconfig patch.
Thanks,
Bhupesh
_______________________________________________
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] hw_breakpoint: Remove unused '__register_perf_hw_breakpoint' declaration
From: Bhupesh Sharma @ 2020-05-15 9:15 UTC (permalink / raw)
To: Mark Rutland, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra
Cc: Catalin Marinas, Bhupesh SHARMA, Will Deacon,
Linux Kernel Mailing List, linux-arm-kernel
In-Reply-To: <CACi5LpM9_O6gRgMgfAXrmZuaO111xJk3=xtjYXK5rKhTF7Znsg@mail.gmail.com>
Hi Peter, Frederic, Ingo
On Thu, Apr 30, 2020 at 9:49 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
>
> Hi Mark,
>
> Thanks for the review.
>
> On Tue, Apr 28, 2020 at 3:37 PM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > Hi Bhupesh,
> >
> > On Tue, Apr 28, 2020 at 02:22:17PM +0530, Bhupesh Sharma wrote:
> > > commit b326e9560a28 ("hw-breakpoints: Use overflow handler instead of
> > > the event callback") removed '__register_perf_hw_breakpoint' function
> > > usage and replaced it with 'register_perf_hw_breakpoint' function.
> > >
> > > Remove the left-over unused '__register_perf_hw_breakpoint' declaration
> > > from 'hw_breakpoint.h' as well.
> > >
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> >
> > This is generic code, so I'm a bit confused as to why you've sent it to
> > us. I'd expect this to go via the perf core maintainers (cc'd).
>
> Oops, my bad. Seems my git patch sending script messed up while
> picking up the perf maintainers (who should have been Cc'ed on the
> patch) :(
>
> Thanks for adding them in the Cc list.
>
> Hi Peter, Frederic, Ingo - Kindly help review this patch and help
> apply the patch (if suitable).
Ping. Any comments on this patch?
Thanks,
Bhupesh
> > FWIW, this looks sane to me, so:
> >
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> >
> > Mark.
> >
> > > ---
> > > include/linux/hw_breakpoint.h | 3 ---
> > > 1 file changed, 3 deletions(-)
> > >
> > > diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
> > > index 6058c3844a76..fe1302da8e0f 100644
> > > --- a/include/linux/hw_breakpoint.h
> > > +++ b/include/linux/hw_breakpoint.h
> > > @@ -72,7 +72,6 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
> > > void *context);
> > >
> > > extern int register_perf_hw_breakpoint(struct perf_event *bp);
> > > -extern int __register_perf_hw_breakpoint(struct perf_event *bp);
> > > extern void unregister_hw_breakpoint(struct perf_event *bp);
> > > extern void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events);
> > >
> > > @@ -115,8 +114,6 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
> > > void *context) { return NULL; }
> > > static inline int
> > > register_perf_hw_breakpoint(struct perf_event *bp) { return -ENOSYS; }
> > > -static inline int
> > > -__register_perf_hw_breakpoint(struct perf_event *bp) { return -ENOSYS; }
> > > static inline void unregister_hw_breakpoint(struct perf_event *bp) { }
> > > static inline void
> > > unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events) { }
> > > --
> > > 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
* Re: [PATCH v3 0/7] firmware: smccc: Add basic SMCCC v1.2 + ARCH_SOC_ID support
From: Sudeep Holla @ 2020-05-15 9:16 UTC (permalink / raw)
To: Etienne Carriere
Cc: mark.rutland, lorenzo.pieralisi, arnd, catalin.marinas,
linux-kernel, steven.price, harb, Sudeep Holla, will,
linux-arm-kernel
In-Reply-To: <20200515075032.5325-1-etienne.carriere@linaro.org>
On Fri, May 15, 2020 at 09:50:32AM +0200, Etienne Carriere wrote:
> > From: Sudeep Holla <sudeep.holla@arm.com>
> >
> > Hi,
> >
> > This patch series adds support for SMCCCv1.2 ARCH_SOC_ID.
> > This doesn't add other changes added in SMCCC v1.2 yet. They will
> > follow these soon along with its first user SPCI/PSA-FF.
> >
> > This is tested using upstream TF-A + the patch[2] fixing the original
> > implementation there.
> >
> >
> > v1[0]->v2[1]:
> > - Incorporated comments from Steven Price in patch 5/5
> > - Fixed build for CONFIG_PSCI_FW=n on some arm32 platforms
> > - Added Steven Price's review tags
> >
> > v2[1]->v3:
> > - Incorporated additional comments from Steven Price in patch 5/5
> > and added his review tags
> > - Refactored SMCCC code from PSCI and moved it under
> > drivers/firmware/smccc/smccc.c
> > - Also moved soc_id.c under drivers/firmware/smccc
> >
> > Regards,
> > Sudeep
>
> Hello Sudeep,
>
> In case it helps. I have successfully tested the 7 patches series
> on some platforms, playing a bit with few configurations.
> Qemu emulator for arm64/cortex-a57 with TF-A (v2.x) as secure firmware.
> Qemu emulator for arm/cortex-a15. OP-TEE (v3.x) as secure firmware.
> A stm32mp15 device (arm/cortex-a7), tested both TF-A (v2.x) and
> OP-TEE (3.7.0, 3.9.0-rc) as runtime secure firmware.
>
> Helper functions arm_smccc_1_1_get_conduit()/arm_smccc_1_1_invoke()
> works as expected AFAICT. No regression seen with older secure
> firmwares.
>
> For the patches 1 to 6, as I poorly tested [v3,7/7] soc ids,
> based on tag next-20200505 [1]:
> Tested-by: Etienne Carriere <etienne.carriere@st.com>
> Reviewed-by: Etienne Carriere <etienne.carriere@st.com>
>
> For [v3,7/7] firmware: smccc: Add ARCH_SOC_ID support
> Acked-by: Etienne Carriere <etienne.carriere@st.com>
>
> [1] 7def1ef0f72c ("Add linux-next specific files for 20200505")
>
Thanks for review and testing Etienne. Much appreciated!
--
Regards,
Sudeep
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/2] firmware: psci: support SMCCC v1.2 for SMCCC conduit
From: Sudeep Holla @ 2020-05-15 9:24 UTC (permalink / raw)
To: Etienne Carriere
Cc: Mark Rutland, lorenzo.pieralisi, maz, linux-kernel, Steven Price,
alexios.zavras, Thomas Gleixner, will,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAN5uoS9gZ7820Fg-6dmm4BO5GW+Y6D3O5Xt3gUQtYVZGafm_XA@mail.gmail.com>
On Thu, May 14, 2020 at 04:56:53PM +0200, Etienne Carriere wrote:
> On Thu, 14 May 2020 at 16:24, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Thu, May 14, 2020 at 10:21:09AM +0200, Etienne Carriere wrote:
> > > Update PSCI driver to support SMCCC v1.2 reported by secure firmware
> > > and indirectly make SMCCC conduit properly set when so. TF-A release
> > > v2.3 implements and reports SMCCC v1.2 since commit [1].
> > >
> > > Link: [1] https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=e34cc0cedca6e229847c232fe58d37fad2610ce9
> > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > > ---
> > > drivers/firmware/psci/psci.c | 14 ++++++++++----
> > > include/linux/psci.h | 1 +
> > > 2 files changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > index 2937d44b5df4..80cf73bea4b0 100644
> > > --- a/drivers/firmware/psci/psci.c
> > > +++ b/drivers/firmware/psci/psci.c
> > > @@ -409,11 +409,17 @@ static void __init psci_init_smccc(void)
> > > feature = psci_features(ARM_SMCCC_VERSION_FUNC_ID);
> > >
> > > if (feature != PSCI_RET_NOT_SUPPORTED) {
> > > - u32 ret;
> > > - ret = invoke_psci_fn(ARM_SMCCC_VERSION_FUNC_ID, 0, 0, 0);
> > > - if (ret == ARM_SMCCC_VERSION_1_1) {
> > > + ver = invoke_psci_fn(ARM_SMCCC_VERSION_FUNC_ID, 0, 0, 0);
> > > +
> > > + switch (ver) {
> > > + case ARM_SMCCC_VERSION_1_1:
> > > psci_ops.smccc_version = SMCCC_VERSION_1_1;
> > > - ver = ret;
> > > + break;
> > > + case ARM_SMCCC_VERSION_1_2:
> > > + psci_ops.smccc_version = SMCCC_VERSION_1_2;
> > > + break;
> > > + default:
> > > + break;
> > > }
> > > }
> > >
> > > diff --git a/include/linux/psci.h b/include/linux/psci.h
> > > index a67712b73b6c..c7d99b7f34ed 100644
> > > --- a/include/linux/psci.h
> > > +++ b/include/linux/psci.h
> > > @@ -24,6 +24,7 @@ bool psci_has_osi_support(void);
> > > enum smccc_version {
> > > SMCCC_VERSION_1_0,
> > > SMCCC_VERSION_1_1,
> > > + SMCCC_VERSION_1_2,
> >
> > I took approach to kill this completely [1] instead of having to keep
> > expanding it for ever.
>
> Yes, I've been pointed to [1]. Discard this change. Sorry for the
> (little) noise.
>
No worries, it's not a noise, just different approach.
--
Regards,
Sudeep
_______________________________________________
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 v3 3/5] ARM: dts: stm32: enable ltdc binding with ili9341 on stm32429-disco board
From: dillon min @ 2020-05-15 9:24 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Alexandre TORGUE, Michael Turquette, Dave Airlie, Linus Walleij,
linux-kernel@vger.kernel.org, open list:DRM PANEL DRIVERS,
linux-stm32@st-md-mailman.stormreply.com, Stephen Boyd,
Rob Herring, thierry.reding@gmail.com, Linux ARM, Daniel Vetter,
Sam Ravnborg, linux-clk, Maxime Coquelin
In-Reply-To: <818b93b4-4431-8338-cd90-ed125ecac615@st.com>
Hi Benjamin,
thanks for reply.
On Fri, May 15, 2020 at 4:31 PM Benjamin GAIGNARD
<benjamin.gaignard@st.com> wrote:
>
>
>
> On 5/14/20 3:07 PM, dillon min wrote:
> > Hi Alexandre,
> >
> > On Thu, May 14, 2020 at 8:53 PM Alexandre Torgue
> > <alexandre.torgue@st.com> wrote:
> >>
> >>
> >> On 5/14/20 10:24 AM, Linus Walleij wrote:
> >>> On Tue, May 12, 2020 at 9:04 AM <dillon.minfei@gmail.com> wrote:
> >>>
> >>>> From: dillon min <dillon.minfei@gmail.com>
> >>>>
> >>>> Enable the ltdc & ili9341 on stm32429-disco board.
> >>>>
> >>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>
> >>> This mostly looks good but...
> >>>
> >>>> +&spi5 {
> >>>> + status = "okay";
> >>>> + pinctrl-0 = <&spi5_pins>;
> >>>> + pinctrl-names = "default";
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <0>;
> >>>> + cs-gpios = <&gpioc 2 GPIO_ACTIVE_LOW>;
> >>>> + dmas = <&dma2 3 2 0x400 0x0>,
> >>>> + <&dma2 4 2 0x400 0x0>;
> >>>> + dma-names = "rx", "tx";
> >>> These DMA assignments seem to be SoC things and should
> >>> rather be in the DTS(I) file where &spi5 is defined, right?
> >>> stm32f429.dtsi I suppose?
> >> I agree with Linus, DMA have to be defined in SoC dtsi. And if a board
> >> doesn't want to use it, we use the "delete-property".
> > Yes, will move to Soc dtsi in next submits.
> >
> > i'm working on write a v4l2-m2m driver for dma2d of stm32 to support
> > pixel conversion
> > alpha blending between foreground and background graphics.
> >
> > as you know, some soc's engineer trying to add this function to drm system.
> >
> > do you know st's planning about soc's hardware accelerator driver on stm32mp?
> > such as chrom-art, will add to drm subsystem via ioctl to access, or to v4l2,
> On stm32mp we do not plan to use chrom-art in drm or v4l2 because it
> does fit
> with userland way of working. We use the GPU to do conversion, scaling,
> blending
> and composition in only one go.
> As explain here [1] DRM subsytem it isn't a solution and v4l2-m2m isn't
> used in any
> mainline compositors like Weston or android surfaceflinger.
>
> Benjamin
>
After check stm32mp's datasheets, they don't have chrom-art ip inside. sorry for
didn't check it yet.
for stm32h7 series with chrom-art, jpeg hardware accelerator inside.
does st has plan to
setup a driver to support it ? i prefer v4l2-m2m should be easier to
implement it.
co work with dcmi, fbdev.
thanks.
best regards.
Dillon
> [1]
> https://www.phoronix.com/scan.php?page=news_item&px=Linux-DRM-No-2D-Accel-API
> >
> > thanks.
> >
> >>> It is likely the same no matter which device is using spi5.
> >>>
> >>> Yours,
> >>> Linus Walleij
> >>>
> > _______________________________________________
> > Linux-stm32 mailing list
> > Linux-stm32@st-md-mailman.stormreply.com
> > https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
_______________________________________________
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] i2c: at91: Restore pinctrl state if can't get scl/sda gpios
From: Wolfram Sang @ 2020-05-15 9:26 UTC (permalink / raw)
To: Codrin Ciubotariu
Cc: alexandre.belloni, linus.walleij, linux-kernel, ludovic.desroches,
linux-i2c, linux-arm-kernel
In-Reply-To: <20200513111322.111114-1-codrin.ciubotariu@microchip.com>
[-- Attachment #1.1: Type: text/plain, Size: 521 bytes --]
On Wed, May 13, 2020 at 02:13:22PM +0300, Codrin Ciubotariu wrote:
> If there is a strict pinmux or if simply the scl/sda gpios are missing,
> the pins will remain in gpio mode, compromizing the I2C bus.
> Change to the default state of the pins before returning the error.
>
> Fixes: a53acc7ebf27 ("i2c: at91: Fix pinmux after devm_gpiod_get() for bus recovery")
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
I squashed it into the other patch and applied it to for-current,
thanks!
[-- 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 5/5] PCI: uniphier: Add error message when failed to get phy
From: Kunihiko Hayashi @ 2020-05-15 9:28 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada
Cc: devicetree, Masami Hiramatsu, linux-pci, linux-kernel, Jassi Brar,
linux-arm-kernel
In-Reply-To: <202005151454.wRtXzaiY%lkp@intel.com>
On 2020/05/15 15:51, kbuild test robot wrote:
> Hi Kunihiko,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on pci/next]
> [also build test WARNING on robh/for-next v5.7-rc5 next-20200514]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url: https://github.com/0day-ci/linux/commits/Kunihiko-Hayashi/PCI-uniphier-Add-features-for-UniPhier-PCIe-host-controller/20200515-125031
> base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
> config: i386-allyesconfig (attached as .config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>
> In file included from include/linux/device.h:15:0,
> from include/linux/pci.h:37,
> from drivers/pci/controller/dwc/pcie-uniphier.c:18:
> drivers/pci/controller/dwc/pcie-uniphier.c: In function 'uniphier_pcie_probe':
>>> drivers/pci/controller/dwc/pcie-uniphier.c:470:16: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=]
> dev_err(dev, "Failed to get phy (%d)n", PTR_ERR(priv->phy));
> ^
> include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
> #define dev_fmt(fmt) fmt
> ^~~
>>> drivers/pci/controller/dwc/pcie-uniphier.c:470:3: note: in expansion of macro 'dev_err'
> dev_err(dev, "Failed to get phy (%d)n", PTR_ERR(priv->phy));
> ^~~~~~~
This should be fixed. I'll fix it in v2.
Thanks,
---
Best Regards
Kunihiko Hayashi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 2/3] drivers: input: keyboard: Add mtk keypad driver
From: Marco Felsch @ 2020-05-15 9:30 UTC (permalink / raw)
To: Fengping Yu
Cc: Dmitry Torokhov, linux-kernel, linux-mediatek, Yingjoe Chen,
Andy Shevchenko, linux-arm-kernel
In-Reply-To: <20200515062007.28346-3-fengping.yu@mediatek.com>
Hi,
On 20-05-15 14:20, Fengping Yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
> This adds matrix keypad support for Mediatek SoCs.
>
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
> ---
> drivers/input/keyboard/Kconfig | 9 ++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/mtk-kpd.c | 231 +++++++++++++++++++++++++++++++
> 3 files changed, 241 insertions(+)
> create mode 100644 drivers/input/keyboard/mtk-kpd.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 28de965a08d5..2b42015e6923 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -782,6 +782,15 @@ config KEYBOARD_BCM
> To compile this driver as a module, choose M here: the
> module will be called bcm-keypad.
>
> +config KEYBOARD_MTK_KPD
> + tristate "MediaTek Keypad Support"
> + depends on OF && HAVE_CLK
Please drop those deps and instead use:
depends on ARCH_MEDIATEK && ARM64
There are still some missing deps:
select CONFIG_REGMAP_MMIO
select INPUT_MATRIXKMAP
and check which clock driver must be enabled:
COMMON_CLK_MT6779_MMSYS
COMMON_CLK_MT6779_IMGSYS
COMMON_CLK_MT6779_IPESYS
COMMON_CLK_MT6779_CAMSYS
COMMON_CLK_MT6779_VDECSYS
COMMON_CLK_MT6779_VENCSYS
COMMON_CLK_MT6779_MFGCFG
COMMON_CLK_MT6779_AUDSYS
> + help
> + Say Y here if you want to use the keypad on MediaTek SoCs.
> + If unsure, say N.
> + To compile this driver as a module, choose M here: the
> + module will be called mtk-kpd.
> +
> config KEYBOARD_MTK_PMIC
> tristate "MediaTek PMIC keys support"
> depends on MFD_MT6397
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 1d689fdd5c00..6c9d852c377e 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
> obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
> obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
> obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
> +obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
> obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
> obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
> obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
> diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
> new file mode 100644
> index 000000000000..66c70c475ee2
> --- /dev/null
> +++ b/drivers/input/keyboard/mtk-kpd.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 MediaTek Inc.
> + * Author Terry Chang <terry.chang@mediatek.com>
> + */
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/input/matrix_keypad.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define MTK_KPD_NAME "mtk-kpd"
> +#define MTK_KPD_MEM 0x0004
> +#define MTK_KPD_DEBOUNCE 0x0018
> +#define MTK_KPD_DEBOUNCE_MASK GENMASK(13, 0)
> +#define MTK_KPD_DEBOUNCE_MAX_US 256000 /*256ms */
Thanks for aligning the defines but the 256ms comment is still useless
as Andy already said.
> +#define MTK_KPD_NUM_MEMS 5
> +#define MTK_KPD_NUM_BITS 136 /* 4*32+8 MEM5 only use 8 BITS */
> +
> +struct mtk_keypad {
> + struct regmap *regmap;
> + struct input_dev *input_dev;
> + struct clk *clk;
> + void __iomem *base;
> + bool wakeup;
> + u32 n_rows;
> + u32 n_cols;
> + DECLARE_BITMAP(keymap_state, MTK_KPD_NUM_BITS);
> +};
> +
> +static const struct regmap_config keypad_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = sizeof(u32),
> + .max_register = 36,
> +};
> +
> +static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
> +{
> + struct mtk_keypad *keypad = dev_id;
> + unsigned short *keycode = keypad->input_dev->keycode;
> + DECLARE_BITMAP(new_state, MTK_KPD_NUM_BITS);
> + DECLARE_BITMAP(change, MTK_KPD_NUM_BITS);
> + int bit_nr;
> + int pressed;
> + unsigned short code;
> +
> + regmap_raw_read(keypad->regmap, MTK_KPD_MEM, new_state, MTK_KPD_NUM_MEMS);
> +
> + bitmap_xor(change, new_state, keypad->keymap_state, MTK_KPD_NUM_BITS);
> +
> + for_each_set_bit(bit_nr, change, MTK_KPD_NUM_BITS) {
> + /* 1: not pressed, 0: pressed */
> + pressed = !test_bit(bit_nr, new_state);
> + dev_dbg(&keypad->input_dev->dev, "%s",
> + pressed ? "pressed" : "released");
> +
> + /* 32bit register only use low 16bit as keypad mem register */
> + code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
> +
> + input_report_key(keypad->input_dev, code, pressed);
> + input_sync(keypad->input_dev);
> +
> + dev_dbg(&keypad->input_dev->dev,
> + "report Linux keycode = %d\n", code);
> + }
> +
> + bitmap_copy(keypad->keymap_state, new_state, MTK_KPD_NUM_BITS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int kpd_probe_clock(struct platform_device *pdev, struct clk *clk)
> +{
> + int ret;
> +
> + clk = devm_clk_get(&pdev->dev, "kpd");
> + if (IS_ERR(clk))
> + return clk;
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(&pdev->dev,
> + (void (*)(void *))clk_disable_unprepare,
^
Please avoid this and
instead use a driver
internal function.
> + clk);
> +
> + return ret;
> +}
That was not my intention. You can do all the stuff within the probe()
routine.
> +
> +static int kpd_pdrv_probe(struct platform_device *pdev)
> +{
> + struct mtk_keypad *keypad;
> + struct pinctrl *keypad_pinctrl;
> + struct pinctrl_state *kpd_default;
> + unsigned int irqnr;
> + u32 debounce;
> + int ret;
> +
> + keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> + if (!keypad)
> + return -ENOMEM;
> +
> + keypad->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(keypad->base))
> + return PTR_ERR(keypad->base);
> +
> + keypad->regmap = devm_regmap_init_mmio(&pdev->dev,
> + keypad->base,
> + &keypad_regmap_cfg);
> + if (IS_ERR(keypad->regmap)) {
> + dev_err(&pdev->dev,
> + "regmap init failed:%ld\n", PTR_ERR(keypad->regmap));
> + return PTR_ERR(keypad->regmap);
> + }
> +
> + bitmap_fill(keypad->keymap_state, MTK_KPD_NUM_BITS);
> +
> + keypad->input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!keypad->input_dev) {
> + dev_err(&pdev->dev, "Failed to allocate input dev\n");
> + return -ENOMEM;
> + }
> +
> + keypad->input_dev->name = MTK_KPD_NAME;
> + keypad->input_dev->id.bustype = BUS_HOST;
> +
> + ret = matrix_keypad_parse_properties(&pdev->dev, &keypad->n_rows,
> + &keypad->n_cols);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to parse keypad params\n");
> + return ret;
> + }
> +
> + if (device_property_read_u32(&pdev->dev, "mediatek,debounce-us",
> + &debounce))
Did you executed checkpatch.pl here? There are a few more style issues
within the patch.
> + debounce = 16000;
> +
> + if (debounce > MTK_KPD_DEBOUNCE_MAX_US) {
> + dev_err(&pdev->dev, "Debounce time exceeds the maximum allowed time 256ms\n");
> + return -EINVAL;
> + }
> +
> + keypad->wakeup = device_property_read_bool(&pdev->dev, "wakeup-source");
> +
> + dev_dbg(&pdev->dev, "n_row=%d n_col=%d debounce=%d\n",
> + keypad->n_rows, keypad->n_cols, debounce);
> +
> + ret = matrix_keypad_build_keymap(NULL, NULL,
> + keypad->n_rows,
> + keypad->n_cols,
> + NULL,
> + keypad->input_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to build keymap\n");
> + return ret;
> + }
> +
> + input_set_drvdata(keypad->input_dev, keypad);
This is useless.
> +
> + regmap_write(keypad->regmap, KP_DEBOUNCE,
> + debounce * 32 / 1000 & MTK_KPD_DEBOUNCE_MASK);
> +
> + if (kpd_probe_clock(pdev, keypad->clk)) {
> + dev_err(&pdev->dev, "Failed to enable clock\n");
> + return ret;
> + }
> +
> + keypad_pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (IS_ERR(keypad_pinctrl))
> + return PTR_ERR(keypad_pinctrl);
> +
> + kpd_default = pinctrl_lookup_state(keypad_pinctrl, "default");
> + if (IS_ERR(kpd_default)) {
> + dev_err(&pdev->dev, "No default pinctrl state\n");
> + return PTR_ERR(kpd_default);
> + }
> +
> + pinctrl_select_state(keypad_pinctrl, kpd_default);
> +
> + irqnr = platform_get_irq(pdev, 0);
> + if (irqnr < 0) {
> + dev_err(&pdev->dev, "Failed to get irq\n");
Andy already said that platform_get_irq() prints a error so you can just
'return -irqnr'.
> + return -irqnr;
> + }
> +
> + ret = devm_request_threaded_irq(&pdev->dev, irqnr,
> + NULL, kpd_irq_handler, 0,
> + MTK_KPD_NAME, keypad);
Still missaligned, pls use checkpatch.pl
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
> + irqnr, ret);
> + return ret;
> + }
> +
> + ret = input_register_device(keypad->input_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register device\n");
> + return ret;
> + }
> +
> + device_init_wakeup(&pdev->dev, keypad->wakeup);
> +
> + return 0;
Here we can 'return device_init_wakeup(&pdev->dev, keypad->wakeup);'
> +}
> +
> +static const struct of_device_id kpd_of_match[] = {
> + {.compatible = "mediatek, mt6779-keypad"},
> + {.compatible = "mediatek,kp"},
^
Missing whitespace
> + {/*sentinel*/}
^ ^
Missing whitespaces
Regards,
Marco
> +};
> +
> +static struct platform_driver kpd_pdrv = {
> + .probe = kpd_pdrv_probe,
> + .driver = {
> + .name = MTK_KPD_NAME,
> + .of_match_table = kpd_of_match,
> + },
> +};
> +module_platform_driver(kpd_pdrv);
> +
> +MODULE_AUTHOR("Mediatek Corporation");
> +MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
> +MODULE_LICENSE("GPL");
> --
> 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] firmware: arm_scmi: fix SMCCC_RET_NOT_SUPPORTED management
From: Sudeep Holla @ 2020-05-15 9:34 UTC (permalink / raw)
To: Etienne Carriere; +Cc: linux-kernel, linux-arm-kernel, Sudeep Holla
In-Reply-To: <CAN5uoS_bimZsFqwaODRRWeCe15JMepQa2z9J0+dq7qNfwxRsug@mail.gmail.com>
On Thu, May 14, 2020 at 05:06:22PM +0200, Etienne Carriere wrote:
> On Thu, 14 May 2020 at 16:29, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Thu, May 14, 2020 at 10:24:28AM +0200, Etienne Carriere wrote:
> > > Fix management of argument a0 output value of arm_smccc_1_1_invoke() that
> > > should consider only SMCCC_RET_NOT_SUPPORTED as reporting an unsupported
> > > function ID as correctly stated in the inline comment.
> > >
> >
> > I agree on the comment part, but ...
> >
> > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > > ---
> > > drivers/firmware/arm_scmi/smc.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
> > > index 49bc4b0e8428..637ad439545f 100644
> > > --- a/drivers/firmware/arm_scmi/smc.c
> > > +++ b/drivers/firmware/arm_scmi/smc.c
> > > @@ -115,7 +115,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
> > > mutex_unlock(&scmi_info->shmem_lock);
> > >
> > > /* Only SMCCC_RET_NOT_SUPPORTED is valid error code */
> > > - if (res.a0)
> > > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> > > return -EOPNOTSUPP;
> >
> > Now this will return 0 for all values other than SMCCC_RET_NOT_SUPPORTED.
> > Is that what we need ? Or do you see non-zero res.a0 for a success case ?
> > If later, we need some fixing, otherwise it is safer to leave it as is
> > IMO.
>
> Firmware following SMCCC v1.x for some OEM/SiP invocation may simply
> not modify invocation register argument a0 on invocation with a
> SCMI-SMC transport function ID.
Yikes, I need to check specification again for this. I will also
check with the firmware implementation team/
> Resulting in res.a0 == scmi_info->func_id here. Which is, by SMCCC
> v1.x not an error.
>
But that may get fatal the result in some other cases, not here for sure.
But I would rather flag that as error so that it is fixed. Anyways I will
check on this again/
> From SMCCC v1.x only SMCCC_RET_NOT_SUPPORTED (-1 signed extended is a
> reserved ) is a generic return error whatever function ID value.
>
Not really, there are couple more I think now. But yes I need to check
on the generic return part.
> Or consider part of the SCMI-SMC transport API that output arg a0
> shall be 0 on success, SMCCC_RET_NOT_SUPPORTED if function ID is not
> supported and any non-zero value for non-generic **error** codes.
>
I prefer that. Anyways I will check and if anything changes I will ping
back on this thread.
--
Regards,
Sudeep
_______________________________________________
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 v3 3/5] ARM: dts: stm32: enable ltdc binding with ili9341 on stm32429-disco board
From: Benjamin GAIGNARD @ 2020-05-15 9:34 UTC (permalink / raw)
To: dillon min
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Alexandre TORGUE, Michael Turquette, Dave Airlie, Linus Walleij,
linux-kernel@vger.kernel.org, open list:DRM PANEL DRIVERS,
linux-stm32@st-md-mailman.stormreply.com, Stephen Boyd,
Rob Herring, thierry.reding@gmail.com, Linux ARM, Daniel Vetter,
Sam Ravnborg, linux-clk, Maxime Coquelin
In-Reply-To: <CAL9mu0L6d2V5qypPfOSeMdhc=DdHkcsaF4GysNG-vfDe5npkhw@mail.gmail.com>
On 5/15/20 11:24 AM, dillon min wrote:
> Hi Benjamin,
>
> thanks for reply.
>
> On Fri, May 15, 2020 at 4:31 PM Benjamin GAIGNARD
> <benjamin.gaignard@st.com> wrote:
>>
>>
>> On 5/14/20 3:07 PM, dillon min wrote:
>>> Hi Alexandre,
>>>
>>> On Thu, May 14, 2020 at 8:53 PM Alexandre Torgue
>>> <alexandre.torgue@st.com> wrote:
>>>>
>>>> On 5/14/20 10:24 AM, Linus Walleij wrote:
>>>>> On Tue, May 12, 2020 at 9:04 AM <dillon.minfei@gmail.com> wrote:
>>>>>
>>>>>> From: dillon min <dillon.minfei@gmail.com>
>>>>>>
>>>>>> Enable the ltdc & ili9341 on stm32429-disco board.
>>>>>>
>>>>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>
>>>>> This mostly looks good but...
>>>>>
>>>>>> +&spi5 {
>>>>>> + status = "okay";
>>>>>> + pinctrl-0 = <&spi5_pins>;
>>>>>> + pinctrl-names = "default";
>>>>>> + #address-cells = <1>;
>>>>>> + #size-cells = <0>;
>>>>>> + cs-gpios = <&gpioc 2 GPIO_ACTIVE_LOW>;
>>>>>> + dmas = <&dma2 3 2 0x400 0x0>,
>>>>>> + <&dma2 4 2 0x400 0x0>;
>>>>>> + dma-names = "rx", "tx";
>>>>> These DMA assignments seem to be SoC things and should
>>>>> rather be in the DTS(I) file where &spi5 is defined, right?
>>>>> stm32f429.dtsi I suppose?
>>>> I agree with Linus, DMA have to be defined in SoC dtsi. And if a board
>>>> doesn't want to use it, we use the "delete-property".
>>> Yes, will move to Soc dtsi in next submits.
>>>
>>> i'm working on write a v4l2-m2m driver for dma2d of stm32 to support
>>> pixel conversion
>>> alpha blending between foreground and background graphics.
>>>
>>> as you know, some soc's engineer trying to add this function to drm system.
>>>
>>> do you know st's planning about soc's hardware accelerator driver on stm32mp?
>>> such as chrom-art, will add to drm subsystem via ioctl to access, or to v4l2,
>> On stm32mp we do not plan to use chrom-art in drm or v4l2 because it
>> does fit
>> with userland way of working. We use the GPU to do conversion, scaling,
>> blending
>> and composition in only one go.
>> As explain here [1] DRM subsytem it isn't a solution and v4l2-m2m isn't
>> used in any
>> mainline compositors like Weston or android surfaceflinger.
>>
>> Benjamin
>>
> After check stm32mp's datasheets, they don't have chrom-art ip inside. sorry for
> didn't check it yet.
>
> for stm32h7 series with chrom-art, jpeg hardware accelerator inside.
> does st has plan to
> setup a driver to support it ? i prefer v4l2-m2m should be easier to
> implement it.
> co work with dcmi, fbdev.
ST doesn't plan to create a driver for chrom-art because nothing in
mainline
userland could use it.
Benjamin
>
> thanks.
>
> best regards.
>
> Dillon
>> [1]
>> https://www.phoronix.com/scan.php?page=news_item&px=Linux-DRM-No-2D-Accel-API
>>> thanks.
>>>
>>>>> It is likely the same no matter which device is using spi5.
>>>>>
>>>>> Yours,
>>>>> Linus Walleij
>>>>>
>>> _______________________________________________
>>> Linux-stm32 mailing list
>>> Linux-stm32@st-md-mailman.stormreply.com
>>> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ACPI: GED: add support for _Exx / _Lxx handler methods
From: Ard Biesheuvel @ 2020-05-15 9:36 UTC (permalink / raw)
To: linux-kernel
Cc: Rafael J. Wysocki, stable, linux-acpi, Ard Biesheuvel,
linux-arm-kernel, Len Brown
Per the ACPI spec, interrupts in the range [0, 255] may be handled
in AML using individual methods whose naming is based on the format
_Exx or _Lxx, where xx is the hex representation of the interrupt
index.
Add support for this missing feature to our ACPI GED driver.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
Cc: <stable@vger.kernel.org> # v4.9+
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
drivers/acpi/evged.c | 22 +++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/evged.c b/drivers/acpi/evged.c
index aba0d0027586..6d7a522952bf 100644
--- a/drivers/acpi/evged.c
+++ b/drivers/acpi/evged.c
@@ -79,6 +79,8 @@ static acpi_status acpi_ged_request_interrupt(struct acpi_resource *ares,
struct resource r;
struct acpi_resource_irq *p = &ares->data.irq;
struct acpi_resource_extended_irq *pext = &ares->data.extended_irq;
+ char ev_name[5];
+ u8 trigger;
if (ares->type == ACPI_RESOURCE_TYPE_END_TAG)
return AE_OK;
@@ -87,14 +89,28 @@ static acpi_status acpi_ged_request_interrupt(struct acpi_resource *ares,
dev_err(dev, "unable to parse IRQ resource\n");
return AE_ERROR;
}
- if (ares->type == ACPI_RESOURCE_TYPE_IRQ)
+ if (ares->type == ACPI_RESOURCE_TYPE_IRQ) {
gsi = p->interrupts[0];
- else
+ trigger = p->triggering;
+ } else {
gsi = pext->interrupts[0];
+ trigger = p->triggering;
+ }
irq = r.start;
- if (ACPI_FAILURE(acpi_get_handle(handle, "_EVT", &evt_handle))) {
+ switch (gsi) {
+ case 0 ... 255:
+ sprintf(ev_name, "_%c%02hhX",
+ trigger == ACPI_EDGE_SENSITIVE ? 'E' : 'L', gsi);
+
+ if (ACPI_SUCCESS(acpi_get_handle(handle, ev_name, &evt_handle)))
+ break;
+ /* fall through */
+ default:
+ if (ACPI_SUCCESS(acpi_get_handle(handle, "_EVT", &evt_handle)))
+ break;
+
dev_err(dev, "cannot locate _EVT method\n");
return AE_ERROR;
}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v2] arm64: dts: ti: k3-am654-main: Update otap-del-sel values
From: Faiz Abbas @ 2020-05-15 9:37 UTC (permalink / raw)
To: linux-kernel, devicetree, linux-arm-kernel; +Cc: nm, t-kristo, robh+dt
In-Reply-To: <20200507181526.12529-1-faiz_abbas@ti.com>
Tero,
On 07/05/20 11:45 pm, Faiz Abbas wrote:
> According to the latest AM65x Data Manual[1], a different output tap
> delay value is optimum for a given speed mode. Update these values.
>
> [1] http://www.ti.com/lit/gpn/am6526
>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
> v2: Rebased to the latest mainline kernel
>
Gentle ping.
Thanks,
Faiz
_______________________________________________
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 1/2] MAINTAINERS: add maintainer for mediatek i2c controller driver
From: Wolfram Sang @ 2020-05-15 9:40 UTC (permalink / raw)
To: Qii Wang
Cc: devicetree, srv_heupstream, leilk.liu, linux-kernel,
linux-mediatek, linux-i2c, linux-arm-kernel
In-Reply-To: <1589461844-15614-2-git-send-email-qii.wang@mediatek.com>
[-- Attachment #1.1: Type: text/plain, Size: 233 bytes --]
On Thu, May 14, 2020 at 09:09:04PM +0800, Qii Wang wrote:
> Add Qii Wang as maintainer for mediatek i2c controller driver.
>
> Signed-off-by: Qii Wang <qii.wang@mediatek.com>
Applied to for-current, thanks for stepping up!
[-- 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 2/2] i2c: mediatek: Add i2c ac-timing adjust support
From: Wolfram Sang @ 2020-05-15 9:40 UTC (permalink / raw)
To: Qii Wang
Cc: devicetree, srv_heupstream, leilk.liu, linux-kernel,
linux-mediatek, linux-i2c, linux-arm-kernel
In-Reply-To: <1589461844-15614-3-git-send-email-qii.wang@mediatek.com>
[-- Attachment #1.1: Type: text/plain, Size: 252 bytes --]
On Thu, May 14, 2020 at 09:09:05PM +0800, Qii Wang wrote:
> This patch adds a algorithm to calculate some ac-timing parameters
> which can fully meet I2C Spec.
>
> Signed-off-by: Qii Wang <qii.wang@mediatek.com>
Applied to for-next, thanks!
[-- 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] firmware: arm_scmi: fix SMCCC_RET_NOT_SUPPORTED management
From: Etienne Carriere @ 2020-05-15 9:57 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-kernel,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20200515093424.GC23671@bogus>
> > Or consider part of the SCMI-SMC transport API that output arg a0
> > shall be 0 on success, SMCCC_RET_NOT_SUPPORTED if function ID is not
> > supported and any non-zero value for non-generic **error** codes.
> >
>
> I prefer that. Anyways I will check and if anything changes I will ping
> back on this thread.
I don't have a strong opinion on whether considering or not 0 as
success, for whatever the function ID used here for SCMI message
notification.
We can assume at least 0 is default returned in a0 when the function
ID is used in SCMI SMC transport.
Thanks for the feedback.
> --
> Regards,
> Sudeep
_______________________________________________
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/4] thermal: cpuidle: Register cpuidle cooling device
From: Sudeep Holla @ 2020-05-15 9:58 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Lorenzo Pieralisi, open list:CPU IDLE TIME MANAGEMENT FRAMEWORK,
Rafael J. Wysocki, open list, open list:CPUIDLE DRIVER - ARM PSCI,
Sudeep Holla, rui.zhang, Lukasz Luba
In-Reply-To: <f3cee834-4946-10bd-a504-df6cf62d9e90@linaro.org>
On Mon, May 04, 2020 at 08:00:01PM +0200, Daniel Lezcano wrote:
>
> Hi,
>
> On 29/04/2020 23:01, Daniel Lezcano wrote:
> > On 29/04/2020 22:02, Lukasz Luba wrote:
> >>
> >>
> >> On 4/29/20 11:36 AM, Daniel Lezcano wrote:
> >>> The cpuidle driver can be used as a cooling device by injecting idle
> >>> cycles. The DT binding for the idle state added an optional
> >>>
> >>> When the property is set, register the cpuidle driver with the idle
> >>> state node pointer as a cooling device. The thermal framework will do
> >>> the association automatically with the thermal zone via the
> >>> cooling-device defined in the device tree cooling-maps section.
> >>>
> >>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> >>> ---
> >>> - V4:
> >>> - Do not check the return value as the function does no longer
> >>> return one
> >>> ---
> >
> > [ ... ]
> >
> >> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
> >
> > Thanks Lukasz for the review.
> >
> > Rafael, as Lorenzo and Sudeep are not responsive, could you consider ack
> > this patch so I can merge the series through the thermal tree ?
>
> Gentle ping ... Sudeep, Lorenzo or Rafael ?
>
Sorry for the delay. I ignore this as it was generic and I was fine with
it. Didn't know you were waiting me, sorry for that.
FWIW:
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
_______________________________________________
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