Re: [RFC][PATCH 7/7] sched: Replace rq::wake_list
On Tue, Jun 09, 2020 at 02:25:09PM -0700, Guenter Roeck wrote:
> >
> > Still occurring on Linus' tree. This needs to be fixed. (And not by
> > removing
> > support for randstruct; that's not a "fix"...)
> >
>
> How about the hack below ?
My test suite failed due to this bug (on my allmodconfig test).
Your hack appears to fix it. I've applied it to my "fixes" patches applied to
my test suite before building my kernels.
Thanks!
-- Steve
>
> Guenter
>
> ---
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index c5d96e3e7fff..df1cbb04f9b3 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -629,6 +629,15 @@ struct wake_q_node {
> struct wake_q_node *next;
> };
>
> +/*
> + * Hack around assumption that wake_entry_type follows wake_entry even with
> + * CONFIG_GCC_PLUGIN_RANDSTRUCT=y.
> + */
> +struct _wake_entry {
> + struct llist_node wake_entry;
> + unsigned intwake_entry_type;
> +};
> +
> struct task_struct {
> #ifdef CONFIG_THREAD_INFO_IN_TASK
> /*
> @@ -653,8 +662,9 @@ struct task_struct {
> unsigned intptrace;
>
> #ifdef CONFIG_SMP
> - struct llist_node wake_entry;
> - unsigned intwake_entry_type;
> + struct _wake_entry _we;
> +#define wake_entry _we.wake_entry
> +#define wake_entry_type _we.wake_entry_type
> int on_cpu;
> #ifdef CONFIG_THREAD_INFO_IN_TASK
> /* Current CPU: */
Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs
On Thu, Jun 18, 2020 at 10:32:06AM -0700, Paul E. McKenney wrote:
> On Thu, Jun 18, 2020 at 07:25:04PM +0200, Uladzislau Rezki wrote:
> > > > + // Handle two first channels.
> > > > + for (i = 0; i < FREE_N_CHANNELS; i++) {
> > > > + for (; bkvhead[i]; bkvhead[i] = bnext) {
> > > > + bnext = bkvhead[i]->next;
> > > > + debug_rcu_bhead_unqueue(bkvhead[i]);
> > > > +
> > > > + rcu_lock_acquire(_callback_map);
> > > > + if (i == 0) { // kmalloc() / kfree().
> > > > + trace_rcu_invoke_kfree_bulk_callback(
> > > > + rcu_state.name,
> > > > bkvhead[i]->nr_records,
> > > > + bkvhead[i]->records);
> > > > +
> > > > + kfree_bulk(bkvhead[i]->nr_records,
> > > > + bkvhead[i]->records);
> > > > + } else { // vmalloc() / vfree().
> > > > + for (j = 0; j < bkvhead[i]->nr_records;
> > > > j++) {
> > > > + trace_rcu_invoke_kfree_callback(
> > > > + rcu_state.name,
> > > > + bkvhead[i]->records[j],
> > > > 0);
> > > > +
> > > > + vfree(bkvhead[i]->records[j]);
> > > > + }
> > > > + }
> > > > + rcu_lock_release(_callback_map);
> > >
> > > Not an emergency, but did you look into replacing this "if" statement
> > > with an array of pointers to functions implementing the legs of the
> > > "if" statement? If nothing else, this would greatly reduced indentation.
> > >
> > >
> > > I am taking this as is, but if you have not already done so, could you
> > > please look into this for a follow-up patch?
> > >
> > I do not think it makes sense, because it would require to check each
> > pointer in the array, what can lead to many branching, i.e. "if-else"
> > instructions.
>
> Mightn't the compiler simply unroll the outer loop? Then the first
> unrolled iteration of that loop would contain the then-clause and
> the second unrolled iteration would contain the else-clause. At that
> point, there would be no checking, just direct calls.
>
> Or am I missing something?
>
If we mix pointers, then we can do free per pointer only. I mean in that
case we will not be able to use kfree_bulk() interface for freeing SLAB
memory and the code would converted to something like:
while (nr_objects_in_array > 0) {
if (is_vmalloc_addr(array[X]))
vfree(array[X]);
else
kfree(array[X]);
}
> > Paul, thank you to take it in!
>
> Thank you for persisting!
>
Welcome :)
--
Vlad Rezki
Re: KASAN: null-ptr-deref Write in media_request_close
syzbot has bisected this bug to:
commit 016baa59bf9f6c2550480b73f18100285e3a4fd2
Author: Ezequiel Garcia
Date: Tue Apr 14 22:06:24 2020 +
media: Kconfig: Don't expose the Request API option
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=108d714910
start commit: 7ae77150 Merge tag 'powerpc-5.8-1' of git://git.kernel.org..
git tree: upstream
final crash:https://syzkaller.appspot.com/x/report.txt?x=128d714910
console output: https://syzkaller.appspot.com/x/log.txt?x=148d714910
kernel config: https://syzkaller.appspot.com/x/.config?x=be4578b3f1083656
dashboard link: https://syzkaller.appspot.com/bug?extid=6bed2d543cf7e48b822b
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17b3fc3510
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12fbb6f110
Reported-by: syzbot+6bed2d543cf7e48b8...@syzkaller.appspotmail.com
Fixes: 016baa59bf9f ("media: Kconfig: Don't expose the Request API option")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
Re: [PATCH 0/3] zone-append support in aio and io-uring
On Wed, Jun 17, 2020 at 11:56:34PM -0700, Christoph Hellwig wrote: On Wed, Jun 17, 2020 at 10:53:36PM +0530, Kanchan Joshi wrote: This patchset enables issuing zone-append using aio and io-uring direct-io interface. For aio, this introduces opcode IOCB_CMD_ZONE_APPEND. Application uses start LBA of the zone to issue append. On completion 'res2' field is used to return zone-relative offset. For io-uring, this introduces three opcodes: IORING_OP_ZONE_APPEND/APPENDV/APPENDV_FIXED. Since io_uring does not have aio-like res2, cqe->flags are repurposed to return zone-relative offset And what exactly are the semantics supposed to be? Remember the unix file abstractions does not know about zones at all. I really don't think squeezing low-level not quite block storage protocol details into the Linux read/write path is a good idea. I was thinking of raw block-access to zone device rather than pristine file abstraction. And in that context, semantics, at this point, are unchanged (i.e. same as direct writes) while flexibility of async-interface gets added. Synchronous-writes on single-zone sound fine, but synchronous-appends on single-zone do not sound that fine. What could be a useful addition is a way for O_APPEND/RWF_APPEND writes to report where they actually wrote, as that comes close to Zone Append while still making sense at our usual abstraction level for file I/O. Thanks for suggesting this. O and RWF_APPEND may not go well with block access as end-of-file will be picked from dev inode. But perhaps a new flag like RWF_ZONE_APPEND can help to transform writes (aio or uring) into append without introducing new opcodes. And, I think, this can fit fine on file-abstraction of ZoneFS as well.
Re: [PATCH v4 5/5] spi: spi-geni-qcom: Don't keep a local state variable
Quoting Douglas Anderson (2020-06-18 08:06:26) > The variable "cur_mcmd" kept track of our current state (idle, xfer, > cs, cancel). We don't really need it, so get rid of it. Instead: > * Use separate condition variables for "chip select done", "cancel > done", and "abort done". This is important so that if a "done" > comes through (perhaps some previous interrupt finally came through) > it can't confuse the cancel/abort function. > * Use the "done" interrupt only for when a chip select or transfer is > done and we can tell the difference by looking at whether "cur_xfer" > is NULL. > > This is mostly a no-op change. However, it is possible it could fix > an issue where a super delayed interrupt for a cancel command could > have confused our waiting for an abort command. > > Signed-off-by: Douglas Anderson > --- Reviewed-by: Stephen Boyd
Re: [PATCH 1/2] media: v4l UAPI: add V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS
Hi Yunfei, On Wed, Jun 17, 2020 at 09:49:27AM +0800, Yunfei Dong wrote: > This patch adds support for the V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS > flag. This flag is used for RO Request. I think this patch series lacks two major things: - a cover letter explaining the feature and what it is needed/useful for, - a user - is there an upstream driver which would implement this feature and benefit from it? Best regards, Tomasz
Re: [PATCH v4 2/5] spi: spi-geni-qcom: Mo' betta locking
Quoting Douglas Anderson (2020-06-18 08:06:23)
> If you added a bit of a delay (like a trace_printk) into the ISR for
> the spi-geni-qcom driver, you would suddenly start seeing some errors
> spit out. The problem was that, though the ISR itself held a lock,
> other parts of the driver didn't always grab the lock.
>
> One example race was this:
> CPU0 CPU1
>
> spi_geni_set_cs()
>mas->cur_mcmd = CMD_CS;
>geni_se_setup_m_cmd(...)
>wait_for_completion_timeout(_done);
>
>geni_spi_isr()
> complete(_done);
>
>pm_runtime_put(mas->dev);
> ... // back to SPI core
> spi_geni_transfer_one()
>setup_fifo_xfer()
> mas->cur_mcmd = CMD_XFER;
> mas->cur_cmd = CMD_NONE; //
> bad!
> return IRQ_HANDLED;
>
> Let's fix this. Before we start messing with hardware, we'll grab the
> lock to make sure that the IRQ handler from some previous command has
> really finished. We don't need to hold the lock unless we're in a
> state where more interrupts can come in, but we at least need to make
> sure the previous IRQ is done. This lock is used exclusively to
> prevent the IRQ handler and non-IRQ from stomping on each other. The
> SPI core handles all other mutual exclusion.
>
> As part of this, we change the way that the IRQ handler detects
> spurious interrupts. Previously we checked for our state variable
> being set to IRQ_NONE, but that was done outside the spinlock. We
> could move it into the spinlock, but instead let's just change it to
> look for the lack of any IRQ status bits being set. This can be done
> outside the lock--the hardware certainly isn't grabbing or looking at
> the spinlock when it updates its status register.
>
> It's possible that this will fix real (but very rare) errors seen in
> the field that look like:
> irq ...: nobody cared (try booting with the "irqpoll" option)
>
> NOTE: an alternate strategy considered here was to always make the
> complete() / spi_finalize_current_transfer() the very last thing in
> our IRQ handler. With such a change you could consider that we could
> be "lockless". In that case, though, we'd have to be very careful w/
> memory barriers so we made sure we didn't have any bugs with weakly
> ordered memory. Using spinlocks makes the driver much easier to
> understand.
>
> Fixes: 561de45f72bd ("spi: spi-geni-qcom: Add SPI driver support for GENI
> based QUP")
> Signed-off-by: Douglas Anderson
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH v3 2/6] drm: msm: a6xx: send opp instead of a frequency
On Fri, Jun 5, 2020 at 9:26 PM Sharat Masetty wrote:
>
> This patch changes the plumbing to send the devfreq recommended opp rather
> than the frequency. Also consolidate and rearrange the code in a6xx to set
> the GPU frequency and the icc vote in preparation for the upcoming
> changes for GPU->DDR scaling votes.
>
> Signed-off-by: Sharat Masetty
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 62
> +++
> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +-
> drivers/gpu/drm/msm/msm_gpu.c | 3 +-
> drivers/gpu/drm/msm/msm_gpu.h | 3 +-
> 4 files changed, 38 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 748cd37..2d8124b 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -100,17 +100,30 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu)
> A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_CLK_OFF));
> }
>
> -static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
> +void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
> {
> - struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
> - struct adreno_gpu *adreno_gpu = _gpu->base;
> - struct msm_gpu *gpu = _gpu->base;
> - int ret;
> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> + struct a6xx_gmu *gmu = _gpu->gmu;
> + u32 perf_index;
> + unsigned long gpu_freq;
> + int ret = 0;
> +
> + gpu_freq = dev_pm_opp_get_freq(opp);
> +
> + if (gpu_freq == gmu->freq)
> + return;
> +
> + for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++)
> + if (gpu_freq == gmu->gpu_freqs[perf_index])
> + break;
> +
> + gmu->current_perf_index = perf_index;
>
> gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0);
>
> gmu_write(gmu, REG_A6XX_GMU_DCVS_PERF_SETTING,
> - ((3 & 0xf) << 28) | index);
> + ((3 & 0xf) << 28) | perf_index);
>
> /*
> * Send an invalid index as a vote for the bus bandwidth and let the
> @@ -126,7 +139,7 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int
> index)
> if (ret)
> dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret);
>
> - gmu->freq = gmu->gpu_freqs[index];
> + gmu->freq = gmu->gpu_freqs[perf_index];
>
> /*
> * Eventually we will want to scale the path vote with the frequency
> but
> @@ -135,25 +148,6 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu,
> int index)
> icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> }
>
> -void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
> -{
> - struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> - struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> - struct a6xx_gmu *gmu = _gpu->gmu;
> - u32 perf_index = 0;
> -
> - if (freq == gmu->freq)
> - return;
> -
> - for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++)
> - if (freq == gmu->gpu_freqs[perf_index])
> - break;
> -
> - gmu->current_perf_index = perf_index;
> -
> - __a6xx_gmu_set_freq(gmu, perf_index);
> -}
this does end up conflicting a bit with some of the newer stuff that
landed this cycle, in particular "drm/msm/a6xx: HFI v2 for A640 and
A650"
Adding Jonathan on CC since I think he will want to test this on
a650/a640 as well..
BR,
-R
> -
> unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu)
> {
> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> @@ -708,6 +702,19 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
> a6xx_gmu_rpmh_off(gmu);
> }
>
> +static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu
> *gmu)
> +{
> + struct dev_pm_opp *gpu_opp;
> + unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index];
> +
> + gpu_opp = dev_pm_opp_find_freq_exact(>pdev->dev, gpu_freq, true);
> + if (IS_ERR_OR_NULL(gpu_opp))
> + return;
> +
> + a6xx_gmu_set_freq(gpu, gpu_opp);
> + dev_pm_opp_put(gpu_opp);
> +}
> +
> int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
> {
> struct adreno_gpu *adreno_gpu = _gpu->base;
> @@ -759,8 +766,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
> gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_MASK, ~A6XX_HFI_IRQ_MASK);
> enable_irq(gmu->hfi_irq);
>
> - /* Set the GPU to the current freq */
> - __a6xx_gmu_set_freq(gmu, gmu->current_perf_index);
> + a6xx_gmu_set_initial_freq(gpu, gmu);
>
> /*
> * "enable" the GX power domain which won't actually do anything but
> it
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
>
Re: [PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
Hi Niklas,
I love your patch! Yet something to improve:
[auto build test ERROR on block/for-next]
[also build test ERROR on linus/master v5.8-rc1 next-20200618]
[cannot apply to hch-configfs/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Niklas-Cassel/nvme-remove-workarounds-for-gcc-bug-wrt-unnamed-fields-in-initializers/20200618-223525
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git
for-next
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from arch/m68k/include/asm/io_mm.h:25,
from arch/m68k/include/asm/io.h:8,
from include/linux/scatterlist.h:9,
from include/linux/dma-mapping.h:11,
from include/rdma/ib_verbs.h:44,
from include/rdma/mr_pool.h:8,
from drivers/nvme/host/rdma.c:10:
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsb':
arch/m68k/include/asm/raw_io.h:83:7: warning: variable '__w' set but not
used [-Wunused-but-set-variable]
83 | ({u8 __w, __v = (b); u32 _addr = ((u32) (addr)); \
| ^~~
arch/m68k/include/asm/raw_io.h:430:3: note: in expansion of macro 'rom_out_8'
430 | rom_out_8(port, *buf++);
| ^
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsw':
arch/m68k/include/asm/raw_io.h:86:8: warning: variable '__w' set but not
used [-Wunused-but-set-variable]
86 | ({u16 __w, __v = (w); u32 _addr = ((u32) (addr)); \
|^~~
arch/m68k/include/asm/raw_io.h:448:3: note: in expansion of macro
'rom_out_be16'
448 | rom_out_be16(port, *buf++);
| ^~~~
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsw_swapw':
arch/m68k/include/asm/raw_io.h:90:8: warning: variable '__w' set but not
used [-Wunused-but-set-variable]
90 | ({u16 __w, __v = (w); u32 _addr = ((u32) (addr)); \
|^~~
arch/m68k/include/asm/raw_io.h:466:3: note: in expansion of macro
'rom_out_le16'
466 | rom_out_le16(port, *buf++);
| ^~~~
In file included from include/linux/kernel.h:11,
from include/linux/list.h:9,
from include/linux/module.h:12,
from drivers/nvme/host/rdma.c:7:
include/linux/scatterlist.h: In function 'sg_set_buf':
arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of
pointer with null pointer [-Wextra]
169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void
*)PAGE_OFFSET && (void *)(kaddr) < high_memory)
| ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
143 | BUG_ON(!virt_addr_valid(buf));
| ^~
include/linux/scatterlist.h:143:10: note: in expansion of macro
'virt_addr_valid'
143 | BUG_ON(!virt_addr_valid(buf));
| ^~~
In file included from arch/m68k/include/asm/bug.h:32,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/stat.h:19,
from include/linux/module.h:13,
from drivers/nvme/host/rdma.c:7:
include/linux/dma-mapping.h: In function 'dma_map_resource':
arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of
pointer with null pointer [-Wextra]
169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void
*)PAGE_OFFSET && (void *)(kaddr) < high_memory)
| ^~
include/asm-generic/bug.h:144:27: note: in definition of macro 'WARN_ON_ONCE'
144 |
Re: [PATCH] dma-mapping: DMA_COHERENT_POOL should select GENERIC_ALLOCATOR
On Thu, 18 Jun 2020, Christoph Hellwig wrote:
> The dma coherent pool code needs genalloc. Move the select over
> from DMA_REMAP, which doesn't actually need it.
>
> Fixes: dbed452a078d ("dma-pool: decouple DMA_REMAP from DMA_COHERENT_POOL")
> Reported-by: kernel test robot
Acked-by: David Rientjes
Thanks Christoph. In the initial bug report from Alex Xu, his .config set
CONFIG_GENERIC_ALLOCATOR already so I think there is very little risk with
this patch.
PC speaker
Is it possible to write a kernel module which, when loaded, will blow the PC speaker?
Re: [PATCH] usbip: tools: fix module name in man page
On 6/18/20 10:56 AM, Shuah Khan wrote:
On 6/17/20 6:08 PM, Antonio Borneo wrote:
Commit 64e62426f40d ("staging: usbip: edit Kconfig and rename
CONFIG options") renamed the module usbip as usbip-host, but the
example in the man page still reports the old module name.
Fix the module name in usbipd.8
Signed-off-by: Antonio Borneo
Fixes: 64e62426f40d ("staging: usbip: edit Kconfig and rename CONFIG
options")
---
tools/usb/usbip/doc/usbipd.8 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/usb/usbip/doc/usbipd.8 b/tools/usb/usbip/doc/usbipd.8
index ac4635db3f03..fb62a756893b 100644
--- a/tools/usb/usbip/doc/usbipd.8
+++ b/tools/usb/usbip/doc/usbipd.8
@@ -73,7 +73,7 @@ USB/IP client can connect and use exported devices.
.SH EXAMPLES
- server:# modprobe usbip
+ server:# modprobe usbip-host
server:# usbipd -D
- Start usbip daemon.
base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
Looks good. Thanks for fixing this.
Acked-by: Shuah Khan
+ Adding Greg
Odd. Looks like get_maintainers gave a very old address
for Greg Kroah-Hartman .
thanks,
-- Shuah
Re: [PATCH 2/2] integrity: Add errno field in audit message
On Wed, 2020-06-17 at 13:44 -0700, Lakshmi Ramasubramanian wrote: > Error code is not included in the audit messages logged by > the integrity subsystem. Add "errno" field in the audit messages > logged by the integrity subsystem and set the value to the error code > passed to integrity_audit_msg() in the "result" parameter. > > Sample audit messages: > > [6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 > auid=4294967295 ses=4294967295 subj=kernel op=add_boot_aggregate > cause=alloc_entry comm="swapper/0" name="boot_aggregate" res=0 errno=-12 > > [8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 > auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 > op=policy_update cause=completed comm="systemd" res=1 errno=0 > > Signed-off-by: Lakshmi Ramasubramanian > Suggested-by: Steve Grubb > --- > security/integrity/integrity_audit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/integrity/integrity_audit.c > b/security/integrity/integrity_audit.c > index 5109173839cc..a265024f82f3 100644 > --- a/security/integrity/integrity_audit.c > +++ b/security/integrity/integrity_audit.c > @@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode > *inode, > audit_log_untrustedstring(ab, inode->i_sb->s_id); > audit_log_format(ab, " ino=%lu", inode->i_ino); > } > - audit_log_format(ab, " res=%d", !result); > + audit_log_format(ab, " res=%d errno=%d", !result, result); > audit_log_end(ab); > } For the reasons that I mentioned previously, unless others are willing to add their Reviewed-by tag not for the audit aspect in particular, but IMA itself, I'm not comfortable making this change all at once. Previously I suggested making the existing integrity_audit_msg() a wrapper for a new function with errno. Steve said, "We normally do not like to have fields that swing in and out ...", but said setting errno to 0 is fine. The original integrity_audit_msg() function would call the new function with errno set to 0. Mimi
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Thu 2020-06-18 18:19:12, Petr Mladek wrote: > On Wed 2020-06-17 10:25:35, Jim Cromie wrote: > > 1. Add a user-flag [u] which works like the [pfmlt] flags, but has no > > effect on callsite behavior; it allows incremental marking of > > arbitrary sets of callsites. > > > > 2. Add [PFMLTU] flags, which negate their counterparts; P===!p etc. > > And in ddebug_read_flags(): > >current code does: [pfmltu_] -> flags > >copy it to: [PFMLTU_] -> mask > > > > also disallow both of a pair: ie no 'pP', no true & false. > > > > 3. Add filtering ops into ddebug_change(), right after all the > > callsite-property selections are complete. These filter on the > > callsite's current flagstate before applying modflags. > > > > Why ? > > > > The u-flag & filter flags > > > > The 'u' flag lets the user assemble an arbitary set of callsites. > > Then using filter flags, user can activate the 'u' callsite set. > > > > #> echo 'file foo.c +u; file bar.c +u' > control # and repeat > > #> echo 'u+p' > control > > > > Of course, you can continue to just activate your set without ever > > marking it 1st, but you could trivially add the markup as you go, then > > be able to use it as a constraint later, to undo or modify your set. > > > > #> echo 'file foo.c +up' >control > > .. monitor, debug, finish .. > > #> echo 'u-p' >control > > > > # then later resume > > #> echo 'u+p' >control > > > > # disable some cluttering messages, and remove from u-set > > #> echo 'file noisy.c function:jabber_* u-pu' >control > > > > # for doc, recollection > > grep =pu control > my-favorite-callsites > > > > Note: > > > > Your flagstate after boot is generally not all =_. -DDEBUG will arm > > compiled callsites by default, $builtinmod.dyndbg=+p bootargs can > > enable them early, and $module.dyndbg=+p bootargs will arm them when > > the module is loaded. But you could manage them with u-flags: > > > > #> echo '-t' >control # clear t-flag to use it as 2ndary > > markup > > #> echo 'p+ut' >control # mark the boot-enabled set of callsites > > #> echo '-p' >control # clean your dmesg -w stream > > > > ... monitor, debug .. > > #> echo 'module of_interest $qterms +pu' >control # build your set of > > useful debugs > > #> echo 'module of_interest $qterms UT+pu' >control # same, but > > dont alter ut marked set > > Does anyone requested this feature, please? > > For me, it is really hard to imagine people using these complex and hacky > steps. I think that all this is motivated by adding support for module specific groups. What about storing the group as yet another information for each message? I mean the same way as we store module name, file, line, function name. Then we could add API to define group for a given message: pr_debug_group(group_id, fmt, ...); the interface for the control file might be via new keyword "group". You could then do something like: echo module=drm group=0x3 +p >control But more importantly you should add functions that might be called when the drm.debug parameter is changes. I have already mentioned it is another reply: dd_enable_module_group(module_name, group_id); dd_disable_module_group(module_name, group_id); It will _not_ need any new flag or flag filtering. Best Regards, Petr
Re: [PATCH] usbip: tools: fix build error for multiple definition
On 6/17/20 6:08 PM, Antonio Borneo wrote:
With GCC 10, building usbip triggers error for multiple definition
of 'udev_context', in:
- libsrc/vhci_driver.c:18 and
- libsrc/usbip_host_common.c:27.
Declare as extern the definition in libsrc/usbip_host_common.c.
Signed-off-by: Antonio Borneo
---
tools/usb/usbip/libsrc/usbip_host_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c
b/tools/usb/usbip/libsrc/usbip_host_common.c
index d1d8ba2a4a40..ca78aa368476 100644
--- a/tools/usb/usbip/libsrc/usbip_host_common.c
+++ b/tools/usb/usbip/libsrc/usbip_host_common.c
@@ -23,7 +23,7 @@
#include "list.h"
#include "sysfs_utils.h"
-struct udev *udev_context;
+extern struct udev *udev_context;
static int32_t read_attr_usbip_status(struct usbip_usb_device *udev)
{
Looks good to me.
Acked-by: Shuah Khan
thanks,
-- Shuah
KMSAN: uninit-value in hash_ip6_add
Hello, syzbot found the following crash on: HEAD commit:f0d5ec90 kmsan: apply __no_sanitize_memory to dotraplinkag.. git tree: https://github.com/google/kmsan.git master console output: https://syzkaller.appspot.com/x/log.txt?x=126592fa10 kernel config: https://syzkaller.appspot.com/x/.config?x=86e4f8af239686c6 dashboard link: https://syzkaller.appspot.com/bug?extid=89bacaf2be1277d1e6de compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+89bacaf2be1277d1e...@syzkaller.appspotmail.com = BUG: KMSAN: uninit-value in __read_once_size include/linux/compiler.h:206 [inline] BUG: KMSAN: uninit-value in hash_ip6_add+0x14eb/0x30c0 net/netfilter/ipset/ip_set_hash_gen.h:892 CPU: 1 PID: 31730 Comm: syz-executor.3 Not tainted 5.7.0-rc4-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1c9/0x220 lib/dump_stack.c:118 kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:121 __msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215 __read_once_size include/linux/compiler.h:206 [inline] hash_ip6_add+0x14eb/0x30c0 net/netfilter/ipset/ip_set_hash_gen.h:892 hash_ip6_uadt+0x8e6/0xad0 net/netfilter/ipset/ip_set_hash_ip.c:267 call_ad+0x2dc/0xbc0 net/netfilter/ipset/ip_set_core.c:1732 ip_set_ad+0xad2/0x1110 net/netfilter/ipset/ip_set_core.c:1820 ip_set_uadd+0xf6/0x110 net/netfilter/ipset/ip_set_core.c:1845 nfnetlink_rcv_msg+0xb86/0xcf0 net/netfilter/nfnetlink.c:229 netlink_rcv_skb+0x451/0x650 net/netlink/af_netlink.c:2469 nfnetlink_rcv+0x3b5/0x3ab0 net/netfilter/nfnetlink.c:563 netlink_unicast_kernel net/netlink/af_netlink.c:1303 [inline] netlink_unicast+0xf9e/0x1100 net/netlink/af_netlink.c:1329 netlink_sendmsg+0x1246/0x14d0 net/netlink/af_netlink.c:1918 sock_sendmsg_nosec net/socket.c:652 [inline] sock_sendmsg net/socket.c:672 [inline] sys_sendmsg+0x12b6/0x1350 net/socket.c:2362 ___sys_sendmsg net/socket.c:2416 [inline] __sys_sendmsg+0x623/0x750 net/socket.c:2449 __do_sys_sendmsg net/socket.c:2458 [inline] __se_sys_sendmsg+0x97/0xb0 net/socket.c:2456 __x64_sys_sendmsg+0x4a/0x70 net/socket.c:2456 do_syscall_64+0xb8/0x160 arch/x86/entry/common.c:297 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x45ca59 Code: 0d b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 db b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7fea85396c78 EFLAGS: 0246 ORIG_RAX: 002e RAX: ffda RBX: 004fe260 RCX: 0045ca59 RDX: RSI: 22c0 RDI: 0003 RBP: 0078bf00 R08: R09: R10: R11: 0246 R12: R13: 0942 R14: 004cc0a6 R15: 7fea853976d4 Uninit was stored to memory at: kmsan_save_stack_with_flags mm/kmsan/kmsan.c:144 [inline] kmsan_internal_chain_origin+0xad/0x130 mm/kmsan/kmsan.c:310 __msan_chain_origin+0x50/0x90 mm/kmsan/kmsan_instr.c:165 ip6_netmask include/linux/netfilter/ipset/pfxlen.h:49 [inline] hash_ip6_netmask net/netfilter/ipset/ip_set_hash_ip.c:185 [inline] hash_ip6_uadt+0x9df/0xad0 net/netfilter/ipset/ip_set_hash_ip.c:263 call_ad+0x2dc/0xbc0 net/netfilter/ipset/ip_set_core.c:1732 ip_set_ad+0xad2/0x1110 net/netfilter/ipset/ip_set_core.c:1820 ip_set_uadd+0xf6/0x110 net/netfilter/ipset/ip_set_core.c:1845 nfnetlink_rcv_msg+0xb86/0xcf0 net/netfilter/nfnetlink.c:229 netlink_rcv_skb+0x451/0x650 net/netlink/af_netlink.c:2469 nfnetlink_rcv+0x3b5/0x3ab0 net/netfilter/nfnetlink.c:563 netlink_unicast_kernel net/netlink/af_netlink.c:1303 [inline] netlink_unicast+0xf9e/0x1100 net/netlink/af_netlink.c:1329 netlink_sendmsg+0x1246/0x14d0 net/netlink/af_netlink.c:1918 sock_sendmsg_nosec net/socket.c:652 [inline] sock_sendmsg net/socket.c:672 [inline] sys_sendmsg+0x12b6/0x1350 net/socket.c:2362 ___sys_sendmsg net/socket.c:2416 [inline] __sys_sendmsg+0x623/0x750 net/socket.c:2449 __do_sys_sendmsg net/socket.c:2458 [inline] __se_sys_sendmsg+0x97/0xb0 net/socket.c:2456 __x64_sys_sendmsg+0x4a/0x70 net/socket.c:2456 do_syscall_64+0xb8/0x160 arch/x86/entry/common.c:297 entry_SYSCALL_64_after_hwframe+0x44/0xa9 Uninit was stored to memory at: kmsan_save_stack_with_flags mm/kmsan/kmsan.c:144 [inline] kmsan_internal_chain_origin+0xad/0x130 mm/kmsan/kmsan.c:310 kmsan_memcpy_memmove_metadata+0x272/0x2e0 mm/kmsan/kmsan.c:247 kmsan_memcpy_metadata+0xb/0x10 mm/kmsan/kmsan.c:267 __msan_memcpy+0x43/0x50 mm/kmsan/kmsan_instr.c:116 ip_set_get_ipaddr6+0x26a/0x300 net/netfilter/ipset/ip_set_core.c:325 hash_ip6_uadt+0x450/0xad0
Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs
On Thu, Jun 18, 2020 at 07:30:49PM +0200, Uladzislau Rezki wrote:
> > I'd suggest:
> >
> > rcu_lock_acquire(_callback_map);
> > trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
> > bkvhead[i]->nr_records, bkvhead[i]->records);
> > if (i == 0) {
> > kfree_bulk(bkvhead[i]->nr_records,
> > bkvhead[i]->records);
> > } else {
> > for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > vfree(bkvhead[i]->records[j]);
> > }
> > }
> > rcu_lock_release(_callback_map);
> >
> There are two different trace functions, one for "bulk" tracing
> messages, and another one is per one call of kfree(), though we use
> to indicate vfree() call.
>
> Probably we can rename it to: trace_rcu_invoke_kvfree_callback();
>
> What do you think?
Works for me!
> > But I'd also suggest a vfree_bulk be added. There are a few things
> > which would be better done in bulk as part of the vfree process
> > (we batch them up already, but i'm sure we could do better).
>
> I was thinking to implement of vfree_bulk() API, but i guess it can
> be done as future work.
>
> Does that sound good?
Yes, definitely a future piece of work.
Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs
> >
> > I don't think that replacing direct function calls with indirect function
> > calls is a great suggestion with the current state of play around branch
> > prediction.
> >
> > I'd suggest:
> >
> > rcu_lock_acquire(_callback_map);
> > trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
> > bkvhead[i]->nr_records, bkvhead[i]->records);
> > if (i == 0) {
> > kfree_bulk(bkvhead[i]->nr_records,
> > bkvhead[i]->records);
> > } else {
> > for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > vfree(bkvhead[i]->records[j]);
> > }
> > }
> > rcu_lock_release(_callback_map);
> >
> > But I'd also suggest a vfree_bulk be added. There are a few things
> > which would be better done in bulk as part of the vfree process
> > (we batch them up already, but i'm sure we could do better).
>
> I suspect that he would like to keep the tracing.
>
> It might be worth trying the branches, given that they would be constant
> and indexed by "i". The compiler might well remove the indirection.
>
> The compiler guys brag about doing so, which of course might or might
> not have any correlation to a given compiler actually doing so. :-/
>
> Having a vfree_bulk() might well be useful, but I would feel more
> confidence in that if there were other callers of kfree_bulk().
>
Hmm... I think replacing that with vfree_bulk() is a good idea though.
>
> But again, either way, future work as far as this series is concerned.
>
What do you mean: is concerned?
We are planning to implement kfree_rcu() to be integrated directly into
SLAB: SLAB, SLUB, SLOB. So, there are plenty of future work :)
--
Vlad Rezki
Re: [PATCH v2 1/3] mfd: core: Make a best effort attempt to match devices with the correct of_nodes
On 2020-06-15 04:26, Lee Jones wrote:
> On Sun, 14 Jun 2020, Frank Rowand wrote:
>
>> Hi Lee,
>>
>> I'm looking at 5.8-rc1.
>>
>> The only use of OF_MFD_CELL() where the same compatible is specified
>> for multiple elements of a struct mfd_cell array is for compatible
>> "stericsson,ab8500-pwm" in drivers/mfd/ab8500-core.c:
>>
>> OF_MFD_CELL("ab8500-pwm",
>> NULL, NULL, 0, 1, "stericsson,ab8500-pwm"),
>> OF_MFD_CELL("ab8500-pwm",
>> NULL, NULL, 0, 2, "stericsson,ab8500-pwm"),
>> OF_MFD_CELL("ab8500-pwm",
>> NULL, NULL, 0, 3, "stericsson,ab8500-pwm"),
OF_MFD_CELL("ab8500-pwm",
NULL, NULL, 0, 0, "stericsson,ab8500-pwm"),
OF_MFD_CELL_REG("ab8500-pwm-mc",
NULL, NULL, 0, 0, "stericsson,ab8500-pwm", 0),
OF_MFD_CELL_REG("ab8500-pwm-mc",
NULL, NULL, 0, 1, "stericsson,ab8500-pwm", 1),
OF_MFD_CELL_REG("ab8500-pwm-mc",
NULL, NULL, 0, 2, "stericsson,ab8500-pwm", 2),
>>
>> The only .dts or .dtsi files where I see compatible "stericsson,ab8500-pwm"
>> are:
>>
>>arch/arm/boot/dts/ste-ab8500.dtsi
>>arch/arm/boot/dts/ste-ab8505.dtsi
>>
>> These two .dtsi files only have a single node with this compatible.
>> Chasing back to .dts and .dtsi files that include these two .dtsi
>> files, I see no case where there are multiple nodes with this
>> compatible.
>>
>> So it looks to me like there is no .dts in mainline that is providing
>> the three "stericsson,ab8500-pwm" nodes that drivers/mfd/ab8500-core.c
>> is expecting. No case that there are multiple mfd child nodes where
>> mfd_add_device() would assign the first of n child nodes with the
>> same compatible to multiple devices.
>>
>> So it appears to me that drivers/mfd/ab8500-core.c is currently broken.
>> Am I missing something here?
>>
>> If I am correct, then either drivers/mfd/ab8500-core.c or
>> ste-ab8500.dtsi and ste-ab8505.dtsi need to be fixed.
>
> Your analysis is correct.
OK, if I'm not overlooking anything, that is good news.
Existing .dts source files only have one "ab8500-pwm" child. They already
work correcly.
Create a new compatible for the case of multiple children. In my example
I will add "-mc" (multiple children) to the existing compatible. There
is likely a better name, but this lets me provide an example.
Modify drivers/mfd/ab8500-core.c to use the new compatible, and new .dts
source files with multiple children use the new compatible:
OF_MFD_CELL("ab8500-pwm",
NULL, NULL, 0, 0, "stericsson,ab8500-pwm"),
OF_MFD_CELL_REG("ab8500-pwm-mc",
NULL, NULL, 0, 0, "stericsson,ab8500-pwm", 0),
OF_MFD_CELL_REG("ab8500-pwm-mc",
NULL, NULL, 0, 1, "stericsson,ab8500-pwm", 1),
OF_MFD_CELL_REG("ab8500-pwm-mc",
NULL, NULL, 0, 2, "stericsson,ab8500-pwm", 2),
The "OF_MFD_CELL" entry is the existing entry, which will handle current
.dts source files. The new "OF_MFD_CELL_REG" entries will handle new
.dts source files.
And of course the patch that creates OF_MFD_CELL_REG() needs to precede
this change.
I would remove the fallback code in the existing patch that tries to
handle an incorrect binding. Just error out if the binding is not
used properly.
-Frank
>
> Although it's not "broken", it just works when it really shouldn't.
>
> I will be fixing the 'ab8500-pwm' case in due course.
>
>> Moving forward, your proposed OF_MFD_CELL_REG() method seems a good
>> approach (I have not completely read the actual code in the patch yet
>> though).
>
> Thanks.
>
Re: [PATCH] driver core:Export the symbol device_is_bound
On Thu, Jun 18, 2020 at 10:51 AM Matthias Kaehlcke wrote: > > On Thu, Jun 18, 2020 at 05:58:20PM +0200, Greg Kroah-Hartman wrote: > > On Thu, Jun 18, 2020 at 08:45:55AM -0700, Matthias Kaehlcke wrote: > > > Hi Greg, > > > > > > On Thu, Jun 18, 2020 at 10:14:43AM +0200, Greg Kroah-Hartman wrote: > > > > On Wed, Jun 03, 2020 at 12:09:52AM +0530, Sandeep Maheswaram wrote: > > > > > Export the symbol device_is_bound so that it can be used by the > > > > > modules. > > > > > > > > What modules need this? > > > > > > drivers/usb/dwc3/dwc3-qcom.c (and probably other dwc3 'wrappers'). > > > > Why wasn't that said here? No context is not good :( > > Agreed, this patch should probably have been part of a series to establish > the context. > > > > Short summary: QCOM dwc3 support is split in two drivers, the core dwc3 > > > driver and the QCOM specific parts. dwc3-qcom is probed first (through > > > a DT entry or ACPI), dwc3_qcom_probe() then calls of_platform_populate() > > > to probe the core part. After a successful return from _populate() the > > > driver assumes that the core device is fully initialized. However the > > > latter is not correct, the driver core doesn't propagate errors from > > > probe() to platform_populate(). The dwc3-qcom driver would use > > > device_is_bound() to make sure the core device was probed successfully. > > > > why does the dwc3-qcom driver care? > > Currently the dwc3-qcom driver uses the core device to determine if the > controller is used in peripheral mode and it runtime resumes the XHCI > device when it sees a wakeup interrupt. > > The WIP patch to add interconnect support relies on the core driver > to determine the max speed of the controller. > > > And why is the driver split in a way that requires such "broken" > > structures? Why can't that be fixed instead? > > It seems determining the mode could be easily changed by getting it through > the pdev, as in st_dwc3_probe(). Not sure about the other two parts, > determining the maximum speed can involve evaluating hardware registers, > which currently are 'owned' by the core driver. > > Manu or Sandeep who know the hardware and the driver better than me might > have ideas on how to improve things. We never should have had this split either in the DT binding nor driver(s) as if the SoC wrapper crap and licensed IP block are independent things. The thing to do here is either make the DWC3 code a library which drivers call (e.g. SDHCI) or add hooks into the DWC3 driver for platform specifics (e.g. Designware PCI). Neither is a simple solution though. Rob
Re: [PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
On Thu, Jun 18, 2020 at 07:11:24PM +0200, Daniel Wagner wrote: > On Thu, Jun 18, 2020 at 04:32:40PM +0200, Niklas Cassel wrote: > > If, for some reason, we want to allow builds with gcc < 4.6.0 > > even though the minimum gcc version is now 4.8.0, > > Just one thing to watch out: the stable trees are still using > older version of gcc. Note sure how relevant this is though. While the AUTOSEL can be a bit annoying when autoselecting patches to backport that you didn't intend, I think that it in most cases backports fixes that has a Fixes-tag, which this doesn't. Kind regards, Niklas
Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs
On Thu, Jun 18, 2020 at 07:25:04PM +0200, Uladzislau Rezki wrote:
> > > + // Handle two first channels.
> > > + for (i = 0; i < FREE_N_CHANNELS; i++) {
> > > + for (; bkvhead[i]; bkvhead[i] = bnext) {
> > > + bnext = bkvhead[i]->next;
> > > + debug_rcu_bhead_unqueue(bkvhead[i]);
> > > +
> > > + rcu_lock_acquire(_callback_map);
> > > + if (i == 0) { // kmalloc() / kfree().
> > > + trace_rcu_invoke_kfree_bulk_callback(
> > > + rcu_state.name, bkvhead[i]->nr_records,
> > > + bkvhead[i]->records);
> > > +
> > > + kfree_bulk(bkvhead[i]->nr_records,
> > > + bkvhead[i]->records);
> > > + } else { // vmalloc() / vfree().
> > > + for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > > + trace_rcu_invoke_kfree_callback(
> > > + rcu_state.name,
> > > + bkvhead[i]->records[j], 0);
> > > +
> > > + vfree(bkvhead[i]->records[j]);
> > > + }
> > > + }
> > > + rcu_lock_release(_callback_map);
> >
> > Not an emergency, but did you look into replacing this "if" statement
> > with an array of pointers to functions implementing the legs of the
> > "if" statement? If nothing else, this would greatly reduced indentation.
> >
> >
> > I am taking this as is, but if you have not already done so, could you
> > please look into this for a follow-up patch?
> >
> I do not think it makes sense, because it would require to check each
> pointer in the array, what can lead to many branching, i.e. "if-else"
> instructions.
Mightn't the compiler simply unroll the outer loop? Then the first
unrolled iteration of that loop would contain the then-clause and
the second unrolled iteration would contain the else-clause. At that
point, there would be no checking, just direct calls.
Or am I missing something?
> Paul, thank you to take it in!
Thank you for persisting!
Thanx, Paul
Re: [pipe] 566d136289: stress-ng.tee.ops_per_sec -84.7% regression
On Wed, Jun 17, 2020 at 10:18 PM Tetsuo Handa wrote: > > This would be because the test case shows higher performance if the pipe > writer does busy wait. > This commit fixed an unkillable busy wait bug when the pipe reader does not > try to read. > > > If you fix the issue, kindly add following tag > > Reported-by: kernel test robot > > We can't fix the issue. ;-) Well, it does highlight that there are potential loads that would prefer spinning to wait for data rather than returning early. Put another way: right now we are very eager to return -EAGAIN for nonblocking pipe writers, and sleeping for blocking ones. I didn't check which of those cases that stress-ng.tee.ops_per_sec thing is testing. But the improvement in the numbers implies that it might be worth it to have optimistic logic for "spin for a bit waiting for a concurrent reader". Kind of like the old logic we used to have to try to avoid extra system calls on the reader side (where we'd give an existing writer the chance to fill the buffer instead of returning early). The old reader-side optimization was somewhat painful, and didn't really help much on SMP anyway. But particularly for the "we just dropped the locks, and we're going to wait" case, maybe it's worth looking at whether dropping the locks now woke somebody else up on another CPU, and we might spin for a short while synchronously... IOW, conceptually all the same optimistic spinning stuff that we do for semaphores.. It would likely be a somewhat involved thing, though. We'd have to make wakeup_pipe_readers/writers() return a "did I wake up somebody else on another CPU" return value for hinting whether it might be worth it, and we'd have to then add the logic to see if it's worth spinning for a while waiting for them to fill the input queue (or empty the output one) and then continue the splice() op. That 84% change sounds like it *might* be worth doing some extra work for. splice() itself might not be so interesting, but the exact same logic is presumably worth something for a pipe read/write pair... Anybody interested in trying? Linus
Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs
> >
> > Not an emergency, but did you look into replacing this "if" statement
> > with an array of pointers to functions implementing the legs of the
> > "if" statement? If nothing else, this would greatly reduced indentation.
>
> I don't think that replacing direct function calls with indirect function
> calls is a great suggestion with the current state of play around branch
> prediction.
>
> I'd suggest:
>
> rcu_lock_acquire(_callback_map);
> trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
> bkvhead[i]->nr_records, bkvhead[i]->records);
> if (i == 0) {
> kfree_bulk(bkvhead[i]->nr_records,
> bkvhead[i]->records);
> } else {
> for (j = 0; j < bkvhead[i]->nr_records; j++) {
> vfree(bkvhead[i]->records[j]);
> }
> }
> rcu_lock_release(_callback_map);
>
There are two different trace functions, one for "bulk" tracing
messages, and another one is per one call of kfree(), though we use
to indicate vfree() call.
Probably we can rename it to: trace_rcu_invoke_kvfree_callback();
What do you think?
>
> But I'd also suggest a vfree_bulk be added. There are a few things
> which would be better done in bulk as part of the vfree process
> (we batch them up already, but i'm sure we could do better).
>
I was thinking to implement of vfree_bulk() API, but i guess it can
be done as future work.
Does that sound good?
--
Vlad Rezki
Re: [Linaro-mm-sig] [PATCH 04/18] dma-fence: prime lockdep annotations
On Thu, Jun 18, 2020 at 05:00:51PM +0200, Daniel Vetter wrote: > On Wed, Jun 17, 2020 at 12:28:35PM -0300, Jason Gunthorpe wrote: > > On Wed, Jun 17, 2020 at 08:48:50AM +0200, Daniel Vetter wrote: > > > > > Now my understanding for rdma is that if you don't have hw page fault > > > support, > > > > The RDMA ODP feature is restartable HW page faulting just like nouveau > > has. The classical MR feature doesn't have this. Only mlx5 HW supports > > ODP today. > > > > > It's only gpus (I think) which are in this awkward in-between spot > > > where dynamic memory management really is much wanted, but the hw > > > kinda sucks. Aside, about 10+ years ago we had a similar problem with > > > gpu hw, but for security: Many gpu didn't have any kinds of page > > > tables to isolate different clients from each another. drivers/gpu > > > fixed this by parsing what userspace submitted to make sure > > > it's only every accessing its own buffers. Most gpus have become > > > reasonable nowadays and do have proper per-process pagetables (gpu > > > process, not the pasid stuff), but even today there's still some of > > > the old model left in some of the smallest SoC. > > > > But I still don't understand why a dma fence is needed inside the GPU > > driver itself in the notifier. > > > > Surely the GPU driver can block and release the notifier directly from > > its own command processing channel? > > > > Why does this fence and all it entails need to leak out across > > drivers? > > So 10 years ago we had this world of every gpu driver is its own bucket, > nothing leaks out to the world. But the world had a different idea how > gpus where supposed to work, with stuff like: Sure, I understand DMA fence, but why does a *notifier* need it? The job of the notifier is to guarentee that the device it is connected to is not doing DMA before it returns. That just means you need to prove that device is done with the buffer. As I've understood GPU that means you need to show that the commands associated with the buffer have completed. This is all local stuff within the driver, right? Why use fence (other than it already exists) Jason
Re: [PATCH v4 06/17] media: mtk-jpeg: Get rid of mtk_smi_larb_get/put
Hi Yong,
On Thu, 2020-06-18 at 17:32 +0800, Yong Wu wrote:
> + Rick
>
> On Sat, 2020-05-30 at 16:10 +0800, Yong Wu wrote:
> >
> > MediaTek IOMMU has already added device_link between the consumer
> > and smi-larb device. If the jpg device call the
> > pm_runtime_get_sync,
> > the smi-larb's pm_runtime_get_sync also be called automatically.
> >
Acked-by: Rick Chang
> > CC: Rick Chang
> > Signed-off-by: Yong Wu
> > Reviewed-by: Evan Green
> > ---
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 22 ---
> > ---
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h | 2 --
> > 2 files changed, 24 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index f82a81a..21fba6f 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -21,7 +21,6 @@
> > #include
> > #include
> > #include
> > -#include
> >
> > #include "mtk_jpeg_hw.h"
> > #include "mtk_jpeg_core.h"
> > @@ -893,11 +892,6 @@ static int mtk_jpeg_queue_init(void *priv,
> > struct vb2_queue *src_vq,
> >
> > static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg)
> > {
> > - int ret;
> > -
> > - ret = mtk_smi_larb_get(jpeg->larb);
> > - if (ret)
> > - dev_err(jpeg->dev, "mtk_smi_larb_get larbvdec fail
> > %d\n", ret);
> > clk_prepare_enable(jpeg->clk_jdec_smi);
> > clk_prepare_enable(jpeg->clk_jdec);
> > }
> > @@ -906,7 +900,6 @@ static void mtk_jpeg_clk_off(struct
> > mtk_jpeg_dev *jpeg)
> > {
> > clk_disable_unprepare(jpeg->clk_jdec);
> > clk_disable_unprepare(jpeg->clk_jdec_smi);
> > - mtk_smi_larb_put(jpeg->larb);
> > }
> >
> > static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > @@ -1051,21 +1044,6 @@ static int mtk_jpeg_release(struct file
> > *file)
> >
> > static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg)
> > {
> > - struct device_node *node;
> > - struct platform_device *pdev;
> > -
> > - node = of_parse_phandle(jpeg->dev->of_node,
> > "mediatek,larb", 0);
> > - if (!node)
> > - return -EINVAL;
> > - pdev = of_find_device_by_node(node);
> > - if (WARN_ON(!pdev)) {
> > - of_node_put(node);
> > - return -EINVAL;
> > - }
> > - of_node_put(node);
> > -
> > - jpeg->larb = >dev;
> > -
> > jpeg->clk_jdec = devm_clk_get(jpeg->dev, "jpgdec");
> > if (IS_ERR(jpeg->clk_jdec))
> > return PTR_ERR(jpeg->clk_jdec);
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 999bd14..8579494 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -47,7 +47,6 @@ enum mtk_jpeg_ctx_state {
> > * @dec_reg_base: JPEG registers mapping
> > * @clk_jdec: JPEG hw working clock
> > * @clk_jdec_smi: JPEG SMI bus clock
> > - * @larb: SMI device
> > */
> > struct mtk_jpeg_dev {
> > struct mutexlock;
> > @@ -61,7 +60,6 @@ struct mtk_jpeg_dev {
> > void __iomem*dec_reg_base;
> > struct clk *clk_jdec;
> > struct clk *clk_jdec_smi;
> > - struct device *larb;
> > };
> >
> > /**
>
Re: [PATCH 2/2] nvmet: remove workarounds for gcc bug wrt unnamed fields in initializers
I'm not against the code cleanup and it always welcome.
Please also have a look at other comment.
>> What is the issue with existing code that we need this patch for ?
>>
>
> Hello Chaitanya,
>
> This is just a cleanup patch, no functional change intended.
>
I can see that.
> It simply performs the initialization at declaration time, which is how we
> usually initialize a subset of all fields.
Absolutely not true in case nvme subsystem.
>
> This is also how it was originally done, but this was changed to a
> non-standard way in order to workaround a compiler bug.
>
and the existing code matches the pattern present in the repo no need to
change if it is not causing any problem.
> Since the compiler bug is no longer relevant, we can go back to the
> standard way of doing things.
Is there any problem with the code now which mandates this change ?
which I don't see any.
>
> Performing initialization in a uniform way makes it easier to read and
> comprehend the code, especially for people unfamiliar with it, since
> it follows the same pattern used in other places of the kernel.
>
This is absolutely not uniform way infact what you are proposing is true
then we need to change all existing function which does not follow this
pattern, have a look at the following list.
In NVMe subsystem case there are more than 10 functions which are on the
similar line of this function and doesn't initialize the variable at the
time declaration.
Please have a look the code :-
nvmf_reg_read32
nvmf_reg_read64
nvmf_reg_write32
nvmf_connect_admin_queue
nvmf_connect_io_queue
nvme_identify_ctrl
nvme_identify_ns_descs
nvme_identify_ns_list
nvme_identify_ns
nvme_features
nvme_get_log
nvme_toggle_streams
nvme_get_stream_params
Also here :-
nvme_user_cmd
nvme_user_cmd64
Last two are an exception of copy_from_user() call before initialization
case, but we can do copy from user from caller and pass that as an
argument if we really want to follow the declare-init pattern.
In several places we don't follow this pattern when function is compact
and it looks ugly for larger structures such as this example. this is
exactly the reason {} used in nvme subsystem.
> Just reading e.g. struct rdma_conn_param param = { }; one assumes that
> all fields will be zero initialized.. reading futher down in the function
No this is standard style and used in nvme/host/core.c everywhere
nothing wrong with this at all, please have a look at the author.
> you realize that this function actually does initialize the struct..
> which causes a mental hiccup, so you need to do a mental pipeline flush
> and go back and read the code from the beginning. This only happens with
> patterns that deviate from the standard way of doing things.
The function is small enough for such hiccup which follows the pattern
which we have it in the tree there is nothing wrong.
>
> Kind regards,
> Niklas
>
[PATCH v9 9/9] firmware: arm_scmi: Add Base notifications support
Make SCMI Base protocol register with the notification core.
Reviewed-by: Jonathan Cameron
Signed-off-by: Cristian Marussi
---
V8 --> V9
- moved pr_info to pr_debug
- removed switch()
- use SCMI_PROTO_QUEUE_SZ
V6 --> V7
- fixed report.timestamp type
- fix max_payld_sz initialization
- fix report layout and initialization
- expose SCMI_EVENT_ in linux/scmi_protocol.h
V5 --> V6
- added handle argument to fill_custom_report()
V4 --> V5
- fixed unsual return construct
V3 --> V4
- scmi_event field renamed
V2 --> V3
- added handle awareness
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/base.c | 109 +--
include/linux/scmi_protocol.h| 9 +++
2 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index ce7d9203e41b..d5a7878d3fbd 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -5,7 +5,15 @@
* Copyright (C) 2018 ARM Ltd.
*/
+#define pr_fmt(fmt) "SCMI Notifications BASE - " fmt
+
+#include
+
#include "common.h"
+#include "notify.h"
+
+#define SCMI_BASE_NUM_SOURCES 1
+#define SCMI_BASE_MAX_CMD_ERR_COUNT1024
enum scmi_base_protocol_cmd {
BASE_DISCOVER_VENDOR = 0x3,
@@ -19,16 +27,25 @@ enum scmi_base_protocol_cmd {
BASE_RESET_AGENT_CONFIGURATION = 0xb,
};
-enum scmi_base_protocol_notify {
- BASE_ERROR_EVENT = 0x0,
-};
-
struct scmi_msg_resp_base_attributes {
u8 num_protocols;
u8 num_agents;
__le16 reserved;
};
+struct scmi_msg_base_error_notify {
+ __le32 event_control;
+#define BASE_TP_NOTIFY_ALL BIT(0)
+};
+
+struct scmi_base_error_notify_payld {
+ __le32 agent_id;
+ __le32 error_status;
+#define IS_FATAL_ERROR(x) ((x) & BIT(31))
+#define ERROR_CMD_COUNT(x) FIELD_GET(GENMASK(9, 0), (x))
+ __le64 msg_reports[SCMI_BASE_MAX_CMD_ERR_COUNT];
+};
+
/**
* scmi_base_attributes_get() - gets the implementation details
* that are associated with the base protocol.
@@ -222,6 +239,84 @@ static int scmi_base_discover_agent_get(const struct
scmi_handle *handle,
return ret;
}
+static int scmi_base_error_notify(const struct scmi_handle *handle, bool
enable)
+{
+ int ret;
+ u32 evt_cntl = enable ? BASE_TP_NOTIFY_ALL : 0;
+ struct scmi_xfer *t;
+ struct scmi_msg_base_error_notify *cfg;
+
+ ret = scmi_xfer_get_init(handle, BASE_NOTIFY_ERRORS,
+SCMI_PROTOCOL_BASE, sizeof(*cfg), 0, );
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->event_control = cpu_to_le32(evt_cntl);
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static bool scmi_base_set_notify_enabled(const struct scmi_handle *handle,
+u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ ret = scmi_base_error_notify(handle, enable);
+ if (ret)
+ pr_debug("FAIL_ENABLED - evt[%X] ret:%d\n", evt_id, ret);
+
+ return !ret;
+}
+
+static void *scmi_base_fill_custom_report(const struct scmi_handle *handle,
+ u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ int i;
+ const struct scmi_base_error_notify_payld *p = payld;
+ struct scmi_base_error_report *r = report;
+
+
+ /*
+* BaseError notification payload is variable in size but
+* up to a maximum length determined by the struct ponted by p.
+* Instead payld_sz is the effective length of this notification
+* payload so cannot be greater of the maximum allowed size as
+* pointed by p.
+*/
+ if (evt_id != SCMI_EVENT_BASE_ERROR_EVENT || sizeof(*p) < payld_sz)
+ return NULL;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->fatal = IS_FATAL_ERROR(le32_to_cpu(p->error_status));
+ r->cmd_count = ERROR_CMD_COUNT(le32_to_cpu(p->error_status));
+ for (i = 0; i < r->cmd_count; i++)
+ r->reports[i] = le64_to_cpu(p->msg_reports[i]);
+ *src_id = 0;
+
+ return r;
+}
+
+static const struct scmi_event base_events[] = {
+ {
+ .id = SCMI_EVENT_BASE_ERROR_EVENT,
+ .max_payld_sz = sizeof(struct scmi_base_error_notify_payld),
+ .max_report_sz = sizeof(struct scmi_base_error_report) +
+ SCMI_BASE_MAX_CMD_ERR_COUNT * sizeof(u64),
+ },
+};
+
+static const struct scmi_event_ops
[PATCH v9 1/9] firmware: arm_scmi: Add notification protocol-registration
Add core SCMI Notifications protocol-registration support: allow protocols to register their own set of supported events, during their initialization phase. Notification core can track multiple platform instances by their handles. Reviewed-by: Jonathan Cameron Signed-off-by: Cristian Marussi --- V8 --> V9 - shortened massive protocol_ struct naming - fixed tabs - reviewed macros to use bitfield.h - fixed WARN_ON() usage - switched to dev_dbg/info with proper dev_fmt - added common define for protocol queue_sz V7 --> V8 - Fixed init/enable procedure, un-needed atomics removed V4 --> V5 - fixed kernel-doc - added barriers for registered protocols and events - using kfifo_alloc and devm_add_action_or_reset V3 --> V4 - removed scratch ISR buffer, move scratch BH buffer into protocol descriptor - converted registered_protocols and registered_events from hashtables into bare fixed-sized arrays - removed unregister protocols' routines (never called really) V2 --> V3 - added scmi_notify_instance to track target platform instance V1 --> V2 - splitted out of V1 patch 04 - moved from IDR maps to real HashTables to store events - scmi_notifications_initialized is now an atomic_t - reviewed protocol registration/unregistration to use devres - fixed: drivers/firmware/arm_scmi/notify.c:483:18-23: ERROR: reference preceded by free on line 482 Reported-by: kbuild test robot Reported-by: Julia Lawall --- drivers/firmware/arm_scmi/Makefile | 2 +- drivers/firmware/arm_scmi/common.h | 4 + drivers/firmware/arm_scmi/notify.c | 439 + drivers/firmware/arm_scmi/notify.h | 57 include/linux/scmi_protocol.h | 3 + 5 files changed, 504 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/arm_scmi/notify.c create mode 100644 drivers/firmware/arm_scmi/notify.h diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile index 1cad32b38b29..11f1e07f603e 100644 --- a/drivers/firmware/arm_scmi/Makefile +++ b/drivers/firmware/arm_scmi/Makefile @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only obj-y = scmi-bus.o scmi-driver.o scmi-protocols.o scmi-transport.o scmi-bus-y = bus.o -scmi-driver-y = driver.o +scmi-driver-y = driver.o notify.o scmi-transport-y = shmem.o scmi-transport-$(CONFIG_MAILBOX) += mailbox.o scmi-transport-$(CONFIG_ARM_PSCI_FW) += smc.o diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 31fe5a22a011..c113e578cc6c 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -6,6 +6,8 @@ * * Copyright (C) 2018 ARM Ltd. */ +#ifndef _SCMI_COMMON_H +#define _SCMI_COMMON_H #include #include @@ -235,3 +237,5 @@ void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem, void shmem_clear_channel(struct scmi_shared_mem __iomem *shmem); bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem, struct scmi_xfer *xfer); + +#endif /* _SCMI_COMMON_H */ diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c new file mode 100644 index ..b0ba4da22343 --- /dev/null +++ b/drivers/firmware/arm_scmi/notify.c @@ -0,0 +1,439 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * System Control and Management Interface (SCMI) Notification support + * + * Copyright (C) 2020 ARM Ltd. + */ +/** + * DOC: Theory of operation + * + * SCMI Protocol specification allows the platform to signal events to + * interested agents via notification messages: this is an implementation + * of the dispatch and delivery of such notifications to the interested users + * inside the Linux kernel. + * + * An SCMI Notification core instance is initialized for each active platform + * instance identified by the means of the usual scmi_handle. + * + * Each SCMI Protocol implementation, during its initialization, registers with + * this core its set of supported events using scmi_register_protocol_events(): + * all the needed descriptors are stored in the registered_protocols and + * registered_events arrays. + */ + +#define dev_fmt(fmt) "SCMI Notifications - " fmt + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "notify.h" + +#define SCMI_MAX_PROTO 256 + +#define PROTO_ID_MASK GENMASK(31, 24) +#define EVT_ID_MASKGENMASK(23, 16) +#define SRC_ID_MASKGENMASK(15, 0) + +/* + * Builds an unsigned 32bit key from the given input tuple to be used + * as a key in hashtables. + */ +#define MAKE_HASH_KEY(p, e, s) \ + (FIELD_PREP(PROTO_ID_MASK, (p)) | \ + FIELD_PREP(EVT_ID_MASK, (e)) | \ + FIELD_PREP(SRC_ID_MASK, (s))) + +#define MAKE_ALL_SRCS_KEY(p, e)\ + MAKE_HASH_KEY((p), (e), SRC_ID_MASK) + +struct scmi_registered_events_desc; + +/** + * struct scmi_notify_instance - Represents an instance
[PATCH v9 3/9] firmware: arm_scmi: Add notification dispatch and delivery
Add core SCMI Notifications dispatch and delivery support logic which is
able, at first, to dispatch well-known received events from the RX ISR to
the dedicated deferred worker, and then, from there, to final deliver the
events to the registered users' callbacks.
Dispatch and delivery is just added here, still not enabled.
Reviewed-by: Jonathan Cameron
Signed-off-by: Cristian Marussi
---
V8 --> V9
- fixed pr_ to dev_
V7 --> V8
- Fixed enabled check in scmi_notify() not to use atomics
- Added a few comments about queueing works
V5 --> V6
- added handle argument to fill_custom_report()
V4 --> V5
- fixed kernel-doc
- fixed unneded var initialization
V3 --> V4
- dispatcher now handles dequeuing of events in chunks (header+payload):
handling of these in_flight events let us remove one unneeded memcpy
on RX interrupt path (scmi_notify)
- deferred dispatcher now access their own per-protocol handlers' table
reducing locking contention on the RX path
V2 --> V3
- exposing wq in sysfs via WQ_SYSFS
V1 --> V2
- splitted out of V1 patch 04
- moved from IDR maps to real HashTables to store event_handlers
- simplified delivery logic
---
drivers/firmware/arm_scmi/notify.c | 359 -
drivers/firmware/arm_scmi/notify.h | 10 +
2 files changed, 365 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/arm_scmi/notify.c
b/drivers/firmware/arm_scmi/notify.c
index 609aa5ab4875..e84fa4dcf5a6 100644
--- a/drivers/firmware/arm_scmi/notify.c
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -47,6 +47,27 @@
* as described in the SCMI Protocol specification, while src_id represents an
* optional, protocol dependent, source identifier (like domain_id, perf_id
* or sensor_id and so forth).
+ *
+ * Upon reception of a notification message from the platform the SCMI RX ISR
+ * passes the received message payload and some ancillary information
(including
+ * an arrival timestamp in nanoseconds) to the core via @scmi_notify() which
+ * pushes the event-data itself on a protocol-dedicated kfifo queue for further
+ * deferred processing as specified in @scmi_events_dispatcher().
+ *
+ * Each protocol has it own dedicated work_struct and worker which, once kicked
+ * by the ISR, takes care to empty its own dedicated queue, deliverying the
+ * queued items into the proper notification-chain: notifications processing
can
+ * proceed concurrently on distinct workers only between events belonging to
+ * different protocols while delivery of events within the same protocol is
+ * still strictly sequentially ordered by time of arrival.
+ *
+ * Events' information is then extracted from the SCMI Notification messages
and
+ * conveyed, converted into a custom per-event report struct, as the void *data
+ * param to the user callback provided by the registered notifier_block, so
that
+ * from the user perspective his callback will look invoked like:
+ *
+ * int user_cb(struct notifier_block *nb, unsigned long event_id, void *report)
+ *
*/
#define dev_fmt(fmt) "SCMI Notifications - " fmt
@@ -67,6 +88,7 @@
#include
#include
#include
+#include
#include "notify.h"
@@ -152,6 +174,9 @@
#define REVT_NOTIFY_DISABLE(revt, eid, sid) \
((revt)->proto->ops->set_notify_enabled((revt)->proto->ni->handle, \
(eid), (sid), false))
+#define REVT_FILL_REPORT(revt, ...) \
+ ((revt)->proto->ops->fill_custom_report((revt)->proto->ni->handle, \
+ __VA_ARGS__))
#define SCMI_PENDING_HASH_SZ 4
#define SCMI_REGISTERED_HASH_SZ6
@@ -164,6 +189,7 @@ struct scmi_registered_events_desc;
* @gid: GroupID used for devres
* @handle: A reference to the platform instance
* @init_work: A work item to perform final initializations of pending handlers
+ * @notify_wq: A reference to the allocated Kernel cmwq
* @pending_mtx: A mutex to protect @pending_events_handlers
* @registered_protocols: A statically allocated array containing pointers to
* all the registered protocol-level specific information
@@ -179,6 +205,7 @@ struct scmi_notify_instance {
struct scmi_handle *handle;
struct work_struct init_work;
+ struct workqueue_struct *notify_wq;
struct mutexpending_mtx;
struct scmi_registered_events_desc **registered_protocols;
@@ -189,12 +216,16 @@ struct scmi_notify_instance {
* struct events_queue - Describes a queue and its associated worker
* @sz: Size in bytes of the related kfifo
* @kfifo: A dedicated Kernel kfifo descriptor
+ * @notify_work: A custom work item bound to this queue
+ * @wq: A reference to the associated workqueue
*
* Each protocol has its own dedicated events_queue descriptor.
*/
struct events_queue {
-
[PATCH v9 2/9] firmware: arm_scmi: Add notification callbacks-registration
Add core SCMI Notifications callbacks-registration support: allow users to register their own callbacks against the desired events. Whenever a registration request is issued against a still non existent event, mark such request as pending for later processing, in order to account for possible late initializations of SCMI Protocols associated to loadable drivers. Reviewed-by: Jonathan Cameron Signed-off-by: Cristian Marussi --- V8 --> V9 - moved out needed defines to previous patch - fixed unneeded NULL inits in macro - shrinked hastables' sizes and introduced meaningful defines - fix comment in scmi_unregister_notifier - move pr_infos to dev_dbg - removed MAP_EVT_TO_ENABLE_CMD (will be inlined) V7 --> V8 - Fixed init check, un-needed atomics removed V6 --> V7 - removed un-needed ktime.h include V4 --> V5 - fix kernel-doc - reviewed REVT_NOTIFY_ENABLE macro - added matching barrier for late_init V3 --> V4 - split registered_handlers hashtable on a per-protocol basis to reduce unneeded contention - introduced pending_handlers table and related late_init worker to finalize handlers registration upon effective protocols' registrations - introduced further safe accessors macros for registered_protocols and registered_events arrays V2 --> V3 - refactored get/put event_handler - removed generic non-handle-based API V1 --> V2 - splitted out of V1 patch 04 - moved from IDR maps to real HashTables to store event_handlers - added proper enable_events refcounting via __scmi_enable_evt() [was broken in V1 when using ALL_SRCIDs notification chains] - reviewed hashtable cleanup strategy in scmi_notifications_exit() - added scmi_register_event_notifier()/scmi_unregister_event_notifier() to include/linux/scmi_protocol.h as a candidate user API [no EXPORTs still] - added notify_ops to handle during initialization as an additional internal API for scmi_drivers --- drivers/firmware/arm_scmi/notify.c | 707 + include/linux/scmi_protocol.h | 46 ++ 2 files changed, 753 insertions(+) diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c index b0ba4da22343..609aa5ab4875 100644 --- a/drivers/firmware/arm_scmi/notify.c +++ b/drivers/firmware/arm_scmi/notify.c @@ -19,18 +19,50 @@ * this core its set of supported events using scmi_register_protocol_events(): * all the needed descriptors are stored in the registered_protocols and * registered_events arrays. + * + * Kernel users interested in some specific event can register their callbacks + * providing the usual notifier_block descriptor, since this core implements + * events' delivery using the standard Kernel notification chains machinery. + * + * Given the number of possible events defined by SCMI and the extensibility + * of the SCMI Protocol itself, the underlying notification chains are created + * and destroyed dynamically on demand depending on the number of users + * effectively registered for an event, so that no support structures or chains + * are allocated until at least one user has registered a notifier_block for + * such event. Similarly, events' generation itself is enabled at the platform + * level only after at least one user has registered, and it is shutdown after + * the last user for that event has gone. + * + * All users provided callbacks and allocated notification-chains are stored in + * the @registered_events_handlers hashtable. Callbacks' registration requests + * for still to be registered events are instead kept in the dedicated common + * hashtable @pending_events_handlers. + * + * An event is identified univocally by the tuple (proto_id, evt_id, src_id) + * and is served by its own dedicated notification chain; information contained + * in such tuples is used, in a few different ways, to generate the needed + * hash-keys. + * + * Here proto_id and evt_id are simply the protocol_id and message_id numbers + * as described in the SCMI Protocol specification, while src_id represents an + * optional, protocol dependent, source identifier (like domain_id, perf_id + * or sensor_id and so forth). */ #define dev_fmt(fmt) "SCMI Notifications - " fmt +#define pr_fmt(fmt) "SCMI Notifications - " fmt #include #include #include #include #include +#include #include #include +#include #include +#include #include #include #include @@ -56,6 +88,74 @@ #define MAKE_ALL_SRCS_KEY(p, e)\ MAKE_HASH_KEY((p), (e), SRC_ID_MASK) +/* + * Assumes that the stored obj includes its own hash-key in a field named 'key': + * with this simplification this macro can be equally used for all the objects' + * types hashed by this implementation. + * + * @__ht: The hashtable name + * @__obj: A pointer to the object type to be retrieved from the hashtable; + *it will be used as a cursor while scanning the hastable and it will + *be possibly left as NULL when @__k is not found + * @__k: The key to search for + */
[PATCH v9 5/9] firmware: arm_scmi: Add Power notifications support
Make SCMI Power protocol register with the notification core.
Reviewed-by: Jonathan Cameron
Signed-off-by: Cristian Marussi
---
V8 --> V9
- moved pr_info to pr_debug
- removed switch()
- use SCMI_PROTO_QUEUE_SZ
V6 --> V7
- fixed report.timestamp type
- removed POWER_STATE_CHANGE_REQUESTED motification handling (deprecated)
- fixed max_payld_sz initialization
- expose SCMI_EVENT_ in linux/scmi_protocol.h
V5 --> V6
- added handle argument to fill_custom_report()
V4 --> V5
- fixed unsual return construct
V3 --> V4
- scmi_event field renamed
V2 --> V3
- added handle awareness
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/power.c | 92 +--
include/linux/scmi_protocol.h | 12
2 files changed, 98 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/arm_scmi/power.c
b/drivers/firmware/arm_scmi/power.c
index cf7f0312381b..b9714755a320 100644
--- a/drivers/firmware/arm_scmi/power.c
+++ b/drivers/firmware/arm_scmi/power.c
@@ -5,19 +5,18 @@
* Copyright (C) 2018 ARM Ltd.
*/
+#define pr_fmt(fmt) "SCMI Notifications POWER - " fmt
+
+#include
+
#include "common.h"
+#include "notify.h"
enum scmi_power_protocol_cmd {
POWER_DOMAIN_ATTRIBUTES = 0x3,
POWER_STATE_SET = 0x4,
POWER_STATE_GET = 0x5,
POWER_STATE_NOTIFY = 0x6,
- POWER_STATE_CHANGE_REQUESTED_NOTIFY = 0x7,
-};
-
-enum scmi_power_protocol_notify {
- POWER_STATE_CHANGED = 0x0,
- POWER_STATE_CHANGE_REQUESTED = 0x1,
};
struct scmi_msg_resp_power_attributes {
@@ -48,6 +47,12 @@ struct scmi_power_state_notify {
__le32 notify_enable;
};
+struct scmi_power_state_notify_payld {
+ __le32 agent_id;
+ __le32 domain_id;
+ __le32 power_state;
+};
+
struct power_dom_info {
bool state_set_sync;
bool state_set_async;
@@ -186,6 +191,75 @@ static struct scmi_power_ops power_ops = {
.state_get = scmi_power_state_get,
};
+static int scmi_power_request_notify(const struct scmi_handle *handle,
+u32 domain, bool enable)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_power_state_notify *notify;
+
+ ret = scmi_xfer_get_init(handle, POWER_STATE_NOTIFY,
+SCMI_PROTOCOL_POWER, sizeof(*notify), 0, );
+ if (ret)
+ return ret;
+
+ notify = t->tx.buf;
+ notify->domain = cpu_to_le32(domain);
+ notify->notify_enable = enable ? cpu_to_le32(BIT(0)) : 0;
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static bool scmi_power_set_notify_enabled(const struct scmi_handle *handle,
+ u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ ret = scmi_power_request_notify(handle, src_id, enable);
+ if (ret)
+ pr_debug("FAIL_ENABLE - evt[%X] dom[%d] - ret:%d\n",
+evt_id, src_id, ret);
+
+ return !ret;
+}
+
+static void *scmi_power_fill_custom_report(const struct scmi_handle *handle,
+ u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ const struct scmi_power_state_notify_payld *p = payld;
+ struct scmi_power_state_changed_report *r = report;
+
+ if (evt_id != SCMI_EVENT_POWER_STATE_CHANGED || sizeof(*p) != payld_sz)
+ return NULL;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->domain_id = le32_to_cpu(p->domain_id);
+ r->power_state = le32_to_cpu(p->power_state);
+ *src_id = r->domain_id;
+
+ return r;
+}
+
+static const struct scmi_event power_events[] = {
+ {
+ .id = SCMI_EVENT_POWER_STATE_CHANGED,
+ .max_payld_sz = sizeof(struct scmi_power_state_notify_payld),
+ .max_report_sz =
+ sizeof(struct scmi_power_state_changed_report),
+ },
+};
+
+static const struct scmi_event_ops power_event_ops = {
+ .set_notify_enabled = scmi_power_set_notify_enabled,
+ .fill_custom_report = scmi_power_fill_custom_report,
+};
+
static int scmi_power_protocol_init(struct scmi_handle *handle)
{
int domain;
@@ -214,6 +288,12 @@ static int scmi_power_protocol_init(struct scmi_handle
*handle)
scmi_power_domain_attributes_get(handle, domain, dom);
}
+ scmi_register_protocol_events(handle,
+ SCMI_PROTOCOL_POWER, SCMI_PROTO_QUEUE_SZ,
+ _event_ops, power_events,
+
[PATCH v9 7/9] firmware: arm_scmi: Add Sensor notifications support
Make SCMI Sensor protocol register with the notification core.
Reviewed-by: Jonathan Cameron
Signed-off-by: Cristian Marussi
---
V8 --> V9
- moved pr_info to pr_debug
- removed switch()
- use SCMI_PROTO_QUEUE_SZ
V6 --> V7
- fixed report.timestamp type
- removed trip_point_notify from .sensor_ops
- fixed max_payld_sz initialization
- expose SCMI_EVENT_ in linux/scmi_protocol.h
V5 --> V6
- added handle argument to fill_custom_report()
V4 --> V5
- fixed unsual return construct
V3 --> V4
- scmi_event field renamed
V2 --> V3
- added handle awareness
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/sensors.c | 69 ++---
include/linux/scmi_protocol.h | 13 +++---
2 files changed, 72 insertions(+), 10 deletions(-)
diff --git a/drivers/firmware/arm_scmi/sensors.c
b/drivers/firmware/arm_scmi/sensors.c
index db1b1ab303da..f88b3d422f45 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -5,7 +5,12 @@
* Copyright (C) 2018 ARM Ltd.
*/
+#define pr_fmt(fmt) "SCMI Notifications SENSOR - " fmt
+
+#include
+
#include "common.h"
+#include "notify.h"
enum scmi_sensor_protocol_cmd {
SENSOR_DESCRIPTION_GET = 0x3,
@@ -14,10 +19,6 @@ enum scmi_sensor_protocol_cmd {
SENSOR_READING_GET = 0x6,
};
-enum scmi_sensor_protocol_notify {
- SENSOR_TRIP_POINT_EVENT = 0x0,
-};
-
struct scmi_msg_resp_sensor_attributes {
__le16 num_sensors;
u8 max_requests;
@@ -71,6 +72,12 @@ struct scmi_msg_sensor_reading_get {
#define SENSOR_READ_ASYNC BIT(0)
};
+struct scmi_sensor_trip_notify_payld {
+ __le32 agent_id;
+ __le32 sensor_id;
+ __le32 trip_point_desc;
+};
+
struct sensors_info {
u32 version;
int num_sensors;
@@ -271,11 +278,57 @@ static int scmi_sensor_count_get(const struct scmi_handle
*handle)
static struct scmi_sensor_ops sensor_ops = {
.count_get = scmi_sensor_count_get,
.info_get = scmi_sensor_info_get,
- .trip_point_notify = scmi_sensor_trip_point_notify,
.trip_point_config = scmi_sensor_trip_point_config,
.reading_get = scmi_sensor_reading_get,
};
+static bool scmi_sensor_set_notify_enabled(const struct scmi_handle *handle,
+ u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ ret = scmi_sensor_trip_point_notify(handle, src_id, enable);
+ if (ret)
+ pr_debug("FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
+evt_id, src_id, ret);
+
+ return !ret;
+}
+
+static void *scmi_sensor_fill_custom_report(const struct scmi_handle *handle,
+ u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ const struct scmi_sensor_trip_notify_payld *p = payld;
+ struct scmi_sensor_trip_point_report *r = report;
+
+ if (evt_id != SCMI_EVENT_SENSOR_TRIP_POINT_EVENT ||
+sizeof(*p) != payld_sz)
+ return NULL;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->sensor_id = le32_to_cpu(p->sensor_id);
+ r->trip_point_desc = le32_to_cpu(p->trip_point_desc);
+ *src_id = r->sensor_id;
+
+ return r;
+}
+
+static const struct scmi_event sensor_events[] = {
+ {
+ .id = SCMI_EVENT_SENSOR_TRIP_POINT_EVENT,
+ .max_payld_sz = sizeof(struct scmi_sensor_trip_notify_payld),
+ .max_report_sz = sizeof(struct scmi_sensor_trip_point_report),
+ },
+};
+
+static const struct scmi_event_ops sensor_event_ops = {
+ .set_notify_enabled = scmi_sensor_set_notify_enabled,
+ .fill_custom_report = scmi_sensor_fill_custom_report,
+};
+
static int scmi_sensors_protocol_init(struct scmi_handle *handle)
{
u32 version;
@@ -299,6 +352,12 @@ static int scmi_sensors_protocol_init(struct scmi_handle
*handle)
scmi_sensor_description_get(handle, sinfo);
+ scmi_register_protocol_events(handle,
+ SCMI_PROTOCOL_SENSOR, SCMI_PROTO_QUEUE_SZ,
+ _event_ops, sensor_events,
+ ARRAY_SIZE(sensor_events),
+ sinfo->num_sensors);
+
sinfo->version = version;
handle->sensor_ops = _ops;
handle->sensor_priv = sinfo;
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 832b03ef37c7..7d1e8d24c880 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -174,18 +174,13 @@ enum scmi_sensor_class
[PATCH v9 4/9] firmware: arm_scmi: Enable notification core
Initialize and enable SCMI Notifications core support during bus/driver
probe phase, so that protocols can start registering their supported
events during their initialization.
Reviewed-by: Jonathan Cameron
Signed-off-by: Cristian Marussi
---
V3 --> V4
- simplified core initialization: protocols events' registrations is now
disjoint from users' callback registrations, so that events' generation
can be enabled earlier for registered events and delayed for pending
ones in order to support deferred (or missing) protocol initialization
V2 --> V3
- reviewed core initialization: all implemented protocols must complete
their protocol-events registration phases before notifications can be
enabled as a whole; in the meantime any user's callback registration
requests possibly issued while the notifications were not enabled
remain pending: a dedicated worker completes the handlers registration
once all protocols have been initialized.
NOTE THAT this can lead to ISSUES with late inserted or missing SCMI
modules (i.e. for protocols defined in the DT and implemented by the
platform but lazily loaded or not loaded at all.), since in these
scenarios notifications dispatching will be enabled later or never.
- reviewed core exit: protocol users (devices) are accounted on probe/
remove, and protocols' events are unregisteredonce last user go
(can happen only at shutdown)
V1 --> V2
- added timestamping
- moved notification init/exit and using devres
---
drivers/firmware/arm_scmi/driver.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/firmware/arm_scmi/driver.c
b/drivers/firmware/arm_scmi/driver.c
index 7483cacf63f9..27288aef74c4 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -26,6 +26,7 @@
#include
#include "common.h"
+#include "notify.h"
#define CREATE_TRACE_POINTS
#include
@@ -204,11 +205,13 @@ __scmi_xfer_put(struct scmi_xfers_info *minfo, struct
scmi_xfer *xfer)
static void scmi_handle_notification(struct scmi_chan_info *cinfo, u32 msg_hdr)
{
+ u64 ts;
struct scmi_xfer *xfer;
struct device *dev = cinfo->dev;
struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
struct scmi_xfers_info *minfo = >rx_minfo;
+ ts = ktime_get_boottime_ns();
xfer = scmi_xfer_get(cinfo->handle, minfo);
if (IS_ERR(xfer)) {
dev_err(dev, "failed to get free message slot (%ld)\n",
@@ -221,6 +224,8 @@ static void scmi_handle_notification(struct scmi_chan_info
*cinfo, u32 msg_hdr)
scmi_dump_header_dbg(dev, >hdr);
info->desc->ops->fetch_notification(cinfo, info->desc->max_msg_size,
xfer);
+ scmi_notify(cinfo->handle, xfer->hdr.protocol_id,
+ xfer->hdr.id, xfer->rx.buf, xfer->rx.len, ts);
trace_scmi_rx_done(xfer->transfer_id, xfer->hdr.id,
xfer->hdr.protocol_id, xfer->hdr.seq,
@@ -789,6 +794,9 @@ static int scmi_probe(struct platform_device *pdev)
if (ret)
return ret;
+ if (scmi_notification_init(handle))
+ dev_err(dev, "SCMI Notifications NOT available.\n");
+
ret = scmi_base_protocol_init(handle);
if (ret) {
dev_err(dev, "unable to communicate with SCMI(%d)\n", ret);
@@ -831,6 +839,8 @@ static int scmi_remove(struct platform_device *pdev)
struct scmi_info *info = platform_get_drvdata(pdev);
struct idr *idr = >tx_idr;
+ scmi_notification_exit(>handle);
+
mutex_lock(_list_mutex);
if (info->users)
ret = -EBUSY;
--
2.17.1
[PATCH v9 6/9] firmware: arm_scmi: Add Perf notifications support
Make SCMI Perf protocol register with the notification core.
Reviewed-by: Jonathan Cameron
Signed-off-by: Cristian Marussi
---
V8 --> V9
- moved pr_info to pr_debug
- inlined MAP_EVT_TO_ENABLE_CMD
- use SCMI_PROTO_QUEUE_SZ
V6 --> V7
- fixed report.timestamp type
- fixed max_payld_sz initialization
- expose SCMI_EVENT_ in linux/scmi_protocol.h
V5 --> V6
- added handle argument to fill_custom_report()
V4 --> V5
- fixed unsual return construct
V3 --> V4
- scmi_event field renamed
V2 --> V3
- added handle awareness
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/perf.c | 139 +--
include/linux/scmi_protocol.h| 17
2 files changed, 151 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index eadc171e254b..19ea773fed5d 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -5,15 +5,19 @@
* Copyright (C) 2018 ARM Ltd.
*/
+#define pr_fmt(fmt) "SCMI Notifications PERF - " fmt
+
#include
#include
#include
#include
#include
#include
+#include
#include
#include "common.h"
+#include "notify.h"
enum scmi_performance_protocol_cmd {
PERF_DOMAIN_ATTRIBUTES = 0x3,
@@ -27,11 +31,6 @@ enum scmi_performance_protocol_cmd {
PERF_DESCRIBE_FASTCHANNEL = 0xb,
};
-enum scmi_performance_protocol_notify {
- PERFORMANCE_LIMITS_CHANGED = 0x0,
- PERFORMANCE_LEVEL_CHANGED = 0x1,
-};
-
struct scmi_opp {
u32 perf;
u32 power;
@@ -86,6 +85,19 @@ struct scmi_perf_notify_level_or_limits {
__le32 notify_enable;
};
+struct scmi_perf_limits_notify_payld {
+ __le32 agent_id;
+ __le32 domain_id;
+ __le32 range_max;
+ __le32 range_min;
+};
+
+struct scmi_perf_level_notify_payld {
+ __le32 agent_id;
+ __le32 domain_id;
+ __le32 performance_level;
+};
+
struct scmi_msg_resp_perf_describe_levels {
__le16 num_returned;
__le16 num_remaining;
@@ -158,6 +170,11 @@ struct scmi_perf_info {
struct perf_dom_info *dom_info;
};
+static enum scmi_performance_protocol_cmd evt_2_cmd[] = {
+ PERF_NOTIFY_LIMITS,
+ PERF_NOTIFY_LEVEL,
+};
+
static int scmi_perf_attributes_get(const struct scmi_handle *handle,
struct scmi_perf_info *pi)
{
@@ -488,6 +505,29 @@ static int scmi_perf_level_get(const struct scmi_handle
*handle, u32 domain,
return scmi_perf_mb_level_get(handle, domain, level, poll);
}
+static int scmi_perf_level_limits_notify(const struct scmi_handle *handle,
+u32 domain, int message_id,
+bool enable)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_perf_notify_level_or_limits *notify;
+
+ ret = scmi_xfer_get_init(handle, message_id, SCMI_PROTOCOL_PERF,
+sizeof(*notify), 0, );
+ if (ret)
+ return ret;
+
+ notify = t->tx.buf;
+ notify->domain = cpu_to_le32(domain);
+ notify->notify_enable = enable ? cpu_to_le32(BIT(0)) : 0;
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
static bool scmi_perf_fc_size_is_valid(u32 msg, u32 size)
{
if ((msg == PERF_LEVEL_GET || msg == PERF_LEVEL_SET) && size == 4)
@@ -710,6 +750,89 @@ static struct scmi_perf_ops perf_ops = {
.est_power_get = scmi_dvfs_est_power_get,
};
+static bool scmi_perf_set_notify_enabled(const struct scmi_handle *handle,
+u8 evt_id, u32 src_id, bool enable)
+{
+ int ret, cmd_id;
+
+ if (unlikely(evt_id >= ARRAY_SIZE(evt_2_cmd)))
+ return false;
+
+ cmd_id = evt_2_cmd[evt_id];
+ ret = scmi_perf_level_limits_notify(handle, src_id, cmd_id, enable);
+ if (ret)
+ pr_debug("FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
+evt_id, src_id, ret);
+
+ return !ret;
+}
+
+static void *scmi_perf_fill_custom_report(const struct scmi_handle *handle,
+ u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ void *rep = NULL;
+
+ switch (evt_id) {
+ case SCMI_EVENT_PERFORMANCE_LIMITS_CHANGED:
+ {
+ const struct scmi_perf_limits_notify_payld *p = payld;
+ struct scmi_perf_limits_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ break;
+
+ r->timestamp = timestamp;
+ r->agent_id =
[PATCH v9 8/9] firmware: arm_scmi: Add Reset notifications support
Make SCMI Reset protocol register with the notification core.
Reviewed-by: Jonathan Cameron
Signed-off-by: Cristian Marussi
---
V8 --> V9
- moved pr_info to pr_debug
- removed switch()
- use SCMI_PROTO_QUEUE_SZ
V6 --> V7
- fixed report.timestamp type
- added agent_id notification field
- fixed .max_payld_sz initialization
- expose SCMI_EVENT_ in linux/scmi_protocol.h
V5 --> V6
- added handle argument to fill_custom_report()
V4 --> V5
- fixed unsual return construct
V3 --> V4
- scmi_event field renamed
V2 --> V3
- added handle awareness
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/reset.c | 96 +--
include/linux/scmi_protocol.h | 8 +++
2 files changed, 100 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/arm_scmi/reset.c
b/drivers/firmware/arm_scmi/reset.c
index de73054554f3..2aea26475376 100644
--- a/drivers/firmware/arm_scmi/reset.c
+++ b/drivers/firmware/arm_scmi/reset.c
@@ -5,7 +5,12 @@
* Copyright (C) 2019 ARM Ltd.
*/
+#define pr_fmt(fmt) "SCMI Notifications RESET - " fmt
+
+#include
+
#include "common.h"
+#include "notify.h"
enum scmi_reset_protocol_cmd {
RESET_DOMAIN_ATTRIBUTES = 0x3,
@@ -13,10 +18,6 @@ enum scmi_reset_protocol_cmd {
RESET_NOTIFY = 0x5,
};
-enum scmi_reset_protocol_notify {
- RESET_ISSUED = 0x0,
-};
-
#define NUM_RESET_DOMAIN_MASK 0x
#define RESET_NOTIFY_ENABLEBIT(0)
@@ -40,6 +41,18 @@ struct scmi_msg_reset_domain_reset {
#define ARCH_COLD_RESET(ARCH_RESET_TYPE | COLD_RESET_STATE)
};
+struct scmi_msg_reset_notify {
+ __le32 id;
+ __le32 event_control;
+#define RESET_TP_NOTIFY_ALLBIT(0)
+};
+
+struct scmi_reset_issued_notify_payld {
+ __le32 agent_id;
+ __le32 domain_id;
+ __le32 reset_state;
+};
+
struct reset_dom_info {
bool async_reset;
bool reset_notify;
@@ -190,6 +203,75 @@ static struct scmi_reset_ops reset_ops = {
.deassert = scmi_reset_domain_deassert,
};
+static int scmi_reset_notify(const struct scmi_handle *handle, u32 domain_id,
+bool enable)
+{
+ int ret;
+ u32 evt_cntl = enable ? RESET_TP_NOTIFY_ALL : 0;
+ struct scmi_xfer *t;
+ struct scmi_msg_reset_notify *cfg;
+
+ ret = scmi_xfer_get_init(handle, RESET_NOTIFY,
+SCMI_PROTOCOL_RESET, sizeof(*cfg), 0, );
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->id = cpu_to_le32(domain_id);
+ cfg->event_control = cpu_to_le32(evt_cntl);
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static bool scmi_reset_set_notify_enabled(const struct scmi_handle *handle,
+ u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ ret = scmi_reset_notify(handle, src_id, enable);
+ if (ret)
+ pr_debug("FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
+evt_id, src_id, ret);
+
+ return !ret;
+}
+
+static void *scmi_reset_fill_custom_report(const struct scmi_handle *handle,
+ u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ const struct scmi_reset_issued_notify_payld *p = payld;
+ struct scmi_reset_issued_report *r = report;
+
+ if (evt_id != SCMI_EVENT_RESET_ISSUED || sizeof(*p) != payld_sz)
+ return NULL;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->domain_id = le32_to_cpu(p->domain_id);
+ r->reset_state = le32_to_cpu(p->reset_state);
+ *src_id = r->domain_id;
+
+ return r;
+}
+
+static const struct scmi_event reset_events[] = {
+ {
+ .id = SCMI_EVENT_RESET_ISSUED,
+ .max_payld_sz = sizeof(struct scmi_reset_issued_notify_payld),
+ .max_report_sz = sizeof(struct scmi_reset_issued_report),
+ },
+};
+
+static const struct scmi_event_ops reset_event_ops = {
+ .set_notify_enabled = scmi_reset_set_notify_enabled,
+ .fill_custom_report = scmi_reset_fill_custom_report,
+};
+
static int scmi_reset_protocol_init(struct scmi_handle *handle)
{
int domain;
@@ -218,6 +300,12 @@ static int scmi_reset_protocol_init(struct scmi_handle
*handle)
scmi_reset_domain_attributes_get(handle, domain, dom);
}
+ scmi_register_protocol_events(handle,
+ SCMI_PROTOCOL_RESET, SCMI_PROTO_QUEUE_SZ,
+ _event_ops, reset_events,
+
[PATCH v9 0/9] SCMI Notifications Core Support
Hi all,
this series wants to introduce SCMI Notification Support, built on top of
the standard Kernel notification chain subsystem.
At initialization time each SCMI Protocol takes care to register with the
new SCMI notification core the set of its own events which it intends to
support.
Using the API exposed via scmi_handle.notify_ops a Kernel user can register
its own notifier_t callback (via a notifier_block as usual) against any
registered event as identified by the tuple:
(proto_id, event_id, src_id)
where src_id represents a generic source identifier which is protocol
dependent like domain_id, performance_id, sensor_id and so forth.
(users can anyway do NOT provide any src_id, and subscribe instead to ALL
the existing (if any) src_id sources for that proto_id/evt_id combination)
Each of the above tuple-specified eventis will be served on its own
dedicated blocking notification chain, dynamically allocated on-demand when
at least one user has shown interest on that event.
Upon a notification delivery all the users' registered notifier_t callbacks
will be in turn invoked and fed with the event_id as @action param and a
generated custom per-event struct _report as @data param.
(as in include/linux/scmi_protocol.h)
The final step of notification delivery via users' callback invocation is
instead delegated to a pool of deferred workers (Kernel cmwq): each
SCMI protocol has its own dedicated worker and dedicated queue to push
events from the rx ISR to the worker.
Based on scmi-next/for-next/scmi 5.8 [1], on top of:
commit 5a897e3ab429 ("firmware: arm_scmi: fix psci dependency")
This series has been tested on JUNO with an experimental firmware only
supporting Perf Notifications.
Thanks
Cristian
v8 --> v9:
- rebased on top of scmi-next 5.8
- moved some pr_info() to dev_dbg()
- moved around some macros definitions (using FIELD_PREPARE properly)
- introduced some meaningful define
- shrunk hashtables' sizes
- shortened the naming of some massively long data struct
v7 --> v8:
- removed unneeded initialized/enabled atomics, added proper barriers
- added a few comments about queueing work item and kfifos
v6 --> v7:
- rebased on top of scmi-next 5.7, dropped the initial 4 patches
since now already queued on base scmi-next [1]
- fixed some events' proto initialization
- removed some notify_enabled explicit methods exposed in some protocol_ops
since not supposed to be used directly when using this notification
framework (and of no other known use)
- exposing SCMI_EVENT_ enums in scmi_protocol.h
- added agent_id field in RESET_ISSUED payload as per reviewed SCMI spec
- removed POWER_STATE_CHANGE_REQUESTED pre-notification definition and
handling as per reviewedSCMI spec
- fixed report.timestamp field type
v5 --> v6:
- added handle argument to fill_custom_report() helper
v4 --> v5:
- fixed kernel-doc
- added proper barriers around registered protocols and events
initialization
- reviewed queues allocation using devm_add_action_or_reset
- reviewed REVT_NOTIFY_ENABLE macro
v3 --> v4:
- dropped RFC tag
- avoid one unneeded evt payload memcpy on the ISR RC code path by
redesigning dispatcher to handle partial queue-reads (in_flight events,
only header)
- fixed the initialization issue exposed by late SCMI modules loading by
reviewing the init process to support possible late events registrations
by protocols and early callbacks registrations by users (pending)
- cleanup/simplification of exit path: SCMI protocols are generally never
de-initialized after the initial device creation, so do not deinit
notification core either (we do halt the delivery, stop the wq and empty
the queues though)
- reduced contention on regustered_events_handler to the minimum during
delivery by splitting the common registered_events_handlers hashtable
into a number of per-protocol tables
- converted registered_protocols and registered_events hastable to
fixed size arrays: simpler and lockless in our usage scenario
v2 --> v3:
- added platform instance awareness to the notification core: a
notification instance is created for each known handle
- reviewed notification core initialization and shutdown process
- removed generic non-handle-rooted registration API
- added WQ_SYSFS flag to workqueue instance
v1 --> v2:
- dropped anti-tampering patch
- rebased on top of scmi-for-next-5.6, which includes Viresh series that
make SCMI core independent of transport (5c8a47a5a91d)
- add a few new SCMI transport methods on top of Viresh patch to address
needs of SCMI Notifications
- reviewed/renamed scmi_handle_xfer_delayed_resp()
- split main SCMI Notification core patch (~1k lines) into three chunks:
protocol-registration / callbacks-registration / dispatch-and-delivery
- removed awkward usage of IDR maps in favour of pure hashtables
- added enable/disable refcounting in notification core (was broken in v1)
- removed per-protocol candidate API: a single generic API is now
[PATCH] Revert "ARM: sti: Implement dummy L2 cache's write_sec"
From: Patrice Chotard
This reverts commit 7b8e0188fa717cd9abc4fb52587445b421835c2a.
Initially, STiH410-B2260 was supposed to be secured, that's why
l2c_write_sec was stubbed to avoid secure register access from
non secure world.
But by default, STiH410-B2260 is running in non secure mode,
so L2 cache register accesses are authorized, l2c_write_sec stub
is not needed.
With this patch, L2 cache is configured and performance are enhanced.
Signed-off-by: Patrice Chotard
Cc: Alain Volmat
---
arch/arm/mach-sti/board-dt.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/arch/arm/mach-sti/board-dt.c b/arch/arm/mach-sti/board-dt.c
index dcb98937fcf5..ffecbf29646f 100644
--- a/arch/arm/mach-sti/board-dt.c
+++ b/arch/arm/mach-sti/board-dt.c
@@ -20,14 +20,6 @@ static const char *const stih41x_dt_match[] __initconst = {
NULL
};
-static void sti_l2_write_sec(unsigned long val, unsigned reg)
-{
- /*
-* We can't write to secure registers as we are in non-secure
-* mode, until we have some SMI service available.
-*/
-}
-
DT_MACHINE_START(STM, "STi SoC with Flattened Device Tree")
.dt_compat = stih41x_dt_match,
.l2c_aux_val= L2C_AUX_CTRL_SHARED_OVERRIDE |
@@ -36,5 +28,4 @@ DT_MACHINE_START(STM, "STi SoC with Flattened Device Tree")
L2C_AUX_CTRL_WAY_SIZE(4),
.l2c_aux_mask = 0xcfff,
.smp= smp_ops(sti_smp_ops),
- .l2c_write_sec = sti_l2_write_sec,
MACHINE_END
--
2.17.1
Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs
> > + // Handle two first channels.
> > + for (i = 0; i < FREE_N_CHANNELS; i++) {
> > + for (; bkvhead[i]; bkvhead[i] = bnext) {
> > + bnext = bkvhead[i]->next;
> > + debug_rcu_bhead_unqueue(bkvhead[i]);
> > +
> > + rcu_lock_acquire(_callback_map);
> > + if (i == 0) { // kmalloc() / kfree().
> > + trace_rcu_invoke_kfree_bulk_callback(
> > + rcu_state.name, bkvhead[i]->nr_records,
> > + bkvhead[i]->records);
> > +
> > + kfree_bulk(bkvhead[i]->nr_records,
> > + bkvhead[i]->records);
> > + } else { // vmalloc() / vfree().
> > + for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > + trace_rcu_invoke_kfree_callback(
> > + rcu_state.name,
> > + bkvhead[i]->records[j], 0);
> > +
> > + vfree(bkvhead[i]->records[j]);
> > + }
> > + }
> > + rcu_lock_release(_callback_map);
>
> Not an emergency, but did you look into replacing this "if" statement
> with an array of pointers to functions implementing the legs of the
> "if" statement? If nothing else, this would greatly reduced indentation.
>
>
> I am taking this as is, but if you have not already done so, could you
> please look into this for a follow-up patch?
>
I do not think it makes sense, because it would require to check each
pointer in the array, what can lead to many branching, i.e. "if-else"
instructions.
Paul, thank you to take it in!
--
Vlad Rezki
WARNING with LBR + precise_ip=2 + bpf_get_stackid()
Hi,
We are debugging some WARNING with LBR, precise_ip=2 and bpf_get_stackid(),
like:
[36000.334284] WARNING: stack recursion on stack type 1
[36000.334288] WARNING: can't access registers at
syscall_return_via_sysret+0x12/0x7f
This happens when we attach BPF program to perf_event with:
struct perf_event_attr attr = {
.type = PERF_TYPE_HARDWARE,
.config = PERF_COUNT_HW_CPU_CYCLES,
.precise_ip = 2,
.sample_type = PERF_SAMPLE_IP | PERF_SAMPLE_BRANCH_STACK,
.branch_sample_type = PERF_SAMPLE_BRANCH_USER |
PERF_SAMPLE_BRANCH_NO_FLAGS |
PERF_SAMPLE_BRANCH_NO_CYCLES |
PERF_SAMPLE_BRANCH_CALL_STACK,
.sample_period = 50,
.size = sizeof(struct perf_event_attr),
};
and calls bpf_get_stackid() from the BPF program. I pushed a reproducer to
https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/log/?h=lbr_issue
under tools/bpf/ministrobe/. It requires latest CLANG to build. I also included
the binary that should just run on CentOS 7.
This warning is usually harmless, but it may also cause double fault #DF. Here
are
some analysis from Tejun:
"""
The exact pathway is still under investigation but it triggered a sanity
warning in
the kernel backtrace acquisition code looking like the following:
[ 2051.040745] WARNING: stack recursion on stack type 1
This is from `arch/x86/kernel/dumpstack_64.c::get_stack_info()` and is warning
that
while walking the stack to obtain backtrace it encountered the same type of
stack
more than once. The warning in itself isn't critical. It's a sanity check and
when
the check fails, it just stops walking the stack and it is likely that the
warning
is being triggered spuriously given that more machines which trigger these
warnings
continue to run fine than not, which is unlikely with actual stack corruptions.
A
possibility is that stack acquisition is happening from a context that the code
can't handle.
However, on some machines, this caused #DF (double fault) and thus immediate
crash.
Backtracing a DF'd kernel is a bit cumbersome, so the following contains
spurious
entries, but it does show what happened:
PID: 80430 TASK: 888d92c62a80 CPU: 24 COMMAND: "25_scheduler"
#0 [fe4cfd88] machine_kexec at 8104a646
#1 [fe4cfdd8] __crash_kexec at 8114a82f
#2 [fe4cfea0] panic at 810ba99c
#3 [fe4cff20] df_debug at 8104e21d
#4 [fe4cff30] do_double_fault at 8101c9b4
#5 [fe4cff50] double_fault at 81c00b3e
[exception RIP: vsnprintf+14]
RIP: 81a3ee6e RSP: fe4d0ff8 RFLAGS: 00010082
RAX: fe4d1070 RBX: fe4d1101 RCX: fe4d1050
RDX: 822c42f5 RSI: 7fff RDI: fe4d111a
RBP: fe4d10a0 R8: 0067 R9: 82209d05
R10: 822a5fd0 R11: 822a6358 R12: 0019
R13: 0001 R14: 0019 R15: 822b5fd6
ORIG_RAX: CS: 0010 SS: 0018
--- ---
#6 [fe4d0ff8] vsnprintf at 81a3ee6e
bt: cannot transition from exception stack to current process stack:
exception stack pointer: fe4cfd88
process stack pointer: fe4d1048
current stack base: c900241a
0xfe4d1040 kallsyms_token_index
0xfe4d1048 sprintf
0xfe4d1078 kallsyms_lookup
0xfe4d1098 kallsyms_names
0xfe4d10a8 __sprint_symbol
0xfe4d10d8 textbuf.47672
0xfe4d10e0 always_kmsg_dump
0xfe4d10f8 symbol_string
0xfe4d11c0 textbuf.47672
0xfe4d11e8 textbuf.47672
0xfe4d11f8 textbuf.47672
0xfe4d1200 always_kmsg_dump
0xfe4d1210 kallsyms_token_index
0xfe4d1218 vsnprintf
0xfe4d1220 textbuf.47672
0xfe4d1258 kallsyms_token_index
0xfe4d1270 vscnprintf
0xfe4d1280 vprintk_store
0xfe4d1290 wake_up_klogd
0xfe4d12b0 kallsyms_token_index
0xfe4d12c8 vprintk_emit
0xfe4d1300 entry_SYSCALL_64
0xfe4d1318 vprintk_deferred
0xfe4d1328 printk_deferred
0xfe4d1360 entry_SYSCALL_64
0xfe4d1380 __start_orc_unwind
0xfe4d1388 unwind_next_frame.cold.7
0xfe4d13c8 perf_callchain_kernel
0xfe4d1418 entry_SYSCALL_64
0xfe4d1450 get_perf_callchain
0xfe4d14b0 bpf_get_stack
0xfe4d1730 bpf_overflow_handler
0xfe4d1788 __perf_event_overflow
0xfe4d17a0 x86_pmu
0xfe4d17b8 __intel_pmu_pebs_event
0xfe4d17e0 setup_pebs_fixed_sample_data
0xfe4d1890 entry_SYSCALL_64
0xfe4d1ab8 intel_pmu_drain_pebs_nhm
0xfe4d1ac0 setup_pebs_fixed_sample_data
0xfe4d1bb8 handle_pmi_common
0xfe4d1d00 insn_get_sib.part.5
0xfe4d1d10 insn_get_displacement.part.6
0xfe4d1d20
Re: [PATCH v3 2/4] dt-bindings: nvmem: Add properties needed for blowing fuses
Hi,
On Thu, Jun 18, 2020 at 9:55 AM Stephen Boyd wrote:
>
> Quoting Doug Anderson (2020-06-18 08:32:20)
> > Hi,
> >
> > On Thu, Jun 18, 2020 at 7:01 AM Srinivas Kandagatla
> > >
> > > On the other note:
> > >
> > > clock-names are not mandatory according to
> > > Documentation/devicetree/bindings/clock/clock-bindings.txt
> > >
> > > For this particular case where clock-names = "sec" is totally used for
> > > indexing and nothing else!
> >
> > So I guess in the one-clock case it's more optional and if you feel
> > strongly I'll get rid of clk-names here. ...but if we ever need
> > another clock we probably will want to add it back and (I could be
> > corrected) I believe it's convention to specify clk-names even with
> > one clock.
>
> TL;DR: I suggest you call this "core" if you want to keep the
> clock-name, or just drop it if there's only one clk and move on.
Ah, true. "core" sounds good.
> It's not required to have clock-names with one clk, and indeed it's not
> required to have clock-names at all. The multi clk scenario is a little
> more difficult to handle because historically the clk_get() API has been
> name based and not index based like platform resources. When there is
> one clk the driver can pass NULL as the 'con_id' argument to clk_get()
> and it will do the right thing. And when you have more than one clk you
> can pass NULL still and get the first clk, that should be in the same
> index, and then other clks by name.
>
> So far nobody has added clk_get_by_index() but I suppose if it was
> important the API could be added. Working with only legacy clkdev
> lookups would fail of course, but clock-names could be fully deprecated
> and kernel images may be smaller because we're not storing piles of
> strings and doing string comparisons. Given that it's been this way for
> a long time and we have DT schema checking it doesn't seem very
> important to mandate anything one way or the other though. I certainly
> don't feel good when I see of_clk_*() APIs being used by platform
> drivers, but sometimes it is required.
>
> To really put this into perspective, consider the fact that most drivers
> have code that figures out what clk names to look for and then they pile
> them into arrays and just turn them all on and off together. Providing
> fine grained clk control here is a gigantic waste of time, and requiring
> clock-names is just more hoops that driver authors feel they have to
> jump through for $reasons. We have clk_bulk_get_all() for this, but that
> doesn't solve the one rate changing clk among the sea of clk gates
> problem. In general, driver authors don't care and we should probably be
> providing a richer while simpler API to them that manages power state of
> some handful of clks, regulators, and power domains for a device while
> also letting them control various knobs like clk rate when necessary.
>
> BTW, on qcom platforms they usually name clks "core" and "iface" for the
> core clk and the interface clk used to access the registers of a device.
> Sometimes there are esoteric ones like "axi". In theory this cuts down
> on the number of strings the kernel keeps around but I like that it
> helps provide continuity across drivers and DTs for their SoCs. If you
> ask the hardware engineer what the clk name is for the hardware block
> they'll tell you the globally unique clk name like
> "gcc_qupv3_uart9_core_clk", which is the worst name to use.
OK, sounds about what I expected. I suppose the path of least
resistance would be to just drop clock-names. I guess I'm just
worried that down the road someone will want to specify the "iface"
clock too. If that ever happens, we're stuck with these options:
1. Be the first ones to require adding clk_get_by_index().
2. Use the frowned upon of_clk_get() API which allows getting by index.
3. Get the first clock with clk_get(NULL) and the second clock with
clk_get("iface") and figure out how to specify this happily in the
yaml.
If we just define clock-names now then we pretty much match the
pattern of everyone else.
Srinivas: reading all that if you still want me to drop clock-names
then I will. I'll use clk_get(NULL) to get the clock and if/when we
ever need an "iface" clock (maybe we never will?) we can figure it out
then.
-Doug
Re: [PATCH 4/4] pci: export untrusted attribute in sysfs
Thanks Greg and Andy for your continued inputs, and thanks Ashok for chiming in.
On Thu, Jun 18, 2020 at 9:23 AM Raj, Ashok wrote:
>
> Hi Greg,
>
>
> On Thu, Jun 18, 2020 at 06:02:12PM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Jun 18, 2020 at 08:03:49AM -0700, Rajat Jain wrote:
> > > Hello,
> > >
> > > On Thu, Jun 18, 2020 at 2:14 AM Andy Shevchenko
> > > wrote:
> > > >
> > > > On Thu, Jun 18, 2020 at 11:36 AM Greg Kroah-Hartman
> > > > wrote:
> > > > >
> > > > > On Thu, Jun 18, 2020 at 11:12:56AM +0300, Andy Shevchenko wrote:
> > > > > > On Wed, Jun 17, 2020 at 10:56 PM Rajat Jain
> > > > > > wrote:
> > > > > > > On Wed, Jun 17, 2020 at 12:31 AM Christoph Hellwig
> > > > > > > wrote:
> > > > > >
> > > > > > ...
> > > > > >
> > > > > > > (and likely call it "external" instead of "untrusted".
> > > > > >
> > > > > > Which is not okay. 'External' to what? 'untrusted' has been
> > > > > > carefully
> > > > > > chosen by the meaning of it.
> > > > > > What external does mean for M.2. WWAN card in my laptop? It's in
> > > > > > ACPI
> > > > > > tables, but I can replace it.
> > > > >
> > > > > Then your ACPI tables should show this, there is an attribute for it,
> > > > > right?
> > > >
> > > > There is a _PLD() method, but it's for the USB devices (or optional
> > > > for others, I don't remember by heart). So, most of the ACPI tables,
> > > > alas, don't show this.
> > > >
> > > > > > This is only one example. Or if firmware of some device is altered,
> > > > > > and it's internal (whatever it means) is it trusted or not?
> > > > >
> > > > > That is what people are using policy for today, if you object to this,
> > > > > please bring it up to those developers :)
> > > >
> > > > > > So, please leave it as is (I mean name).
> > > > >
> > > > > firmware today exports this attribute, why do you not want userspace
> > > > > to
> > > > > also know it?
> > >
> > > To clarify, the attribute exposed by the firmware today is
> > > "ExternalFacingPort" and "external-facing" respectively:
> > >
> > > 617654aae50e ("PCI / ACPI: Identify untrusted PCI devices")
> > > 9cb30a71ac45d("PCI: OF: Support "external-facing" property")
> > >
> > > The kernel flag was named "untrusted" though, hence the assumption
> > > that "external=untrusted" is currently baked into the kernel today.
> > > IMHO, using "external" would fix that (The assumption can thus be
> > > contained in the IOMMU drivers) and at the same time allow more use of
> > > this attribute.
> > >
> > > > >
> > > > > Trust is different, yes, don't get the two mixed up please. That
> > > > > should
> > > > > be a different sysfs attribute for obvious reasons.
> > > >
> > > > Yes, as a bottom line that's what I meant as well.
> > >
> > > So what is the consensus here? I don't have a strong opinion - but it
> > > seemed to me Greg is saying "external" and Andy is saying "untrusted"?
> >
> > Those two things are totally separate things when it comes to a device.
>
> Agree that these are two separate attributes, and better not mixed.
+1.
>
> >
> > One (external) describes the location of the device in the system.
> >
> > The other (untrusted) describes what you want the kernel to do with this
> > device (trust or not trust it).
> >
> > One you can change (from trust to untrusted or back), the other you can
> > not, it is a fixed read-only property that describes the hardware device
> > as defined by the firmware.
Correct. I believe what is being described by the firmware is a fixed
read-only property describing the location of the device ("external")
- not what to do with it ("untrusted").
>
> The genesis is due to lack of a mechanism to establish if the device
> is trusted or not was the due lack of some specs and implementation around
> Component Measurement And Authentication (CMA). Treating external as
> untrusted was the best first effort. i.e trust internal
> devices and don't trust external devices for enabling ATS.
>
> But that said external is just describing topology, and if Linux wants to
> use that in the policy that's different. Some day external device may also
> use CMA to estabilish trust. FWIW even internal devices aren't trust
> worthy, except maybe RCIEP's.
Correct. Since the firmware is actually describing the unchangeable
topology (and not the policy), the takeaway I am taking from this
discussion is that the flag should be called "external".
Like I said, I don't have any hard opinions on this. So if you feel
that my conclusion is wrong and consensus was the other way around
("untrusted"), let me know and I'll be happy to change this.
Thanks,
Rajat
>
> >
> > Depending on the policy you wish to define, you can use the location of
> > the device to determine if you want to trust the device or not.
> >
>
> Cheers,
> Ashok
Re: [PATCH v8 9/9] firmware: arm_scmi: Add Base notifications support
On Mon, Jun 08, 2020 at 06:02:39PM +0100, Sudeep Holla wrote:
> On Wed, May 20, 2020 at 09:11:18AM +0100, Cristian Marussi wrote:
> > Make SCMI Base protocol register with the notification core.
> >
> > Reviewed-by: Jonathan Cameron
> > Signed-off-by: Cristian Marussi
> > ---
> > V6 --> V7
> > - fixed report.timestamp type
> > - fix max_payld_sz initialization
> > - fix report layout and initialization
> > - expose SCMI_EVENT_ in linux/scmi_protocol.h
> > V5 --> V6
> > - added handle argument to fill_custom_report()
> > V4 --> V5
> > - fixed unsual return construct
> > V3 --> V4
> > - scmi_event field renamed
> > V2 --> V3
> > - added handle awareness
> > V1 --> V2
> > - simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
> > logic out of protocol. ALL_SRCIDs logic is now in charge of the
> > notification core, together with proper reference counting of enables
> > - switched to devres protocol-registration
> > ---
> > drivers/firmware/arm_scmi/base.c | 118 +--
> > include/linux/scmi_protocol.h| 9 +++
> > 2 files changed, 123 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/firmware/arm_scmi/base.c
> > b/drivers/firmware/arm_scmi/base.c
> > index ce7d9203e41b..dcb098d8d823 100644
> > --- a/drivers/firmware/arm_scmi/base.c
> > +++ b/drivers/firmware/arm_scmi/base.c
> > @@ -5,7 +5,13 @@
> > * Copyright (C) 2018 ARM Ltd.
> > */
> >
> > +#include
> > +
> > #include "common.h"
> > +#include "notify.h"
> > +
> > +#define SCMI_BASE_NUM_SOURCES 1
> > +#define SCMI_BASE_MAX_CMD_ERR_COUNT1024
> >
>
> I am not sure of this, see below.
>
Ok
> > enum scmi_base_protocol_cmd {
> > BASE_DISCOVER_VENDOR = 0x3,
> > @@ -19,16 +25,25 @@ enum scmi_base_protocol_cmd {
> > BASE_RESET_AGENT_CONFIGURATION = 0xb,
> > };
> >
> > -enum scmi_base_protocol_notify {
> > - BASE_ERROR_EVENT = 0x0,
> > -};
> > -
> > struct scmi_msg_resp_base_attributes {
> > u8 num_protocols;
> > u8 num_agents;
> > __le16 reserved;
> > };
> >
> > +struct scmi_msg_base_error_notify {
> > + __le32 event_control;
> > +#define BASE_TP_NOTIFY_ALL BIT(0)
> > +};
> > +
> > +struct scmi_base_error_notify_payld {
> > + __le32 agent_id;
> > + __le32 error_status;
> > +#define IS_FATAL_ERROR(x) ((x) & BIT(31))
> > +#define ERROR_CMD_COUNT(x) FIELD_GET(GENMASK(9, 0), (x))
> > + __le64 msg_reports[SCMI_BASE_MAX_CMD_ERR_COUNT];
>
> This entire payload needs to fit in shmem and having huge shmem just
> for this sounds not so good to me. If this can be large, it needs to
> be iterated multiple times to get the full log.
>
I think it's a good point, if a platform implementation would generate
such jumbo payloads for this events no matter what the actual trasport
message size was, we're definitely going to receive corrupted packets.
Spec says about BASE_ERROR_EVENT:
Bits[9:0] Command count, number of commands in the
command list. A value of zero is possible if the
error cannot be attributed.
I'll ask Souvik if it's not the case to amend the spec to highlight this
possibility. (being errors notifications I suppose platform could simply
chunk the big packet in multiple pieces to fit into the current transport)
Anyway reviewing this implementation this payld struct here is defined as
to be big enough to contain the maximum allowed size by the current spec,
it is not that I am expecting to strictly receive packets sized exacly as
that; it's the same appproach I use thorughout the notifications: I reserve
an area big enough to hold any possible packet (.max_payld_sz) and then I
just check that I received a packets that fits (sizeof(*p) < payld_sz), in
case I received a variable length payload event packet as this, or check,
for other fixed-size payload events, that the received packet is of the
exact specified length.
It's just that I'm trying to pre-allocate spare areas that can fit any
possible packet length (if variable) for any possible transport.
> > +};
> > +
> > /**
> > * scmi_base_attributes_get() - gets the implementation details
> > * that are associated with the base protocol.
> > @@ -222,6 +237,95 @@ static int scmi_base_discover_agent_get(const struct
> > scmi_handle *handle,
> > return ret;
> > }
> >
> > +static int scmi_base_error_notify(const struct scmi_handle *handle, bool
> > enable)
> > +{
> > + int ret;
> > + u32 evt_cntl = enable ? BASE_TP_NOTIFY_ALL : 0;
> > + struct scmi_xfer *t;
> > + struct scmi_msg_base_error_notify *cfg;
> > +
> > + ret = scmi_xfer_get_init(handle, BASE_NOTIFY_ERRORS,
> > +SCMI_PROTOCOL_BASE, sizeof(*cfg), 0, );
> > + if (ret)
> > + return ret;
> > +
> > + cfg = t->tx.buf;
> > + cfg->event_control = cpu_to_le32(evt_cntl);
> > +
> > + ret = scmi_do_xfer(handle, t);
> > +
> > + scmi_xfer_put(handle, t);
> > + return ret;
> > +}
> > +
> > +static bool scmi_base_set_notify_enabled(const
Re: [PATCH 27/29] docs: dt: minor adjustments at writing-schema.rst
On Mon, Jun 15, 2020 at 08:47:06AM +0200, Mauro Carvalho Chehab wrote: > There are two literal blocks that aren't mark as such. Mark them, > in order to make the document to produce a better html output. > > While here, also add a SPDX header to it. > > Signed-off-by: Mauro Carvalho Chehab > --- > Documentation/devicetree/writing-schema.rst | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) Applied, thanks. Rob
[PATCH v4] xfs: Fix false positive lockdep warning with sb_internal & fs_reclaim
Depending on the workloads, the following circular locking dependency
warning between sb_internal (a percpu rwsem) and fs_reclaim (a pseudo
lock) may show up:
==
WARNING: possible circular locking dependency detected
5.0.0-rc1+ #60 Tainted: GW
--
fsfreeze/4346 is trying to acquire lock:
26f1d784 (fs_reclaim){+.+.}, at:
fs_reclaim_acquire.part.19+0x5/0x30
but task is already holding lock:
72bfc54b (sb_internal){}, at: percpu_down_write+0xb4/0x650
which lock already depends on the new lock.
:
Possible unsafe locking scenario:
CPU0CPU1
lock(sb_internal);
lock(fs_reclaim);
lock(sb_internal);
lock(fs_reclaim);
*** DEADLOCK ***
4 locks held by fsfreeze/4346:
#0: b478ef56 (sb_writers#8){}, at: percpu_down_write+0xb4/0x650
#1: 1ec487a9 (>s_umount_key#28){}, at:
freeze_super+0xda/0x290
#2: 3edbd5a0 (sb_pagefaults){}, at: percpu_down_write+0xb4/0x650
#3: 72bfc54b (sb_internal){}, at: percpu_down_write+0xb4/0x650
stack backtrace:
Call Trace:
dump_stack+0xe0/0x19a
print_circular_bug.isra.10.cold.34+0x2f4/0x435
check_prev_add.constprop.19+0xca1/0x15f0
validate_chain.isra.14+0x11af/0x3b50
__lock_acquire+0x728/0x1200
lock_acquire+0x269/0x5a0
fs_reclaim_acquire.part.19+0x29/0x30
fs_reclaim_acquire+0x19/0x20
kmem_cache_alloc+0x3e/0x3f0
kmem_zone_alloc+0x79/0x150
xfs_trans_alloc+0xfa/0x9d0
xfs_sync_sb+0x86/0x170
xfs_log_sbcount+0x10f/0x140
xfs_quiesce_attr+0x134/0x270
xfs_fs_freeze+0x4a/0x70
freeze_super+0x1af/0x290
do_vfs_ioctl+0xedc/0x16c0
ksys_ioctl+0x41/0x80
__x64_sys_ioctl+0x73/0xa9
do_syscall_64+0x18f/0xd23
entry_SYSCALL_64_after_hwframe+0x49/0xbe
This is a false positive as all the dirty pages are flushed out before
the filesystem can be frozen.
One way to avoid this splat is to add GFP_NOFS to the affected allocation
calls. This is what PF_MEMALLOC_NOFS per-process flag is for. This does
reduce the potential source of memory where reclaim can be done. This
shouldn't matter unless the system is really running out of memory.
In that particular case, the filesystem freeze operation may fail while
it was succeeding previously.
Without this patch, the command sequence below will show that the lock
dependency chain sb_internal -> fs_reclaim exists.
# fsfreeze -f /home
# fsfreeze --unfreeze /home
# grep -i fs_reclaim -C 3 /proc/lockdep_chains | grep -C 5 sb_internal
After applying the patch, such sb_internal -> fs_reclaim lock dependency
chain can no longer be found. Because of that, the locking dependency
warning will not be shown.
Suggested-by: Dave Chinner
Signed-off-by: Waiman Long
---
fs/xfs/xfs_super.c | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 379cbff438bc..1b94b9bfa4d7 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -913,11 +913,33 @@ xfs_fs_freeze(
struct super_block *sb)
{
struct xfs_mount*mp = XFS_M(sb);
+ unsigned long pflags;
+ int ret;
+ /*
+* A fs_reclaim pseudo lock is added to check for potential deadlock
+* condition with fs reclaim. The following lockdep splat was hit
+* occasionally. This is actually a false positive as the allocation
+* is being done only after the frozen filesystem is no longer dirty.
+* One way to avoid this splat is to add GFP_NOFS to the affected
+* allocation calls. This is what PF_MEMALLOC_NOFS is for.
+*
+* CPU0CPU1
+*
+* lock(sb_internal);
+* lock(fs_reclaim);
+* lock(sb_internal);
+* lock(fs_reclaim);
+*
+* *** DEADLOCK ***
+*/
+ current_set_flags_nested(, PF_MEMALLOC_NOFS);
xfs_stop_block_reaping(mp);
xfs_save_resvblks(mp);
xfs_quiesce_attr(mp);
- return xfs_sync_sb(mp, true);
+ ret = xfs_sync_sb(mp, true);
+ current_restore_flags_nested(, PF_MEMALLOC_NOFS);
+ return ret;
}
STATIC int
--
2.18.1
Re: [PATCH 15/29] dt: fix reference to olpc,xo1.75-ec.txt
On Mon, 15 Jun 2020 08:46:54 +0200, Mauro Carvalho Chehab wrote:
> This file was converted and renamed.
>
> Fixes: 7882d822b3f9 ("dt-bindings: spi: Convert spi-pxa2xx to json-schema")
> Signed-off-by: Mauro Carvalho Chehab
> ---
> Documentation/devicetree/bindings/misc/olpc,xo1.75-ec.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Applied, thanks!
Re: [PATCH v3 05/15] arm64: kvm: Build hyp-entry.S separately for VHE/nVHE
Hi David,
On 2020-06-18 13:25, David Brazdil wrote:
This patch is part of a series which builds KVM's non-VHE hyp code
separately
from VHE and the rest of the kernel.
The above comment doesn't really belong here, and us only fit for the
cover letter.
hyp-entry.S contains implementation of KVM hyp vectors. This code is
mostly
shared between VHE/nVHE, therefore compile it under both VHE and nVHE
build
rules. nVHE-specific host HVC handler is hidden behind
__KVM_NVHE_HYPERVISOR__.
Adjust code which selects which KVM hyp vecs to install to choose the
correct
VHE/nVHE symbol.
Signed-off-by: David Brazdil
---
arch/arm64/include/asm/kvm_asm.h | 7 ++-
arch/arm64/include/asm/kvm_mmu.h | 16 ++--
arch/arm64/include/asm/mmu.h | 7 ---
arch/arm64/kernel/cpu_errata.c | 4 +++-
arch/arm64/kernel/image-vars.h | 12
arch/arm64/kvm/hyp/hyp-entry.S | 2 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/va_layout.c | 2 +-
8 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_asm.h
b/arch/arm64/include/asm/kvm_asm.h
index 6a682d66a640..2baa69324cc9 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -76,7 +76,12 @@ struct kvm_vcpu;
extern char __kvm_hyp_init[];
extern char __kvm_hyp_init_end[];
-extern char __kvm_hyp_vector[];
+DECLARE_KVM_HYP_SYM(__kvm_hyp_vector);
+
+#ifdef CONFIG_KVM_INDIRECT_VECTORS
+DECLARE_KVM_HYP_SYM(__bp_harden_hyp_vecs);
+extern atomic_t arm64_el2_vector_last_slot;
+#endif
extern void __kvm_flush_vm_context(void);
extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t
ipa);
diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index b12bfc1f051a..5bfc7ee61997 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -461,11 +461,15 @@ extern int __kvm_harden_el2_vector_slot;
static inline void *kvm_get_hyp_vector(void)
{
struct bp_hardening_data *data = arm64_get_bp_hardening_data();
- void *vect = kern_hyp_va(kvm_ksym_ref(__kvm_hyp_vector));
int slot = -1;
+ void *vect = kern_hyp_va(has_vhe()
+ ? kvm_ksym_ref(__kvm_hyp_vector)
+ : kvm_ksym_ref_nvhe(__kvm_hyp_vector));
If you are introducing has_vhe() at this stage, then you might as well
not apply kernel_hyp_va() to the address. This also make the declaration
block look a bit ugly (yes, I'm a bit of a maniac). I'd rather see
something like:
diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index 5bfc7ee61997..e915c47744bc 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -457,19 +457,25 @@ static inline int kvm_write_guest_lock(struct kvm
*kvm, gpa_t gpa,
extern void *__kvm_bp_vect_base;
extern int __kvm_harden_el2_vector_slot;
+#define get_hyp_vector_address(v) \
+({ \
+ void *__v; \
+ if (has_vhe()) \
+ __v = kvm_ksym_ref(v); \
+ else\
+ __v = kern_hyp_va(kvm_ksym_ref_nvhe(v));\
+ __v;\
+})
+
/* This is called on both VHE and !VHE systems */
static inline void *kvm_get_hyp_vector(void)
{
struct bp_hardening_data *data = arm64_get_bp_hardening_data();
+ void *vect = get_hyp_vector_address(__kvm_hyp_vector);
int slot = -1;
- void *vect = kern_hyp_va(has_vhe()
- ? kvm_ksym_ref(__kvm_hyp_vector)
- : kvm_ksym_ref_nvhe(__kvm_hyp_vector));
if (cpus_have_const_cap(ARM64_HARDEN_BRANCH_PREDICTOR) && data->fn) {
- vect = kern_hyp_va(has_vhe()
- ? kvm_ksym_ref(__bp_harden_hyp_vecs)
- : kvm_ksym_ref_nvhe(__bp_harden_hyp_vecs));
+ vect = get_hyp_vector_address(__bp_harden_hyp_vecs);
slot = data->hyp_vectors_slot;
}
if (cpus_have_const_cap(ARM64_HARDEN_BRANCH_PREDICTOR) && data->fn) {
- vect = kern_hyp_va(kvm_ksym_ref(__bp_harden_hyp_vecs));
+ vect = kern_hyp_va(has_vhe()
+ ? kvm_ksym_ref(__bp_harden_hyp_vecs)
+ : kvm_ksym_ref_nvhe(__bp_harden_hyp_vecs));
slot = data->hyp_vectors_slot;
}
@@ -494,12 +498,11 @@ static inline int kvm_map_vectors(void)
* HBP + HEL2 -> use hardened vertors and use exec mapping
*/
if (cpus_have_const_cap(ARM64_HARDEN_BRANCH_PREDICTOR)) {
- __kvm_bp_vect_base = kvm_ksym_ref(__bp_harden_hyp_vecs);
- __kvm_bp_vect_base = kern_hyp_va(__kvm_bp_vect_base);
+ __kvm_bp_vect_base =
Re: [PATCH 07/25] mm/csky: Use mm_fault_accounting()
On Thu, Jun 18, 2020 at 7:38 AM Peter Xu wrote:
>
> GUP needs the per-task accounting, but not the perf events. We can do that by
> slightly changing the new approach into:
>
> bool major = (ret & VM_FAULT_MAJOR) || (flags & FAULT_FLAG_TRIED);
>
> if (major)
> current->maj_flt++;
> else
> current->min_flt++;
>
> if (!regs)
> return ret;
>
> if (major)
> perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MAJ, 1, regs,
> address);
> else
> perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MIN, 1, regs,
> address);
Ack, I think this is the right thing to do.
No normal situation will ever notice the difference, with remote
accesses being as rare and specialized as they are. But being able to
remote the otherwise unused 'tsk' parameter sounds like the right
thing to do too.
It might be worth adding a comment about why.
Also, honestly, how about we remove the 'major' variable entirely, and
instead make the code be something like
unsigned long *flt;
int event_type;
...
/* Major fault */
if ((ret & VM_FAULT_MAJOR) || (flags & FAULT_FLAG_TRIED)) {
flt = >maj_flt;
event_type = PERF_COUNT_SW_PAGE_FAULTS_MAJ;
} else {
flt = >min_flt;
event_type = PERF_COUNT_SW_PAGE_FAULTS_MIN;
}
*flt++;
if (regs)
perf_sw_event(event_type, 1, regs, address);
instead. Less source code duplication, and I bet it improves code
generation too.
Linus
Re: [PATCH v2] Revert "zram: convert remaining CLASS_ATTR() to CLASS_ATTR_RO()"
On 6/17/20 1:49 PM, Greg Kroah-Hartman wrote:
From: Wade Mealing
Turns out that the permissions for 0400 really are what we want here,
otherwise any user can read from this file.
[fixed formatting, added changelog, and made attribute static - gregkh]
Reported-by: Wade Mealing
Cc: stable
Fixes: f40609d1591f ("zram: convert remaining CLASS_ATTR() to CLASS_ATTR_RO()")
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1847832
Signed-off-by: Greg Kroah-Hartman
Reviewed-by: Steffen Maier
---
v2: fix read/write confusion in the changelog, thanks to Steffen for the
review.
was more specific as to the changes I made to the original patch.
drivers/block/zram/zram_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 6e2ad90b17a3..270dd810be54 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2021,7 +2021,8 @@ static ssize_t hot_add_show(struct class *class,
return ret;
return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
}
-static CLASS_ATTR_RO(hot_add);
+static struct class_attribute class_attr_hot_add =
+ __ATTR(hot_add, 0400, hot_add_show, NULL);
static ssize_t hot_remove_store(struct class *class,
struct class_attribute *attr,
--
Mit freundlichen Gruessen / Kind regards
Steffen Maier
Linux on IBM Z Development
https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
On Thu, Jun 18, 2020 at 04:32:40PM +0200, Niklas Cassel wrote: > If, for some reason, we want to allow builds with gcc < 4.6.0 > even though the minimum gcc version is now 4.8.0, Just one thing to watch out: the stable trees are still using older version of gcc. Note sure how relevant this is though.
Re: [PATCH] usbip: tools: add in man page how to load the client's module
On 6/17/20 6:08 PM, Antonio Borneo wrote: While the man page usbipd.8 already informs the user on which kernel module has to be used on server side, the man page usbip.8 does not provide any equivalent information on client side. Also, it could be hard for a newby to identify the proper usbip client kernel module, due to the name "vhci-hcd" that has no immediate assonance with usbip. Add in usbip.8 the command to add the module vhci-hcd, similarly as it's already present in usbipd.8 for usbip-host. Signed-off-by: Antonio Borneo --- tools/usb/usbip/doc/usbip.8 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/usb/usbip/doc/usbip.8 b/tools/usb/usbip/doc/usbip.8 index a6097be25d28..81313f4913b4 100644 --- a/tools/usb/usbip/doc/usbip.8 +++ b/tools/usb/usbip/doc/usbip.8 @@ -85,6 +85,8 @@ List local USB devices. client:# usbip list --remote=server - List exportable usb devices on the server. While you are making changes, please change the above to the following. This is more accurate. List devices exported by remote=server +client:# modprobe vhci-hcd + client:# usbip attach --remote=server --busid=1-2 - Connect the remote USB device. thanks, -- Shuah
Re: New mode DM-Verity error handling
On Thu, Jun 18 2020 at 12:50pm -0400, Sami Tolvanen wrote: > On Thu, Jun 18, 2020 at 11:44:45AM -0400, Mike Snitzer wrote: > > I do not accept that panicing the system because of verity failure is > > reasonable. > > > > In fact, even rebooting (via DM_VERITY_MODE_RESTART) looks very wrong. > > > > The device should be put in a failed state and left for admin recovery. > > That's exactly how the restart mode works on some Android devices. The > bootloader sees the verification error and puts the device in recovery > mode. Using the restart mode on systems without firmware support won't > make sense, obviously. OK, so I need further justification from Samsung why they are asking for this panic mode. Thanks, Mike
Re: [PATCH 14/29] dt: Fix broken references to renamed docs
On Mon, 15 Jun 2020 08:46:53 +0200, Mauro Carvalho Chehab wrote: > Some files got renamed. Those were all fixed automatically by > > ./scripts/documentation-file-ref-check --fix > > Signed-off-by: Mauro Carvalho Chehab > --- > Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt | 2 +- > Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt | 4 ++-- > Documentation/devicetree/bindings/display/imx/ldb.txt | 4 ++-- > Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt | 2 +- > MAINTAINERS | 4 ++-- > 5 files changed, 8 insertions(+), 8 deletions(-) > Applied, thanks!
Re: [PATCH v3 04/15] arm64: kvm: Handle calls to prefixed hyp functions
Hi David, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.8-rc1 next-20200618] [cannot apply to kvmarm/next arm64/for-next/core arm-perf/for-next/perf] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/David-Brazdil/Split-off-nVHE-hyp-code/20200618-203230 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1b5044021070efa3259f3e9548dc35d1eb6aa844 config: arm64-allyesconfig (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>, old ones prefixed by <<): In file included from include/linux/kvm_host.h:36, from arch/arm64/kvm/arm.c:11: arch/arm64/kvm/arm.c: In function 'kvm_arch_vcpu_ioctl_run': >> arch/arm64/include/asm/kvm_host.h:460:26: warning: variable 'ret' set but >> not used [-Wunused-but-set-variable] 460 | typeof(f(__VA_ARGS__)) ret; | ^~~ >> arch/arm64/include/asm/kvm_host.h:488:10: note: in expansion of macro >> 'kvm_call_hyp_nvhe_ret' 488 |ret = kvm_call_hyp_nvhe_ret(f, ##__VA_ARGS__); | ^ >> arch/arm64/kvm/arm.c:754:10: note: in expansion of macro 'kvm_call_hyp_ret' 754 |ret = kvm_call_hyp_ret(__kvm_vcpu_run_nvhe, vcpu); | ^~~~ -- In file included from arch/arm64/include/asm/percpu.h:228, from arch/arm64/include/asm/smp.h:28, from include/linux/smp.h:89, from include/linux/percpu.h:7, from include/linux/context_tracking_state.h:5, from include/linux/hardirq.h:5, from include/linux/kvm_host.h:7, from arch/arm64/kvm/debug.c:9: arch/arm64/kvm/debug.c: In function 'kvm_arm_init_debug': >> arch/arm64/include/asm/kvm_host.h:460:26: warning: variable 'ret' set but >> not used [-Wunused-but-set-variable] 460 | typeof(f(__VA_ARGS__)) ret; | ^~~ include/asm-generic/percpu.h:72:26: note: in definition of macro 'raw_cpu_generic_to_op' 72 | *raw_cpu_ptr(&(pcp)) op val; | ^~~ include/linux/percpu-defs.h:377:11: note: in expansion of macro 'raw_cpu_write_1' 377 | case 1: stem##1(variable, __VA_ARGS__);break; | ^~~~ include/linux/percpu-defs.h:421:34: note: in expansion of macro '__pcpu_size_call' 421 | #define raw_cpu_write(pcp, val) __pcpu_size_call(raw_cpu_write_, pcp, val) | ^~~~ include/linux/percpu-defs.h:452:2: note: in expansion of macro 'raw_cpu_write' 452 | raw_cpu_write(pcp, val); | ^ >> arch/arm64/kvm/debug.c:68:2: note: in expansion of macro '__this_cpu_write' 68 | __this_cpu_write(mdcr_el2, kvm_call_hyp_ret(__kvm_get_mdcr_el2)); | ^~~~ >> arch/arm64/include/asm/kvm_host.h:488:10: note: in expansion of macro >> 'kvm_call_hyp_nvhe_ret' 488 |ret = kvm_call_hyp_nvhe_ret(f, ##__VA_ARGS__); | ^ >> arch/arm64/kvm/debug.c:68:29: note: in expansion of macro 'kvm_call_hyp_ret' 68 | __this_cpu_write(mdcr_el2, kvm_call_hyp_ret(__kvm_get_mdcr_el2)); | ^~~~ >> arch/arm64/include/asm/kvm_host.h:460:26: warning: variable 'ret' set but >> not used [-Wunused-but-set-variable] 460 | typeof(f(__VA_ARGS__)) ret; | ^~~ include/asm-generic/percpu.h:72:26: note: in definition of macro 'raw_cpu_generic_to_op' 72 | *raw_cpu_ptr(&(pcp)) op val; | ^~~ include/linux/percpu-defs.h:378:11: note: in expansion of macro 'raw_cpu_write_2' 378 | case 2: stem##2(variable, __VA_ARGS__);break; | ^~~~ include/linux/percpu-defs.h:421:34: note: in expansion of macro '__pcpu_size_call' 421 | #define raw_cpu_write(pcp, val) __pcpu_size_call(raw_cpu_write_, pcp, val) | ^~~~ include/linux/percpu-defs.h:452:2: note: in expansion of macro 'raw_cpu_write' 452 | raw_cpu_write(pcp, val); | ^ >> arch/arm64/kvm/debug.c:68:2: note: in expansion of macro '__this_cpu_write' 68 | __this_cpu_write(mdcr_el2, kvm_call_hyp_ret(__kvm_get_mdcr_el2)); | ^~~~ >> arch/arm64/include/asm/kvm_host.h:488:10: note: in expansion of macro >> 'kvm_call_hyp_
Re: [PATCH v6 4/5] cpufreq: qcom: Update the bandwidth levels on frequency change
On Wed, Jun 17, 2020 at 10:13:21PM +0530, Sibi Sankar wrote:
> On 2020-06-17 03:41, Matthias Kaehlcke wrote:
> > Hi Sibi,
> >
> > after doing the review I noticed that Viresh replied on the cover letter
> > that he picked the series up for v5.9, so I'm not sure if it makes sense
> > to send a v7.
> >
> > On Wed, Jun 17, 2020 at 02:35:00AM +0530, Sibi Sankar wrote:
> >
> > > > > @@ -112,7 +178,7 @@ static int qcom_cpufreq_hw_read_lut(struct
> > > > > device *cpu_dev,
> > > > >
> > > > > if (freq != prev_freq && core_count != LUT_TURBO_IND) {
> > > > > table[i].frequency = freq;
> > > > > - dev_pm_opp_add(cpu_dev, freq * 1000, volt);
> > > > > + qcom_cpufreq_update_opp(cpu_dev, freq, volt);
> > > >
> > > > This is the cross-validation mentioned above, right? Shouldn't it
> > > > include
> > > > a check of the return value?
> > >
> > > Yes, this is the cross-validation step,
> > > we adjust the voltage if opp-tables are
> > > present/added successfully and enable
> > > them, else we would just do a add opp.
> > > We don't want to exit early on a single
> > > opp failure. We will error out a bit
> > > later if the opp-count ends up to be
> > > zero.
> >
> > At least an error/warning message would seem convenient when
> > adjusting/adding
> > an OPP fails, otherwise you would only notice by looking at the sysfs
> > attributes (if you'd even spot a single/few OPPs to be missing).
>
> I did consider the case where adjust
> voltage fails and we do report the
> freq for which it fails for as well.
> If adding a OPP fails we will still
> it being listed in the sysfs cpufreq
> scaling_available_frequencies since
> it lists the freq_table in khz there
> instead.
Ah, right, I missed that v6 added the error log to
qcom_cpufreq_update_opp(), please ignore my comment :)
Re: [PATCH v10 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe controller errors
On Thu, Jun 18, 2020 at 04:35:31PM +, Shiju Jose wrote:
> >-Original Message-
> >From: Andy Shevchenko [mailto:andriy.shevche...@linux.intel.com]
> >Sent: 18 June 2020 16:56
> >To: Shiju Jose
> >Cc: linux-a...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> >ker...@vger.kernel.org; r...@rjwysocki.net; helg...@kernel.org;
> >b...@alien8.de; james.mo...@arm.com; l...@kernel.org;
> >tony.l...@intel.com; dan.carpen...@oracle.com;
> >zhangligu...@linux.alibaba.com; Wangkefeng (OS Kernel Lab)
> >; jroe...@suse.de; Linuxarm
> >; yangyicong ; Jonathan
> >Cameron ; tanxiaofei
> >
> >Subject: Re: [PATCH v10 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe
> >controller errors
> >
> >On Thu, Jun 18, 2020 at 04:40:51PM +0100, Shiju Jose wrote:
...
> >> Reviewed-by: Andy Shevchenko
> >
> >Hmm... Did I give a tag?
Yes, and please, be sure that you got explicit tags from reviewers.
...
> >> + for_each_set_bit_from(idx, (const unsigned long *)>val_bits,
> >
> >Can't you make val_bits unsigned long? Because this casting is incorrect.
> >Otherwise, make a local copy into unsigned long variable.
>
> The data val_bits in the error record is 64 bits, thus used u64.
> Casting is added because of a compilation warning on _find_nex_bit_ function
> as it
> expects the type of the address as const unsigned long*.
> Probably I will make local copy of val_bits into unsigned long variable.
I see. So, something like this:
unsigned long bits[] = { BITMAP_FROM_U64(edata->val_bits) };
...
for_each_set_bit_from(i, bits, ...)
...
looks plausible. Or if you have better idea...
--
With Best Regards,
Andy Shevchenko
Re: [PATCH] arm: dts: am335x-pocketbeagle: add gpio-line-names
* Drew Fustini [200617 17:10]:
> Tony - does this look ok for 5.9?
Yes looks OK to me.
Just wondering, are the line with "NA" not used internally either?
If the "NA" lines are used internally, we should probably use
"Reserved" or "Internal" or something like that to avoid later
on having to patch them with internal device names..
> If so, I might start making other variants like BeagleBone Blue and
> BeagleBone {Green,Black} Wireless and submit those when ready.
OK yeah makes sense.
Regards,
Tony
Re: [PATCH] sparse: use identifiers to define address spaces
On Thu, Jun 18, 2020 at 3:06 AM kernel test robot wrote: > > I love your patch! Perhaps something to improve: The new warnings don't seem to be due to the kernel test robot having an old version of sparse, but just because the error strings changed, and presumably the kernel test robot has some "ignore old sparse warnings" logic. So the warnings all look new, even if they aren't. I'm planning on applying that patch soon, so this is just a heads-up that that will (again) cause the big number of warning string changes, but hopefully that will be a one-time event and the test robot will learn. Linus
Re: [PATCH] sparse: use identifiers to define address spaces
Hi Luc, I love your patch! Perhaps something to improve: [auto build test WARNING on linux/master] [also build test WARNING on linus/master v5.8-rc1 next-20200618] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Luc-Van-Oostenryck/sparse-use-identifiers-to-define-address-spaces/20200618-060614 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a5dc8300df75e8b8384b4c82225f1e4a0b4d9b55 config: x86_64-randconfig-s031-20200618 (attached as .config) compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-rc1-10-gc17b1b06-dirty # save the attached .config to linux build tree make W=1 C=1 ARCH=x86_64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot sparse warnings: (new ones prefixed by >>) >> fs/f2fs/trace.c:142:9: sparse: sparse: incorrect type in assignment >> (different address spaces) @@ expected void **slot @@ got void >> [noderef] __rcu ** @@ fs/f2fs/trace.c:142:9: sparse: expected void **slot >> fs/f2fs/trace.c:142:9: sparse: got void [noderef] __rcu ** >> fs/f2fs/trace.c:142:9: sparse: sparse: incorrect type in assignment >> (different address spaces) @@ expected void **slot @@ got void >> [noderef] __rcu ** @@ fs/f2fs/trace.c:142:9: sparse: expected void **slot >> fs/f2fs/trace.c:142:9: sparse: got void [noderef] __rcu ** >> fs/f2fs/trace.c:142:9: sparse: sparse: incorrect type in argument 1 >> (different address spaces) @@ expected void [noderef] __rcu **slot @@ >> got void **slot @@ >> fs/f2fs/trace.c:142:9: sparse: expected void [noderef] __rcu **slot fs/f2fs/trace.c:142:9: sparse: got void **slot >> fs/f2fs/trace.c:142:9: sparse: sparse: incorrect type in assignment >> (different address spaces) @@ expected void **slot @@ got void >> [noderef] __rcu ** @@ fs/f2fs/trace.c:142:9: sparse: expected void **slot >> fs/f2fs/trace.c:142:9: sparse: got void [noderef] __rcu ** -- kernel/sched/core.c:512:38: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected struct task_struct *curr @@ got struct task_struct [noderef] __rcu *curr @@ kernel/sched/core.c:512:38: sparse: expected struct task_struct *curr kernel/sched/core.c:512:38: sparse: got struct task_struct [noderef] __rcu *curr kernel/sched/core.c:567:9: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct sched_domain *[assigned] sd @@ got struct sched_domain [noderef] __rcu *parent @@ kernel/sched/core.c:567:9: sparse: expected struct sched_domain *[assigned] sd kernel/sched/core.c:567:9: sparse: got struct sched_domain [noderef] __rcu *parent kernel/sched/core.c:1432:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct task_struct *p @@ got struct task_struct [noderef] __rcu *curr @@ kernel/sched/core.c:1432:33: sparse: expected struct task_struct *p kernel/sched/core.c:1432:33: sparse: got struct task_struct [noderef] __rcu *curr kernel/sched/core.c:1432:68: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct task_struct *tsk @@ got struct task_struct [noderef] __rcu *curr @@ kernel/sched/core.c:1432:68: sparse: expected struct task_struct *tsk kernel/sched/core.c:1432:68: sparse: got struct task_struct [noderef] __rcu *curr kernel/sched/core.c:2176:17: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct sched_domain *[assigned] sd @@ got struct sched_domain [noderef] __rcu *parent @@ kernel/sched/core.c:2176:17: sparse: expected struct sched_domain *[assigned] sd kernel/sched/core.c:2176:17: sparse: got struct sched_domain [noderef] __rcu *parent kernel/sched/core.c:2342:36: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct task_struct const *p @@ got struct task_struct [noderef] __rcu *curr @@ kernel/sched/core.c:2342:36: sparse: expected struct task_struct const *p kernel/sched/core.c:2342:36: sparse: got struct task_struct [noderef] __rcu *curr kernel/sched/core.c:3650:38: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected struct task_struct *curr @@ got struct task_struct [noderef] __rcu *curr @@ kernel/sched/core.c:3650:38: sparse: expected struct task_struct *curr kernel/sched/core.c:3650:38: sparse: got struct task_struct [noderef] _
Re: [PATCH v2 02/12] ocxl: Change type of pasid to unsigned int
Le 18/06/2020 à 17:37, Fenghua Yu a écrit : The first 3 patches clean up pasid and flag defitions to prepare for following patches. If you think this patch can be dropped, we will drop it. Yes, I think that's the case. Thanks, Fred
Re: [PATCH] usbip: tools: fix module name in man page
On 6/17/20 6:08 PM, Antonio Borneo wrote:
Commit 64e62426f40d ("staging: usbip: edit Kconfig and rename
CONFIG options") renamed the module usbip as usbip-host, but the
example in the man page still reports the old module name.
Fix the module name in usbipd.8
Signed-off-by: Antonio Borneo
Fixes: 64e62426f40d ("staging: usbip: edit Kconfig and rename CONFIG options")
---
tools/usb/usbip/doc/usbipd.8 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/usb/usbip/doc/usbipd.8 b/tools/usb/usbip/doc/usbipd.8
index ac4635db3f03..fb62a756893b 100644
--- a/tools/usb/usbip/doc/usbipd.8
+++ b/tools/usb/usbip/doc/usbipd.8
@@ -73,7 +73,7 @@ USB/IP client can connect and use exported devices.
.SH EXAMPLES
-server:# modprobe usbip
+server:# modprobe usbip-host
server:# usbipd -D
- Start usbip daemon.
base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
Looks good. Thanks for fixing this.
Acked-by: Shuah Khan
thanks,
-- Shuah
Re: [PATCH v2 2/8] iommu: arm-smmu-impl: Use qcom impl for sm8150 and sm8250 compatibles
On 2020-06-09 20:40, Jonathan Marek wrote: Use the qcom implementation for IOMMU hardware on sm8150 and sm8250 SoCs. Given a promise that anyone who wants to add more of these in future converts it into an of_device_id table exported from arm-smmu-qcom, Reviewed-by Robin Murphy Signed-off-by: Jonathan Marek --- drivers/iommu/arm-smmu-impl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c index c75b9d957b70..f5f6cab626be 100644 --- a/drivers/iommu/arm-smmu-impl.c +++ b/drivers/iommu/arm-smmu-impl.c @@ -172,7 +172,9 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu) smmu->impl = _impl; if (of_device_is_compatible(np, "qcom,sdm845-smmu-500") || - of_device_is_compatible(np, "qcom,sc7180-smmu-500")) + of_device_is_compatible(np, "qcom,sc7180-smmu-500") || + of_device_is_compatible(np, "qcom,sm8150-smmu-500") || + of_device_is_compatible(np, "qcom,sm8250-smmu-500")) return qcom_smmu_impl_init(smmu); return smmu;
Re: [PATCH v3 04/15] arm64: kvm: Handle calls to prefixed hyp functions
Hi David,
On 2020-06-18 13:25, David Brazdil wrote:
From: Andrew Scull
This patch is part of a series which builds KVM's non-VHE hyp code
separately
from VHE and the rest of the kernel.
Once hyp functions are moved to a hyp object, they will have prefixed
symbols.
This change declares and gets the address of the prefixed version for
calls to
the hyp functions.
To aid migration, the hyp functions that have not yet moved have their
prefixed
versions aliased to their non-prefixed version. This begins with all
the hyp
functions being listed and will reduce to none of them once the
migration is
complete.
Signed-off-by: Andrew Scull
Extracted kvm_call_hyp nVHE branches into own helper macros.
Signed-off-by: David Brazdil
nit: if you want to add this kind of comment, try to write it between
square brackets, without blank lines in between:
Signed-off-by: Andrew Scull
[David: Extracted kvm_call_hyp nVHE branches into own helper macros.]
Signed-off-by: David Brazdil
---
arch/arm64/include/asm/kvm_asm.h | 19 +++
arch/arm64/include/asm/kvm_host.h | 19 ---
arch/arm64/kernel/image-vars.h| 15 +++
3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_asm.h
b/arch/arm64/include/asm/kvm_asm.h
index 352aaebf4198..6a682d66a640 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -42,6 +42,24 @@
#include
+/*
+ * Translate name of a symbol defined in nVHE hyp to the name seen
+ * by kernel proper. All nVHE symbols are prefixed by the build system
+ * to avoid clashes with the VHE variants.
+ */
+#define kvm_nvhe_sym(sym) __kvm_nvhe_##sym
+
+#define DECLARE_KVM_VHE_SYM(sym) extern char sym[]
+#define DECLARE_KVM_NVHE_SYM(sym) extern char kvm_nvhe_sym(sym)[]
+
+/*
+ * Define a pair of symbols sharing the same name but one defined in
+ * VHE and the other in nVHE hyp implementations.
+ */
+#define DECLARE_KVM_HYP_SYM(sym) \
+ DECLARE_KVM_VHE_SYM(sym); \
+ DECLARE_KVM_NVHE_SYM(sym)
+
/* Translate a kernel address of @sym into its equivalent linear
mapping */
#define kvm_ksym_ref(sym) \
({ \
@@ -50,6 +68,7 @@
val = lm_alias(); \
val;\
})
+#define kvm_ksym_ref_nvhe(sym) kvm_ksym_ref(kvm_nvhe_sym(sym))
struct kvm;
struct kvm_vcpu;
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index c3e6fcc664b1..e782f98243d3 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -448,6 +448,20 @@ void kvm_arm_resume_guest(struct kvm *kvm);
u64 __kvm_call_hyp(void *hypfn, ...);
+#define kvm_call_hyp_nvhe(f, ...) \
+ do {\
+ DECLARE_KVM_NVHE_SYM(f);\
I wanted to move this out to __kvm_call_hyp, but the nVHE ssbs code
got in the way... Oh well.
+ __kvm_call_hyp(kvm_ksym_ref_nvhe(f), ##__VA_ARGS__);\
+ } while(0)
+
+#define kvm_call_hyp_nvhe_ret(f, ...) \
+ ({ \
+ DECLARE_KVM_NVHE_SYM(f);\
+ typeof(f(__VA_ARGS__)) ret; \
+ ret = __kvm_call_hyp(kvm_ksym_ref_nvhe(f), \
+##__VA_ARGS__);\
You don't need to redefine ret here. actually, you can just evaluate
the expression and let C do its magic:
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index e782f98243d3..49d1a5cd8f8f 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -457,9 +457,7 @@ u64 __kvm_call_hyp(void *hypfn, ...);
#define kvm_call_hyp_nvhe_ret(f, ...) \
({ \
DECLARE_KVM_NVHE_SYM(f);\
- typeof(f(__VA_ARGS__)) ret; \
- ret = __kvm_call_hyp(kvm_ksym_ref_nvhe(f), \
-##__VA_ARGS__);\
+ __kvm_call_hyp(kvm_ksym_ref_nvhe(f), ##__VA_ARGS__);\
})
/*
+ })
+
/*
* The couple of isb() below are there to guarantee the same behaviour
* on VHE as on !VHE, where the eret to EL1 acts as a context
@@ -459,7 +473,7 @@ u64 __kvm_call_hyp(void *hypfn, ...);
f(__VA_ARGS__); \
Re: [PATCH] driver core:Export the symbol device_is_bound
On Thu, Jun 18, 2020 at 05:58:20PM +0200, Greg Kroah-Hartman wrote: > On Thu, Jun 18, 2020 at 08:45:55AM -0700, Matthias Kaehlcke wrote: > > Hi Greg, > > > > On Thu, Jun 18, 2020 at 10:14:43AM +0200, Greg Kroah-Hartman wrote: > > > On Wed, Jun 03, 2020 at 12:09:52AM +0530, Sandeep Maheswaram wrote: > > > > Export the symbol device_is_bound so that it can be used by the modules. > > > > > > What modules need this? > > > > drivers/usb/dwc3/dwc3-qcom.c (and probably other dwc3 'wrappers'). > > Why wasn't that said here? No context is not good :( Agreed, this patch should probably have been part of a series to establish the context. > > Short summary: QCOM dwc3 support is split in two drivers, the core dwc3 > > driver and the QCOM specific parts. dwc3-qcom is probed first (through > > a DT entry or ACPI), dwc3_qcom_probe() then calls of_platform_populate() > > to probe the core part. After a successful return from _populate() the > > driver assumes that the core device is fully initialized. However the > > latter is not correct, the driver core doesn't propagate errors from > > probe() to platform_populate(). The dwc3-qcom driver would use > > device_is_bound() to make sure the core device was probed successfully. > > why does the dwc3-qcom driver care? Currently the dwc3-qcom driver uses the core device to determine if the controller is used in peripheral mode and it runtime resumes the XHCI device when it sees a wakeup interrupt. The WIP patch to add interconnect support relies on the core driver to determine the max speed of the controller. > And why is the driver split in a way that requires such "broken" > structures? Why can't that be fixed instead? It seems determining the mode could be easily changed by getting it through the pdev, as in st_dwc3_probe(). Not sure about the other two parts, determining the maximum speed can involve evaluating hardware registers, which currently are 'owned' by the core driver. Manu or Sandeep who know the hardware and the driver better than me might have ideas on how to improve things.
Re: [dm-devel] New mode DM-Verity error handling
On Thu, Jun 18, 2020 at 11:44:45AM -0400, Mike Snitzer wrote: > I do not accept that panicing the system because of verity failure is > reasonable. > > In fact, even rebooting (via DM_VERITY_MODE_RESTART) looks very wrong. > > The device should be put in a failed state and left for admin recovery. That's exactly how the restart mode works on some Android devices. The bootloader sees the verification error and puts the device in recovery mode. Using the restart mode on systems without firmware support won't make sense, obviously. Sami
[PATCH v1 2/6] serial: sunsab: Return proper error code from console ->setup() hook
For unifying console ->setup() handling, which is pure documented,
return error code, rather than non-zero arbitrary number.
Signed-off-by: Andy Shevchenko
Cc: "David S. Miller"
---
drivers/tty/serial/sunsab.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sunsab.c b/drivers/tty/serial/sunsab.c
index 1eb703c980e0..bab551f46963 100644
--- a/drivers/tty/serial/sunsab.c
+++ b/drivers/tty/serial/sunsab.c
@@ -886,7 +886,7 @@ static int sunsab_console_setup(struct console *con, char
*options)
* though...
*/
if (up->port.type != PORT_SUNSAB)
- return -1;
+ return -EINVAL;
printk("Console: ttyS%d (SAB82532)\n",
(sunsab_reg.minor - 64) + con->index);
--
2.27.0
[PATCH v1 0/6] console: unify return codes from ->setup() hook
Some of the console providers treat error code, returned by ->setup() hook, differently. Here is the unification of the behaviour. The drivers checked by one of the below criteria: 1/ the driver has explicit struct console .setup assignment 2/ the driver has assigned callback to the setup member All such drivers were read in order to see if there is any problematic return codes, and fixed accordingly which is this series in the result. Andy Shevchenko (6): mips: Return proper error code from console ->setup() hook serial: sunsab: Return proper error code from console ->setup() hook serial: sunzilog: Return proper error code from console ->setup() hook tty: hvc: Return proper error code from console ->setup() hook console: Propagate error code from console ->setup() console: Fix trivia typo 'change' -> 'chance' arch/mips/fw/arc/arc_con.c| 4 +++- drivers/tty/hvc/hvsi.c| 2 +- drivers/tty/serial/sunsab.c | 2 +- drivers/tty/serial/sunzilog.c | 2 +- kernel/printk/printk.c| 8 5 files changed, 10 insertions(+), 8 deletions(-) -- 2.27.0
Re: [PATCH] ASoC: rockchip: Fix a reference count leak.
On Sat, 13 Jun 2020 15:51:58 -0500, wu000...@umn.edu wrote: > Calling pm_runtime_get_sync increments the counter even in case of > failure, causing incorrect ref count if pm_runtime_put is not called in > error handling paths. Call pm_runtime_put if pm_runtime_get_sync fails. Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: rockchip: Fix a reference count leak. commit: f141a422159a199f4c8dedb7e0df55b3b2cf16cd All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
[PATCH v1 6/6] console: Fix trivia typo 'change' -> 'chance'
I bet the word 'chance' has to be used in 'had a chance to be called', but, alas, I'm not native speaker... Signed-off-by: Andy Shevchenko Cc: Benjamin Herrenschmidt --- kernel/printk/printk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index aaea3ad182e1..6623e975675a 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2705,7 +2705,7 @@ static int try_enable_new_console(struct console *newcon, bool user_specified) /* * Some consoles, such as pstore and netconsole, can be enabled even * without matching. Accept the pre-enabled consoles only when match() -* and setup() had a change to be called. +* and setup() had a chance to be called. */ if (newcon->flags & CON_ENABLED && c->user_specified == user_specified) return 0; -- 2.27.0
[PATCH v1 3/6] serial: sunzilog: Return proper error code from console ->setup() hook
For unifying console ->setup() handling, which is pure documented, return error code, rather than non-zero arbitrary number. Signed-off-by: Andy Shevchenko Cc: "David S. Miller" --- drivers/tty/serial/sunzilog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/sunzilog.c b/drivers/tty/serial/sunzilog.c index 103ab8c556e7..7ea06bbc6197 100644 --- a/drivers/tty/serial/sunzilog.c +++ b/drivers/tty/serial/sunzilog.c @@ -1221,7 +1221,7 @@ static int __init sunzilog_console_setup(struct console *con, char *options) int baud, brg; if (up->port.type != PORT_SUNZILOG) - return -1; + return -EINVAL; printk(KERN_INFO "Console: ttyS%d (SunZilog zs%d)\n", (sunzilog_reg.minor - 64) + con->index, con->index); -- 2.27.0
Re: [PATCH v3 0/5] spi: spi-geni-qcom: Fixes / perf improvements
On Tue, 16 Jun 2020 03:40:45 -0700, Douglas Anderson wrote: > This patch series is a new version of the previous patch posted: > [PATCH v2] spi: spi-geni-qcom: Speculative fix of "nobody cared" about > interrupt > > https://lore.kernel.org/r/20200317133653.v2.1.I752ebdcfd5e8bf0de06d66e767b8974932b3620e@changeid > > At this point I've done enough tracing to know that there was a real > race in the old code (not just weakly ordered memory problems) and > that should be fixed with the locking patches. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: spi-geni-qcom: No need for irqsave variant of spinlock calls commit: 539afdf969d6ad7ded543d9abf14596aec411fe9 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
[PATCH v1 4/6] tty: hvc: Return proper error code from console ->setup() hook
For unifying console ->setup() handling, which is pure documented, return error code, rather than non-zero arbitrary number. Signed-off-by: Andy Shevchenko --- drivers/tty/hvc/hvsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c index 66f95f758be0..e8c58f9bd263 100644 --- a/drivers/tty/hvc/hvsi.c +++ b/drivers/tty/hvc/hvsi.c @@ -1128,7 +1128,7 @@ static int __init hvsi_console_setup(struct console *console, char *options) int ret; if (console->index < 0 || console->index >= hvsi_count) - return -1; + return -EINVAL; hp = _ports[console->index]; /* give the FSP a chance to change the baud rate when we re-open */ -- 2.27.0
[PATCH v1 1/6] mips: Return proper error code from console ->setup() hook
For unifying console ->setup() handling, which is pure documented,
return error code, rather than non-zero arbitrary number.
Signed-off-by: Andy Shevchenko
Cc: Thomas Bogendoerfer
---
arch/mips/fw/arc/arc_con.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/mips/fw/arc/arc_con.c b/arch/mips/fw/arc/arc_con.c
index 365e3913231e..7fdce236b298 100644
--- a/arch/mips/fw/arc/arc_con.c
+++ b/arch/mips/fw/arc/arc_con.c
@@ -28,7 +28,9 @@ static void prom_console_write(struct console *co, const char
*s,
static int prom_console_setup(struct console *co, char *options)
{
- return !(prom_flags & PROM_FLAG_USE_AS_CONSOLE);
+ if (prom_flags & PROM_FLAG_USE_AS_CONSOLE)
+ return 0;
+ return -ENODEV;
}
static struct console arc_cons = {
--
2.27.0
Re: [PATCH 1/2] spi: spidev: fix a race between spidev_release and spidev_remove
On Thu, 18 Jun 2020 11:21:24 +0800, Zhenzhong Duan wrote: > Imagine below scene, spidev is referenced after it's freed. > > spidev_release()spidev_remove() > ... > spin_lock_irq(>spi_lock); > spidev->spi = NULL; > spin_unlock_irq(>spi_lock); > mutex_lock(_list_lock); > dofree = (spidev->spi == NULL); > if (dofree) > kfree(spidev); > mutex_unlock(_list_lock); > mutex_lock(_list_lock); > list_del(>device_entry); > device_destroy(spidev_class, spidev->devt); > clear_bit(MINOR(spidev->devt), minors); > if (spidev->users == 0) > kfree(spidev); > mutex_unlock(_list_lock); > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/2] spi: spidev: fix a race between spidev_release and spidev_remove commit: abd42781c3d2155868821f1b947ae45bbc0d [2/2] spi: spidev: fix a potential use-after-free in spidev_release() commit: 06096cc6c5a84ced929634b0d79376b94c65a4bd All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
[PATCH v1 5/6] console: Propagate error code from console ->setup()
Since console ->setup() hook returns meaningful error codes,
propagate it to the caller of try_enable_new_console().
Signed-off-by: Andy Shevchenko
Cc: Benjamin Herrenschmidt
---
kernel/printk/printk.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8c14835be46c..aaea3ad182e1 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2668,7 +2668,7 @@ early_param("keep_bootcon", keep_bootcon_setup);
static int try_enable_new_console(struct console *newcon, bool user_specified)
{
struct console_cmdline *c;
- int i;
+ int i, err;
for (i = 0, c = console_cmdline;
i < MAX_CMDLINECONSOLES && c->name[0];
@@ -2691,8 +2691,8 @@ static int try_enable_new_console(struct console *newcon,
bool user_specified)
return 0;
if (newcon->setup &&
- newcon->setup(newcon, c->options) != 0)
- return -EIO;
+ (err = newcon->setup(newcon, c->options)) != 0)
+ return err;
}
newcon->flags |= CON_ENABLED;
if (i == preferred_console) {
--
2.27.0
Re: [PATCH 13/29] dt: fix broken links due to txt->yaml renames
On Mon, 15 Jun 2020 08:46:52 +0200, Mauro Carvalho Chehab wrote: > There are some new broken doc links due to yaml renames > at DT. Developers should really run: > > ./scripts/documentation-file-ref-check > > in order to solve those issues while submitting patches. > This tool can even fix most of the issues with: > > ./scripts/documentation-file-ref-check --fix > > Signed-off-by: Mauro Carvalho Chehab > --- > Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 +- > .../devicetree/bindings/display/rockchip/rockchip-drm.yaml| 2 +- > Documentation/devicetree/bindings/net/mediatek-bluetooth.txt | 2 +- > Documentation/devicetree/bindings/sound/audio-graph-card.txt | 2 +- > Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt | 2 +- > Documentation/mips/ingenic-tcu.rst| 2 +- > MAINTAINERS | 4 ++-- > 7 files changed, 8 insertions(+), 8 deletions(-) > Applied, thanks!
Re: [PATCH] tracing: Use linker magic instead of recasting ftrace_ops_list_func()
On Thu, 18 Jun 2020 01:12:37 +0200
Jann Horn wrote:
> static ftrace_func_t ftrace_ops_get_list_func(struct ftrace_ops *ops)
> +static ftrace_asm_func_t ftrace_ops_get_list_func(struct ftrace_ops *ops)
> {
> +#if FTRACE_FORCE_LIST_FUNC
> + return ftrace_ops_list_func;
> +#else
> /*
> * If this is a dynamic, RCU, or per CPU ops, or we force list func,
> * then it needs to call the list anyway.
> */
> - if (ops->flags & (FTRACE_OPS_FL_DYNAMIC | FTRACE_OPS_FL_RCU) ||
> - FTRACE_FORCE_LIST_FUNC)
> + if (ops->flags & (FTRACE_OPS_FL_DYNAMIC | FTRACE_OPS_FL_RCU))
> return ftrace_ops_list_func;
>
> return ftrace_ops_get_func(ops);
But ftrace_ops_get_func() returns ftrace_func_t type, wont this complain?
-- Steve
> +#endif
> }
Re: [PATCH 12/29] dt: update a reference for reneases pcar file renamed to yaml
On Mon, 15 Jun 2020 08:46:51 +0200, Mauro Carvalho Chehab wrote:
> This file was renamed, but its reference at pfc-pinctl.txt is
> still pointing to the old file.
>
> Fixes: 7f7d408e5a00 ("dt-bindings: gpio: rcar: Convert to json-schema")
> Signed-off-by: Mauro Carvalho Chehab
> ---
> .../devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Applied, thanks!
Re: [PATCH v5 3/3] net: phy: mscc: handle the clkout control on some phy variants
On Thu, Jun 18, 2020 at 06:01:29PM +0200, Heiko Stübner wrote: > Am Donnerstag, 18. Juni 2020, 17:47:48 CEST schrieb Russell King - ARM Linux > admin: > > On Thu, Jun 18, 2020 at 05:41:54PM +0200, Heiko Stübner wrote: > > > Though I'm not sure how this fits in the whole bringup of ethernet phys. > > > Like the phy is dependent on the underlying ethernet controller to > > > actually turn it on. > > > > > > I guess we should check the phy-state and if it's not accessible, just > > > keep the values and if it's in a suitable state do the configuration. > > > > > > Calling a vsc8531_config_clkout() from both the vsc8531_config_init() > > > as well as the clk_(un-)prepare and clk_set_rate functions and being > > > protected by a check against phy_is_started() ? > > > > It sounds like it doesn't actually fit the clk API paradym then. I > > see that Rob suggested it, and from the DT point of view, it makes > > complete sense, but then if the hardware can't actually be used in > > the way the clk API expects it to be used, then there's a semantic > > problem. > > > > What is this clock used for? > > It provides a source for the mac-clk for the actual transfers, here to > provide the 125MHz clock needed for the RGMII interface . > > So right now the old rk3368-lion devicetree just declares a stub > fixed-clock and instructs the soc's clock controller to use it [0] . > And in the cover-letter here, I show the update variant with using > the clock defined here. > > > I've added the idea from my previous mail like shown below [1]. > which would take into account the phy-state. > > But I guess I'll wait for more input before spamming people with v6. Let's get a handle on exactly what this is. The RGMII bus has two clocks: RXC and TXC, which are clocked at one of 125MHz, 25MHz or 2.5MHz depending on the RGMII data rate. Some PHYs replace TXC with GTX clock, which always runs at 125MHz. These clocks are not what you're referring to. You are referring to another commonly provided clock between the MAC and the PHY, something which is not unique to your PHY. We seem to be heading down a path where different PHYs end up doing different things in DT for what is basically the same hardware setup, which really isn't good. :( We have at803x using: qca,clk-out-frequency qca,clk-out-strength qca,keep-pll-enabled which are used to control the CLK_25M output pin on the device, which may be used to provide a reference clock for the MAC side, selecting between 25M, 50M, 62.5M and 125MHz. This was introduced in November 2019, so not that long ago. Broadcom PHYs configure their 125MHz clock through the PHY device flags passed from the MAC at attach/connect time. There's the dp83867 and dp83869 configuration (I'm not sure I can make sense of it from reading the code) using ti,clk-output-sel - but it looks like it's the same kind of thing. Introduced February 2018 into one driver, and November 2019 in the other. It seems the Micrel PHYs produce a 125MHz clock irrespective of any configuration (maybe configured by firmware, or hardware strapping?) So it seems we have four ways of doing the same thing today, and now the suggestion is to implement a fifth different way. I think there needs to be some consolidation here, maybe choosing one approach and sticking with it. Hence, I disagree with Rob - we don't need a fifth approach, we need to choose one approach and decide that's our policy for this and apply it evenly across the board, rather than making up something different each time a new PHY comes along. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [Cluster-devel] [PATCH v11 16/25] fs: Convert mpage_readpages to mpage_readahead
On Thu, Jun 18, 2020 at 5:03 PM Matthew Wilcox wrote:
>
> On Thu, Jun 18, 2020 at 02:46:03PM +0200, Andreas Gruenbacher wrote:
> > On Wed, Jun 17, 2020 at 4:22 AM Matthew Wilcox wrote:
> > > On Wed, Jun 17, 2020 at 02:57:14AM +0200, Andreas Grünbacher wrote:
> > > > Right, the approach from the following thread might fix this:
> > > >
> > > > https://lore.kernel.org/linux-fsdevel/20191122235324.17245-1-agrue...@redhat.com/T/#t
> > >
> > > In general, I think this is a sound approach.
> > >
> > > Specifically, I think FAULT_FLAG_CACHED can go away. map_pages()
> > > will bring in the pages which are in the page cache, so when we get to
> > > gfs2_fault(), we know there's a reason to acquire the glock.
> >
> > We'd still be grabbing a glock while holding a dependent page lock.
> > Another process could be holding the glock and could try to grab the
> > same page lock (i.e., a concurrent writer), leading to the same kind
> > of deadlock.
>
> What I'm saying is that gfs2_fault should just be:
>
> +static vm_fault_t gfs2_fault(struct vm_fault *vmf)
> +{
> + struct inode *inode = file_inode(vmf->vma->vm_file);
> + struct gfs2_inode *ip = GFS2_I(inode);
> + struct gfs2_holder gh;
> + vm_fault_t ret;
> + int err;
> +
> + gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, );
> + err = gfs2_glock_nq();
> + if (err) {
> + ret = block_page_mkwrite_return(err);
> + goto out_uninit;
> + }
> + ret = filemap_fault(vmf);
> + gfs2_glock_dq();
> +out_uninit:
> + gfs2_holder_uninit();
> + return ret;
> +}
>
> because by the time gfs2_fault() is called, map_pages() has already been
> called and has failed to insert the necessary page, so we should just
> acquire the glock now instead of trying again to look for the page in
> the page cache.
Okay, that's great.
Thanks,
Andreas
RE: [PATCH] x86/asm/64: Align start of __clear_user() loop to 16-bytes
From: Alexey Dobriyan > Sent: 18 June 2020 14:17 ... > > > diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c > > > index fff28c6f73a2..b0dfac3d3df7 100644 > > > --- a/arch/x86/lib/usercopy_64.c > > > +++ b/arch/x86/lib/usercopy_64.c > > > @@ -24,6 +24,7 @@ unsigned long __clear_user(void __user *addr, unsigned > > > long size) > > > asm volatile( > > > " testq %[size8],%[size8]\n" > > > " jz 4f\n" > > > + " .align 16\n" > > > "0: movq $0,(%[dst])\n" > > > " addq $8,%[dst]\n" > > > " decl %%ecx ; jnz 0b\n" > > > > You can do better that that loop. > > Change 'dst' to point to the end of the buffer, negate the count > > and divide by 8 and you get: > > "0: movq $0,($[dst],%%ecx,8)\n" > > " add $1,%%ecx" > > " jnz 0b\n" > > which might run at one iteration per clock especially on cpu that pair > > the add and jnz into a single uop. > > (You need to use add not inc.) > > /dev/zero should probably use REP STOSB etc just like everything else. Almost certainly it shouldn't, and neither should anything else. Potentially it could use whatever memset() is patched to. That MIGHT be 'rep stos' on some cpu variants, but in general it is slow. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH v3 1/2] mfd: da9063: Fix revision handling to correctly select reg tables
On Thu, 18 Jun 2020, Adam Thomson wrote: > On 18 June 2020 12:15, Lee Jones wrote: > > > > > > The current implementation performs checking in the i2c_probe() > > > > > function of the variant_code but does this immediately after the > > > > > containing struct has been initialised as all zero. This means the > > > > > check for variant code will always default to using the BB tables > > > > > and will never select AD. The variant code is subsequently set > > > > > by device_init() and later used by the RTC so really it's a little > > > > > fortunate this mismatch works. > > > > > > > > > > This update adds raw I2C read access functionality to read the chip > > > > > and variant/revision information (common to all revisions) so that > > > > > it can subsequently correctly choose the proper regmap tables for > > > > > real initialisation. > > > > > > > > > > Signed-off-by: Adam Thomson > > > > > > > --- > > > > > drivers/mfd/da9063-core.c| 31 -- > > > > > drivers/mfd/da9063-i2c.c | 184 > > +++- > > > > --- > > > > > include/linux/mfd/da9063/registers.h | 15 ++- > > > > > 3 files changed, 177 insertions(+), 53 deletions(-) [...] > > > > Rather than open coding this, does it make sense to register a small > > > > (temporary?) Device ID Regmap to read from? > > > > > > The original patch submission did exactly that but you indicated you > > > weren't > > > keen due to overheads, hence the implementation above. Actually what we > > have > > > here is a bit smaller than the regmap approach and I really I'd rather not > > > have to respin again just to revert to something that was dismissed in > > > the first > > > place over 6 months ago. > > > > Actually the conversation went like: > > > > Lee: > > IIUC, you have a dependency issue whereby the device type is required > > before you can select the correct Regmap configuration. Is that > > correct? > > > > If so, using Regmap for the initial register reads sounds like > > over-kill. What's stopping you simply using raw reads before the > > Regmap is instantiated? > > > > Adam: > > Actually nothing and I did consider this at the start. Nice thing with > > regmap > > is it's all tidily contained and provides the page swapping mechanism to > > access > > higher page registers like the variant information. Given this is only > > once at > > probe time it felt like this was a reasonable solution. However if you're > > not > > keen I can update to use raw access instead. > > > > Lee: > > It would be nice to compare the 2 solutions side by side. I can't see > > the raw reads of a few device-ID registers being anywhere near 170 > > lines though. > > > > Ah, they are I2C transactions? Not the nice readl()s I had in mind. > > > > Adam: > > I can knock something together though just to see what it looks like. > > > > Lee: > > Well, I'd appreciated that, thanks. > > > > So now we can see them side-by-side we can take them on their own > > merits. When I initially requested raw reads, I had readl()s in mind, > > rather than the extensive code required to read the required registers > > via I2C. > > To be fair those changes were in V2 of the patch set, which is why I was a > quite > surprised by your suggestion today as you hadn't made this comment against > that > version, given the previous discussion. It would be very difficult to remember complete revision history for every patch received. Especially with the lack of a provided patch-level changelog. So I have to take each submission on its own merits. This is particularly true for patch-sets ranging over 6 months or more. The Regmap vs raw accesses decision could easily have gone either way, so it's not surprising the other was considered during the review of each submission. Both times were phrased as an "I wonder if" enquiry, rather than a demand. > > However, it looks like there is very little difference between them, > > thus I do not see a benefit to reverting it back. The current version > > seems fine. > > > > I'll conduct a full review shortly, when I have a little more spare > > time (looking at my current TODO list, this will probably be Monday > > now). Although everything does look fine at first glance. > > Thanks. That would be appreciated. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
RE: [PATCH v10 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe controller errors
Hi Andy,
>-Original Message-
>From: Andy Shevchenko [mailto:andriy.shevche...@linux.intel.com]
>Sent: 18 June 2020 16:56
>To: Shiju Jose
>Cc: linux-a...@vger.kernel.org; linux-...@vger.kernel.org; linux-
>ker...@vger.kernel.org; r...@rjwysocki.net; helg...@kernel.org;
>b...@alien8.de; james.mo...@arm.com; l...@kernel.org;
>tony.l...@intel.com; dan.carpen...@oracle.com;
>zhangligu...@linux.alibaba.com; Wangkefeng (OS Kernel Lab)
>; jroe...@suse.de; Linuxarm
>; yangyicong ; Jonathan
>Cameron ; tanxiaofei
>
>Subject: Re: [PATCH v10 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe
>controller errors
>
>On Thu, Jun 18, 2020 at 04:40:51PM +0100, Shiju Jose wrote:
>> From: Yicong Yang
>>
>> The HiSilicon HIP PCIe controller is capable of handling errors on
>> root port and perform port reset separately at each root port.
>>
>> Add error handling driver for HIP PCIe controller to log and report
>> recoverable errors. Perform root port reset and restore link status
>> after the recovery.
>>
>> Following are some of the PCIe controller's recoverable errors 1.
>> completion transmission timeout error.
>> 2. CRS retry counter over the threshold error.
>> 3. ECC 2 bit errors
>> 4. AXI bresponse/rresponse errors etc.
>>
>> The driver placed in the drivers/pci/controller/ because the HIP PCIe
>> controller does not use DWC ip.
>
>> Reviewed-by: Andy Shevchenko
>
>Hmm... Did I give a tag?
>
>...
>
>> +static guid_t hisi_pcie_sec_guid =
>> +GUID_INIT(0xB2889FC9, 0xE7D7, 0x4F9D,
>> +0xA8, 0x67, 0xAF, 0x42, 0xE9, 0x8B, 0xE7, 0x72);
>
>Drop one TAB in each line and add two spaces before 0xA8 on the last.
Sure.
>
>
>...
>
>> +idx = HISI_PCIE_LOCAL_VALID_ERR_MISC;
>
>> +for_each_set_bit_from(idx, (const unsigned long *)>val_bits,
>
>Can't you make val_bits unsigned long? Because this casting is incorrect.
>Otherwise, make a local copy into unsigned long variable.
The data val_bits in the error record is 64 bits, thus used u64.
Casting is added because of a compilation warning on _find_nex_bit_ function as
it
expects the type of the address as const unsigned long*.
Probably I will make local copy of val_bits into unsigned long variable.
>
>> + HISI_PCIE_LOCAL_VALID_ERR_MISC +
>HISI_PCIE_ERR_MISC_REGS)
>> +dev_info(dev, "ERR_MISC_%d = 0x%x\n", idx -
>HISI_PCIE_LOCAL_VALID_ERR_MISC,
>> + edata->err_misc[idx]);
>
>...
>
>> +static int hisi_pcie_error_handler_probe(struct platform_device
>> +*pdev) {
>> +struct hisi_pcie_error_private *priv;
>> +int ret;
>> +
>
>> +priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
>
>(1)
>
>> +if (!priv)
>> +return -ENOMEM;
>> +
>> +priv->nb.notifier_call = hisi_pcie_notify_error;
>> +priv->dev = >dev;
>> +ret = ghes_register_event_notifier(>nb);
>> +if (ret) {
>> +dev_err(>dev,
>> +"Failed to register hisi_pcie_notify_error
>function\n");
>> +return ret;
>> +}
>> +
>> +platform_set_drvdata(pdev, priv);
>> +
>> +return 0;
>> +}
>> +
>> +static int hisi_pcie_error_handler_remove(struct platform_device
>> +*pdev) {
>> +struct hisi_pcie_error_private *priv = platform_get_drvdata(pdev);
>> +
>> +ghes_unregister_event_notifier(>nb);
>
>> +kfree(priv);
>
>See (1), as I told you, this is double free.
>Have you tested this?
>
>> +return 0;
>> +}
>
>--
>With Best Regards,
>Andy Shevchenko
>
Thanks,
Shiju
Re: [PATCH v3 03/15] arm64: kvm: Add build rules for separate nVHE object files
Hi David,
On 2020-06-18 13:25, David Brazdil wrote:
Add new folder arch/arm64/kvm/hyp/nvhe and a Makefile for building code
that
runs in EL2 under nVHE KVM.
Compile each source file into a `.hyp.tmp.o` object first, then prefix
all
its symbols with "__kvm_nvhe_" using `objcopy` and produce a `.hyp.o`.
Suffixes were chosen so that it would be possible for VHE and nVHE to
share
some source files, but compiled with different CFLAGS. nVHE build rules
add
-D__KVM_NVHE_HYPERVISOR__.
The nVHE ELF symbol prefix is added to kallsyms.c as ignored. EL2-only
symbols
will never appear in EL1 stack traces.
Signed-off-by: David Brazdil
---
arch/arm64/kernel/image-vars.h | 12 +++
arch/arm64/kvm/hyp/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/Makefile | 35
scripts/kallsyms.c | 1 +
4 files changed, 49 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/nvhe/Makefile
diff --git a/arch/arm64/kernel/image-vars.h
b/arch/arm64/kernel/image-vars.h
index be0a63ffed23..f32b406e90c0 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -51,4 +51,16 @@ __efistub__ctype = _ctype;
#endif
+#ifdef CONFIG_KVM
+
+/*
+ * KVM nVHE code has its own symbol namespace prefixed by __kvm_nvhe_,
to
+ * isolate it from the kernel proper. The following symbols are
legally
+ * accessed by it, therefore provide aliases to make them linkable.
+ * Do not include symbols which may not be safely accessed under
hypervisor
+ * memory mappings.
+ */
+
+#endif /* CONFIG_KVM */
+
#endif /* __ARM64_KERNEL_IMAGE_VARS_H */
diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
index 5d8357ddc234..5f4f217532e0 100644
--- a/arch/arm64/kvm/hyp/Makefile
+++ b/arch/arm64/kvm/hyp/Makefile
@@ -6,7 +6,7 @@
ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
$(DISABLE_STACKLEAK_PLUGIN)
-obj-$(CONFIG_KVM) += hyp.o
+obj-$(CONFIG_KVM) += hyp.o nvhe/
obj-$(CONFIG_KVM_INDIRECT_VECTORS) += smccc_wa.o
hyp-y := vgic-v3-sr.o timer-sr.o aarch32.o vgic-v2-cpuif-proxy.o
sysreg-sr.o \
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile
b/arch/arm64/kvm/hyp/nvhe/Makefile
new file mode 100644
index ..7d64235dba62
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for Kernel-based Virtual Machine module, HYP/nVHE part
+#
+
+asflags-y := -D__KVM_NVHE_HYPERVISOR__
+ccflags-y := -D__KVM_NVHE_HYPERVISOR__ -fno-stack-protector \
+-DDISABLE_BRANCH_PROFILING $(DISABLE_STACKLEAK_PLUGIN)
+
+obj-y :=
+
+obj-y := $(patsubst %.o,%.hyp.o,$(obj-y))
+extra-y := $(patsubst %.hyp.o,%.hyp.tmp.o,$(obj-y))
+
+$(obj)/%.hyp.tmp.o: $(src)/%.c FORCE
+ $(call if_changed_rule,cc_o_c)
+$(obj)/%.hyp.tmp.o: $(src)/%.S FORCE
+ $(call if_changed_rule,as_o_S)
+$(obj)/%.hyp.o: $(obj)/%.hyp.tmp.o FORCE
+ $(call if_changed,hypcopy)
+
+quiet_cmd_hypcopy = HYPCOPY $@
+ cmd_hypcopy = $(OBJCOPY) --prefix-symbols=__kvm_nvhe_ $< $@
+
+# KVM nVHE code is run at a different exception code with a different
map, so
+# compiler instrumentation that inserts callbacks or checks into the
code may
+# cause crashes. Just disable it.
+GCOV_PROFILE := n
+KASAN_SANITIZE := n
+UBSAN_SANITIZE := n
+KCOV_INSTRUMENT:= n
+
+# Skip objtool checking for this directory because nVHE code is
compiled with
+# non-standard build rules.
+OBJECT_FILES_NON_STANDARD := y
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 6dc3078649fa..0096cd965332 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -109,6 +109,7 @@ static bool is_ignored_symbol(const char *name,
char type)
".LASANPC", /* s390 kasan local symbols */
"__crc_", /* modversions */
"__efistub_", /* arm64 EFI stub namespace */
+ "__kvm_nvhe_",/* arm64 non-VHE KVM namespace */
NULL
};
I guess that one of the first use of this __KVM_NVHE_HYPERVISOR__
flag could be the has_vhe() predicate: if you're running the nVHE
code, you are *guaranteed* not to use VHE at all.
Something like:
diff --git a/arch/arm64/include/asm/virt.h
b/arch/arm64/include/asm/virt.h
index 5051b388c654..b2cb8fce43dd 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -85,10 +85,8 @@ static inline bool is_kernel_in_hyp_mode(void)
static __always_inline bool has_vhe(void)
{
- if (cpus_have_final_cap(ARM64_HAS_VIRT_HOST_EXTN))
- return true;
-
- return false;
+ return (__is_defined(__KVM_NVHE_HYPERVISOR__) &&
+ cpus_have_final_cap(ARM64_HAS_VIRT_HOST_EXTN));
}
#endif /* __ASSEMBLY__ */
Thanks,
M.
--
Jazz is not dead. It just smells funny...
[PATCH v2] tracing/boottime: Fix kprobe multiple events
Fix boottime kprobe events to report and abort after each failure when
adding probes.
As an example, when we try to set multiprobe kprobe events in
bootconfig like this:
ftrace.event.kprobes.vfsevents {
probes = "vfs_read $arg1 $arg2,,
!error! not reported;?", // leads to error
"vfs_write $arg1 $arg2"
}
This will not work as expected. After
commit da0f1f4167e3af69e ("tracing/boottime: Fix kprobe event API usage"),
the function trace_boot_add_kprobe_event will not produce any error
message when adding a probe fails at kprobe_event_gen_cmd_start.
Furthermore, we continue to add probes when kprobe_event_gen_cmd_end fails
(and kprobe_event_gen_cmd_start did not fail). In this case the function
even returns successfully when the last call to kprobe_event_gen_cmd_end
is successful.
The behaviour of reporting and aborting after failures is not
consistent.
The function trace_boot_add_kprobe_event now reports each failure and
stops adding probes immediately.
Cc: linux-ker...@i4.cs.fau.de
Co-developed-by: Maximilian Werner
Signed-off-by: Maximilian Werner
Signed-off-by: Sascha Ortmann
---
kernel/trace/trace_boot.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index 9de29bb45a27..be893eb22071 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -101,12 +101,16 @@ trace_boot_add_kprobe_event(struct xbc_node *node, const
char *event)
kprobe_event_cmd_init(, buf, MAX_BUF_LEN);
ret = kprobe_event_gen_cmd_start(, event, val);
- if (ret)
+ if (ret) {
+ pr_err("Failed to generate probe: %s\n", buf);
break;
+ }
ret = kprobe_event_gen_cmd_end();
- if (ret)
+ if (ret) {
pr_err("Failed to add probe: %s\n", buf);
+ break;
+ }
}
return ret;
--
2.17.1
Re: [PATCH v5 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
Hi Alex,
On 02/04/2020 19:35, Alex Riesen wrote:
> As all known variants of the Salvator board have the HDMI decoder
> chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
> endpoint and the connection definitions are placed in the common board
> file.
>
> For the same reason, the CLK_C clock line and I2C configuration (similar
> to the ak4613, on the same interface) are added into the common file.
>
> Signed-off-by: Alexander Riesen
>
> --
>
> v5: Add dummy ssi4 node to the rcar sound card in r8a77961, as the
> devices (Salvator-X 2nd version with R-Car M3 W+) also reference
> salvator-common.dtsi.
> Suggested-by: Geert Uytterhoeven
>
> v2: Also add ssi4_ctrl pin group in the sound pins. The pins are
> responsible for SCK4 (sample clock) WS4 and (word boundary input),
> and are required for SSI audio input over I2S.
>
> The adv748x shall provide its own implementation of the output clock
> (MCLK), connected to the audio_clk_c line of the R-Car SoC.
>
> If the frequency of the ADV748x MCLK were fixed, the clock
> implementation were not necessary, but it does not seem so: the MCLK
> depends on the value in a speed multiplier register and the input sample
> rate (48kHz).
>
> Remove audio clock C from the clocks of adv7482.
>
> The clocks property of the video-receiver node lists the input
> clocks of the device, which is quite the opposite from the
> original intention: the adv7482 on Salvator X boards is a
> provide of the MCLK clock for I2S audio output.
>
> Remove old definition of _card.dais and reduce size of changes
> in the Salvator-X specific device tree source.
>
> Declare video-receiver a clock producer, as the adv748x driver
> implements the master clock used I2S audio output.
>
> Suggested-by: Geert Uytterhoeven
>
> v2: The driver provides only MCLK clock, not the SCLK and LRCLK,
> which are part of the I2S protocol.
>
> Suggested-by: Laurent Pinchart
> ---
> .../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +-
> arch/arm64/boot/dts/renesas/r8a77961.dtsi | 1 +
> .../boot/dts/renesas/salvator-common.dtsi | 47 +--
> 3 files changed, 45 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
> b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
> index 2438825c9b22..e16c146808b6 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
> @@ -146,7 +146,8 @@ {
> _card {
> dais = <_port0 /* ak4613 */
> _port1 /* HDMI0 */
> - _port2>; /* HDMI1 */
> + _port2 /* HDMI1 */
> + _port3>; /* adv7482 hdmi-in */
Ah - that was confusing at first... but HDMI0 and HDMI1 are *outputs*,
where of course the adv7482 is an input ;-)
Otherwise, I can't spot anything else yet so:
Reviewed-by: Kieran Bingham
But I fear there may have been some churn around here, so it would be
good to see a rebase too.
--
Kieran
> };
>
> _phy2 {
> diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
> index be3824bda632..b79907beaf31 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
> @@ -861,6 +861,7 @@ rcar_sound,src {
> rcar_sound,ssi {
> ssi0: ssi-0 { };
> ssi1: ssi-1 { };
> + ssi4: ssi-4 { };
> };
> };
>
> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index 98bbcafc8c0d..ead7f8d7a929 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -460,7 +460,7 @@ pca9654: gpio@20 {
> #gpio-cells = <2>;
> };
>
> - video-receiver@70 {
> + adv7482_hdmi_in: video-receiver@70 {
> compatible = "adi,adv7482";
> reg = <0x70 0x71 0x72 0x73 0x74 0x75
> 0x60 0x61 0x62 0x63 0x64 0x65>;
> @@ -469,6 +469,7 @@ video-receiver@70 {
>
> #address-cells = <1>;
> #size-cells = <0>;
> + #clock-cells = <0>; /* the MCLK for I2S output */
>
> interrupt-parent = <>;
> interrupt-names = "intrq1", "intrq2";
> @@ -510,6 +511,15 @@ adv7482_txb: endpoint {
> remote-endpoint = <_in>;
> };
> };
> +
> + port@c {
> + reg = <12>;
> +
> + adv7482_i2s: endpoint {
> + remote-endpoint = <_endpoint3>;
> + system-clock-direction-out;
> + };
> + };
> };
>
> csa_vdd: adc@7c
Re: [PATCH v4 1/2] thermal: add support for the MCU controlled FAN on Khadas boards
On Thu, 18 Jun 2020, Neil Armstrong wrote: > The new Khadas VIM2 and VIM3 boards controls the cooling fan via the > on-board microcontroller. > > This implements the FAN control as thermal devices and as cell of the Khadas > MCU MFD driver. > > Signed-off-by: Neil Armstrong > Reviewed-by: Amit Kucheria Is this an Ack? If so, do you require a pull-request? > --- > Hi Lee, > > Could you apply this patch via the MFD tree since it depends on > the linux/mfd/khadas-mcu.h header ? > > This patch is unchanged from the v3 serie. > > Thanks, > Neil > > drivers/thermal/Kconfig | 11 ++ > drivers/thermal/Makefile | 1 + > drivers/thermal/khadas_mcu_fan.c | 174 +++ > 3 files changed, 186 insertions(+) > create mode 100644 drivers/thermal/khadas_mcu_fan.c -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v5 8/9] dt-bindings: adv748x: add information about serial audio interface (I2S/TDM)
Hi Alex,
On 02/04/2020 19:35, Alex Riesen wrote:
> As the driver has some support for the audio interface of the device,
> the bindings file should mention it.
>
> Signed-off-by: Alexander Riesen
> Reviewed-by: Rob Herring
> Reviewed-by: Laurent Pinchart
Reviewed-by: Kieran Bingham
> --
>
> v3: remove optionality off MCLK clock cell to ensure the description
> matches the hardware no matter if the line is connected.
> Suggested-by: Geert Uytterhoeven
> ---
> .../devicetree/bindings/media/i2c/adv748x.txt| 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/adv748x.txt
> b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
> index 4f91686e54a6..50a753189b81 100644
> --- a/Documentation/devicetree/bindings/media/i2c/adv748x.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
> @@ -2,7 +2,9 @@
>
> The ADV7481 and ADV7482 are multi format video decoders with an integrated
> HDMI receiver. They can output CSI-2 on two independent outputs TXA and TXB
> -from three input sources HDMI, analog and TTL.
> +from three input sources HDMI, analog and TTL. There is also support for an
> +I2S-compatible interface connected to the audio processor of the HDMI
> decoder.
> +The interface has TDM capability (8 slots, 32 bits, left or right justified).
>
> Required Properties:
>
> @@ -16,6 +18,8 @@ Required Properties:
> slave device on the I2C bus. The main address is mandatory, others are
> optional and remain at default values if not specified.
>
> + - #clock-cells: must be <0>
> +
> Optional Properties:
>
>- interrupt-names: Should specify the interrupts as "intrq1", "intrq2"
> and/or
> @@ -47,6 +51,7 @@ are numbered as follows.
> TTL sink9
> TXA source 10
> TXB source 11
> + I2S source 12
>
> The digital output port nodes, when present, shall contain at least one
> endpoint. Each of those endpoints shall contain the data-lanes property as
> @@ -72,6 +77,7 @@ Example:
>
> #address-cells = <1>;
> #size-cells = <0>;
> + #clock-cells = <0>;
>
> interrupt-parent = <>;
> interrupt-names = "intrq1", "intrq2";
> @@ -113,4 +119,12 @@ Example:
> remote-endpoint = <_in>;
> };
> };
> +
> + port@c {
> + reg = <12>;
> +
> + adv7482_i2s: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
> };
>
--
Regards
--
Kieran
Re: [PATCH 6/6] smp: Cleanup smp_call_function*()
On Wed, Jun 17, 2020 at 11:51:07PM -0700, Christoph Hellwig wrote: > Much better. Although if we touch all the callers we might as well > pass the csd as the argument to the callback, as with that we can > pretty trivially remove the private data field later. My plan was to introduce a new function and type and convert smp_call_function_async() callers over to that. The csd as it exists is useful for the regular smp_call_function*() API. > Btw, it seems the callers that don't have the CSD embedded into the > containing structure seems to be of these two kinds: > > - reimplementing on_each_cpumask (mostly because they can be called >from irq context) These are fairly special purpose constructs; and they come at the cost of extra per-cpu storage and they have the limitiation that they must wait for completion of the first before they can be used again. > - reimplenenting smp_call_function_single because they want >to sleep instead of busy wait These are atrocious pieces of crap (the x86/msr ones), the reason it was done is because virt :/
Re: [PATCH 4/4] pci: export untrusted attribute in sysfs
Hi Greg,
On Thu, Jun 18, 2020 at 06:02:12PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jun 18, 2020 at 08:03:49AM -0700, Rajat Jain wrote:
> > Hello,
> >
> > On Thu, Jun 18, 2020 at 2:14 AM Andy Shevchenko
> > wrote:
> > >
> > > On Thu, Jun 18, 2020 at 11:36 AM Greg Kroah-Hartman
> > > wrote:
> > > >
> > > > On Thu, Jun 18, 2020 at 11:12:56AM +0300, Andy Shevchenko wrote:
> > > > > On Wed, Jun 17, 2020 at 10:56 PM Rajat Jain
> > > > > wrote:
> > > > > > On Wed, Jun 17, 2020 at 12:31 AM Christoph Hellwig
> > > > > > wrote:
> > > > >
> > > > > ...
> > > > >
> > > > > > (and likely call it "external" instead of "untrusted".
> > > > >
> > > > > Which is not okay. 'External' to what? 'untrusted' has been carefully
> > > > > chosen by the meaning of it.
> > > > > What external does mean for M.2. WWAN card in my laptop? It's in ACPI
> > > > > tables, but I can replace it.
> > > >
> > > > Then your ACPI tables should show this, there is an attribute for it,
> > > > right?
> > >
> > > There is a _PLD() method, but it's for the USB devices (or optional
> > > for others, I don't remember by heart). So, most of the ACPI tables,
> > > alas, don't show this.
> > >
> > > > > This is only one example. Or if firmware of some device is altered,
> > > > > and it's internal (whatever it means) is it trusted or not?
> > > >
> > > > That is what people are using policy for today, if you object to this,
> > > > please bring it up to those developers :)
> > >
> > > > > So, please leave it as is (I mean name).
> > > >
> > > > firmware today exports this attribute, why do you not want userspace to
> > > > also know it?
> >
> > To clarify, the attribute exposed by the firmware today is
> > "ExternalFacingPort" and "external-facing" respectively:
> >
> > 617654aae50e ("PCI / ACPI: Identify untrusted PCI devices")
> > 9cb30a71ac45d("PCI: OF: Support "external-facing" property")
> >
> > The kernel flag was named "untrusted" though, hence the assumption
> > that "external=untrusted" is currently baked into the kernel today.
> > IMHO, using "external" would fix that (The assumption can thus be
> > contained in the IOMMU drivers) and at the same time allow more use of
> > this attribute.
> >
> > > >
> > > > Trust is different, yes, don't get the two mixed up please. That should
> > > > be a different sysfs attribute for obvious reasons.
> > >
> > > Yes, as a bottom line that's what I meant as well.
> >
> > So what is the consensus here? I don't have a strong opinion - but it
> > seemed to me Greg is saying "external" and Andy is saying "untrusted"?
>
> Those two things are totally separate things when it comes to a device.
Agree that these are two separate attributes, and better not mixed.
>
> One (external) describes the location of the device in the system.
>
> The other (untrusted) describes what you want the kernel to do with this
> device (trust or not trust it).
>
> One you can change (from trust to untrusted or back), the other you can
> not, it is a fixed read-only property that describes the hardware device
> as defined by the firmware.
The genesis is due to lack of a mechanism to establish if the device
is trusted or not was the due lack of some specs and implementation around
Component Measurement And Authentication (CMA). Treating external as
untrusted was the best first effort. i.e trust internal
devices and don't trust external devices for enabling ATS.
But that said external is just describing topology, and if Linux wants to
use that in the policy that's different. Some day external device may also
use CMA to estabilish trust. FWIW even internal devices aren't trust
worthy, except maybe RCIEP's.
>
> Depending on the policy you wish to define, you can use the location of
> the device to determine if you want to trust the device or not.
>
Cheers,
Ashok
Re: [PATCH v5 6/9] media: adv748x: prepare/enable mclk when the audio is used
Hi Alex,
On 02/04/2020 19:34, Alex Riesen wrote:
> As there is nothing else (the consumers are supposed to do that) which
> enables the clock, do it in the driver.
>
> Signed-off-by: Alexander Riesen
> --
>
> v3: added
> ---
> drivers/media/i2c/adv748x/adv748x-dai.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c
> b/drivers/media/i2c/adv748x/adv748x-dai.c
> index c9191f8f1ca8..185f78023e91 100644
> --- a/drivers/media/i2c/adv748x/adv748x-dai.c
> +++ b/drivers/media/i2c/adv748x/adv748x-dai.c
> @@ -117,11 +117,22 @@ static int adv748x_dai_set_fmt(struct snd_soc_dai *dai,
> unsigned int fmt)
>
> static int adv748x_dai_startup(struct snd_pcm_substream *sub, struct
> snd_soc_dai *dai)
> {
> + int ret;
> struct adv748x_state *state = state_of(dai);
>
> if (sub->stream != SNDRV_PCM_STREAM_CAPTURE)
> return -EINVAL;
this looks quite bunched up so :
Newline...
> - return set_audio_pads_state(state, 1);
> + ret = set_audio_pads_state(state, 1);
> + if (ret)
> + goto fail;
With no action required to cleanup here, I would just
return ret;
and remove the fail: label.
Newline...
> + ret = clk_prepare_enable(mclk_of(state));
> + if (ret)
> + goto fail_pwdn;
newline...
> + return 0;
newline...
Other than that:
Reviewed-by: Kieran Bingham
> +fail_pwdn:
> + set_audio_pads_state(state, 0);
> +fail:
> + return ret;
> }
>
> static int adv748x_dai_hw_params(struct snd_pcm_substream *sub,
> @@ -174,6 +185,7 @@ static void adv748x_dai_shutdown(struct snd_pcm_substream
> *sub, struct snd_soc_d
> {
> struct adv748x_state *state = state_of(dai);
>
> + clk_disable_unprepare(mclk_of(state));
> set_audio_pads_state(state, 0);
> }
>
>
Re: [PATCH -next] lib: fix test_hmm.c reference after free
On 6/17/20 10:31 PM, Randy Dunlap wrote: From: Randy Dunlap Coccinelle scripts report the following errors: lib/test_hmm.c:523:20-26: ERROR: reference preceded by free on line 521 lib/test_hmm.c:524:21-27: ERROR: reference preceded by free on line 521 lib/test_hmm.c:523:28-35: ERROR: devmem is NULL but dereferenced. lib/test_hmm.c:524:29-36: ERROR: devmem is NULL but dereferenced. Fix these by using the local variable 'res' instead of devmem. Signed-off-by: Randy Dunlap Cc: Jérôme Glisse Cc: linux...@kvack.org Cc: Ralph Campbell --- lib/test_hmm.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- linux-next-20200617.orig/lib/test_hmm.c +++ linux-next-20200617/lib/test_hmm.c @@ -520,8 +520,7 @@ static bool dmirror_allocate_chunk(struc err_free: kfree(devmem); err_release: - release_mem_region(devmem->pagemap.res.start, - resource_size(>pagemap.res)); + release_mem_region(res->start, resource_size(res)); err: mutex_unlock(>devmem_lock); return false; Thanks for fixing this! Reviewed-by: Ralph Campbell
[PATCH 1/2] sched: fix thread_union::task visibility
When CONFIG_ARCH_TASK_STRUCT_ON_STACK=y (i.e. ARCH=ia64), task_struct
and the thread stack are shared.
The ifdef condition of CONFIG_ARCH_TASK_STRUCT_ON_STACK is opposite.
Now that the init thread stack is constructed by the linker script,
this is not a practical problem, but let's fix the code just in case.
Fixes: 0500871f21b2 ("Construct init thread stack in the linker script rather
than by union")
Signed-off-by: Masahiro Yamada
---
include/linux/sched.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index b62e6aaf28f0..6d6c4d38c063 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1674,7 +1674,7 @@ extern void ia64_set_curr_task(int cpu, struct
task_struct *p);
void yield(void);
union thread_union {
-#ifndef CONFIG_ARCH_TASK_STRUCT_ON_STACK
+#ifdef CONFIG_ARCH_TASK_STRUCT_ON_STACK
struct task_struct task;
#endif
#ifndef CONFIG_THREAD_INFO_IN_TASK
--
2.25.1
Re: [PATCH v3 20/21] dyndbg: add user-flag, negating-flags, and filtering on flags
On Wed 2020-06-17 10:25:35, Jim Cromie wrote: > 1. Add a user-flag [u] which works like the [pfmlt] flags, but has no > effect on callsite behavior; it allows incremental marking of > arbitrary sets of callsites. > > 2. Add [PFMLTU] flags, which negate their counterparts; P===!p etc. > And in ddebug_read_flags(): >current code does: [pfmltu_] -> flags >copy it to:[PFMLTU_] -> mask > > also disallow both of a pair: ie no 'pP', no true & false. > > 3. Add filtering ops into ddebug_change(), right after all the > callsite-property selections are complete. These filter on the > callsite's current flagstate before applying modflags. > > Why ? > > The u-flag & filter flags > > The 'u' flag lets the user assemble an arbitary set of callsites. > Then using filter flags, user can activate the 'u' callsite set. > > #> echo 'file foo.c +u; file bar.c +u' > control # and repeat > #> echo 'u+p' > control > > Of course, you can continue to just activate your set without ever > marking it 1st, but you could trivially add the markup as you go, then > be able to use it as a constraint later, to undo or modify your set. > > #> echo 'file foo.c +up' >control > .. monitor, debug, finish .. > #> echo 'u-p' >control > > # then later resume > #> echo 'u+p' >control > > # disable some cluttering messages, and remove from u-set > #> echo 'file noisy.c function:jabber_* u-pu' >control > > # for doc, recollection > grep =pu control > my-favorite-callsites > > Note: > > Your flagstate after boot is generally not all =_. -DDEBUG will arm > compiled callsites by default, $builtinmod.dyndbg=+p bootargs can > enable them early, and $module.dyndbg=+p bootargs will arm them when > the module is loaded. But you could manage them with u-flags: > > #> echo '-t' >control # clear t-flag to use it as 2ndary > markup > #> echo 'p+ut' >control # mark the boot-enabled set of callsites > #> echo '-p' >control # clean your dmesg -w stream > > ... monitor, debug .. > #> echo 'module of_interest $qterms +pu' >control # build your set of > useful debugs > #> echo 'module of_interest $qterms UT+pu' >control # same, but dont alter > ut marked set Does anyone requested this feature, please? For me, it is really hard to imagine people using these complex and hacky steps. Not to say that using t-flag as a markup looks like a real hack. People either always need the line number in the kernel log or they do not need it at all. Let me repeat. Please, stop this non-sense. Best Regards, Petr
Re: [PATCH] sparse: use identifiers to define address spaces
Hi Luc, I love your patch! Perhaps something to improve: [auto build test WARNING on linux/master] [also build test WARNING on linus/master v5.8-rc1 next-20200618] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Luc-Van-Oostenryck/sparse-use-identifiers-to-define-address-spaces/20200618-060614 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a5dc8300df75e8b8384b4c82225f1e4a0b4d9b55 config: mips-randconfig-s031-20200618 (attached as .config) compiler: mips64-linux-gcc (GCC) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-rc1-10-gc17b1b06-dirty # save the attached .config to linux build tree make W=1 C=1 ARCH=mips CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot sparse warnings: (new ones prefixed by >>) >> arch/mips/include/asm/syscall.h:78:25: sparse: sparse: incorrect type in >> initializer (different address spaces) @@ expected int const [noderef] >> __user *__gu_ptr @@ got int * @@ >> arch/mips/include/asm/syscall.h:78:25: sparse: expected int const >> [noderef] __user *__gu_ptr arch/mips/include/asm/syscall.h:78:25: sparse: got int * >> arch/mips/include/asm/syscall.h:78:25: sparse: sparse: incorrect type in >> initializer (different address spaces) @@ expected int const [noderef] >> __user *__gu_ptr @@ got int * @@ >> arch/mips/include/asm/syscall.h:78:25: sparse: expected int const >> [noderef] __user *__gu_ptr arch/mips/include/asm/syscall.h:78:25: sparse: got int * -- >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: incorrect type in argument >> 1 (different address spaces) @@ expected void const volatile [noderef] >> __user * @@ got unsigned int [usertype] * @@ >> arch/mips/kernel/signal.c:280:13: sparse: expected void const volatile >> [noderef] __user * arch/mips/kernel/signal.c:280:13: sparse: got unsigned int [usertype] * >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression >> arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space >> '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression >> arch/mips/kernel/signal.c:293:23: sparse: sparse: incorrect type in argument >> 1 (different address spaces) @@ expected void const volatile [noderef] >> __user * @@ got unsigned int * @@ arch/mips/kernel/sign
[PATCH v2] ARM: dts: imx6qdl-gw: add Gateworks System Controller support
Add Gateworks System Controller support to Gateworks Ventana boards:
- add dt bindings for GSC mfd driver and hwmon driver for ADC's and
fan controllers.
- add dt bindings for gpio-keys driver for push-button and interrupt events
Signed-off-by: Tim Harvey
---
v2:
- use keycode bindings from linux-event-codes.h
- fix gw5910/gw5913 vdd_bat ADC mode (these boards use 16bit pre-scaled ADC)
---
arch/arm/boot/dts/imx6qdl-gw51xx.dtsi | 153 +--
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 159 ++--
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 165 +++--
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 167 --
arch/arm/boot/dts/imx6qdl-gw551x.dtsi | 147 --
arch/arm/boot/dts/imx6qdl-gw552x.dtsi | 153 +--
arch/arm/boot/dts/imx6qdl-gw553x.dtsi | 141 +++-
arch/arm/boot/dts/imx6qdl-gw560x.dtsi | 164 +++--
arch/arm/boot/dts/imx6qdl-gw5903.dtsi | 140 +++-
arch/arm/boot/dts/imx6qdl-gw5904.dtsi | 141 +++-
arch/arm/boot/dts/imx6qdl-gw5907.dtsi | 142 -
arch/arm/boot/dts/imx6qdl-gw5910.dtsi | 160 +++-
arch/arm/boot/dts/imx6qdl-gw5912.dtsi | 147 +-
arch/arm/boot/dts/imx6qdl-gw5913.dtsi | 153 ++-
14 files changed, 2075 insertions(+), 57 deletions(-)
diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
index 419a7cd..712458d 100644
--- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
@@ -4,6 +4,7 @@
*/
#include
+#include
/ {
/* these are used by bootloader for disabling nodes */
@@ -19,6 +20,53 @@
bootargs = "console=ttymxc1,115200";
};
+ gpio_keys {
+ compatible = "gpio-keys";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ user_pb {
+ label = "user_pb";
+ gpios = <_gpio 0 GPIO_ACTIVE_LOW>;
+ linux,code = ;
+ };
+
+ user_pb1x {
+ label = "user_pb1x";
+ linux,code = ;
+ interrupt-parent = <>;
+ interrupts = <0>;
+ };
+
+ key_erased {
+ label = "key-erased";
+ linux,code = ;
+ interrupt-parent = <>;
+ interrupts = <1>;
+ };
+
+ eeprom_wp {
+ label = "eeprom_wp";
+ linux,code = ;
+ interrupt-parent = <>;
+ interrupts = <2>;
+ };
+
+ tamper {
+ label = "tamper";
+ linux,code = ;
+ interrupt-parent = <>;
+ interrupts = <5>;
+ };
+
+ switch_hold {
+ label = "switch_hold";
+ linux,code = ;
+ interrupt-parent = <>;
+ interrupts = <7>;
+ };
+ };
+
leds {
compatible = "gpio-leds";
pinctrl-names = "default";
@@ -102,6 +150,103 @@
pinctrl-0 = <_i2c1>;
status = "okay";
+ gsc: gsc@20 {
+ compatible = "gw,gsc";
+ reg = <0x20>;
+ interrupt-parent = <>;
+ interrupts = <4 GPIO_ACTIVE_LOW>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ #size-cells = <0>;
+
+ adc {
+ compatible = "gw,gsc-adc";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ gw,mode = <0>;
+ reg = <0x00>;
+ label = "temp";
+ };
+
+ channel@2 {
+ gw,mode = <1>;
+ reg = <0x02>;
+ label = "vdd_vin";
+ };
+
+ channel@5 {
+ gw,mode = <1>;
+ reg = <0x05>;
+ label = "vdd_3p3";
+ };
+
+ channel@8 {
+ gw,mode = <1>;
+ reg = <0x08>;
+ label = "vdd_bat";
+ };
+
+ channel@b {
+ gw,mode = <1>;
+ reg = <0x0b>;
+
[PATCH 2/2] init: annotate init_task as __init_task_data for all arches
__init_task_data is no-op when CONFIG_ARCH_TASK_STRUCT_ON_STACK=n,
so you can always annotate init_task as __init_task_data.
Signed-off-by: Masahiro Yamada
---
init/init_task.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/init/init_task.c b/init/init_task.c
index 15089d15010a..0110b2941c4d 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -61,11 +61,7 @@ unsigned long init_shadow_call_stack[SCS_SIZE / sizeof(long)]
* Set up the first task table, touch at your own risk!. Base=0,
* limit=0x1f (=2MB)
*/
-struct task_struct init_task
-#ifdef CONFIG_ARCH_TASK_STRUCT_ON_STACK
- __init_task_data
-#endif
-= {
+struct task_struct init_task __init_task_data = {
#ifdef CONFIG_THREAD_INFO_IN_TASK
.thread_info= INIT_THREAD_INFO(init_task),
.stack_refcount = REFCOUNT_INIT(1),
--
2.25.1
Re: [PATCH net] net: dsa: bcm_sf2: Fix node reference count
On 6/18/2020 5:56 AM, Andrew Lunn wrote:
> On Wed, Jun 17, 2020 at 08:42:44PM -0700, Florian Fainelli wrote:
>> of_find_node_by_name() will do an of_node_put() on the "from" argument.
>
>> Fixes: afa3b592953b ("net: dsa: bcm_sf2: Ensure correct sub-node is parsed")
>> Signed-off-by: Florian Fainelli
>> ---
>> drivers/net/dsa/bcm_sf2.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
>> index c1bd21e4b15c..9f62ba3e4345 100644
>> --- a/drivers/net/dsa/bcm_sf2.c
>> +++ b/drivers/net/dsa/bcm_sf2.c
>> @@ -1154,6 +1154,8 @@ static int bcm_sf2_sw_probe(struct platform_device
>> *pdev)
>> set_bit(0, priv->cfp.used);
>> set_bit(0, priv->cfp.unique);
>>
>> +/* Balance of_node_put() done by of_find_node_by_name() */
>> +of_node_get(dn);
>> ports = of_find_node_by_name(dn, "ports");
>
> That if_find_node_by_name() does a put is not very intuitive.
> Maybe document that as well in the kerneldocs?
Yes that is the plan, most callers call it with a NULL from argument but
that is a bit silly if you know what the Device Tree looks like, you can
search quicker to the target node. Thanks.
>
> Reviewed-by: Andrew Lunn
>
> Andrew
>
--
Florian
Re: [PATCH v5 7/9] media: adv748x: only activate DAI if it is described in device tree
Hi Alex,
On 02/04/2020 19:34, Alex Riesen wrote:
> To avoid setting it up even if the hardware is not actually connected
> to anything physically.
>
> Besides, the bindings explicitly notes that port definitions are
> "optional if they are not connected to anything at the hardware level".
>
> Signed-off-by: Alexander Riesen
> ---
> drivers/media/i2c/adv748x/adv748x-dai.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c
> b/drivers/media/i2c/adv748x/adv748x-dai.c
> index 185f78023e91..f9cc47fa9ad1 100644
> --- a/drivers/media/i2c/adv748x/adv748x-dai.c
> +++ b/drivers/media/i2c/adv748x/adv748x-dai.c
> @@ -216,6 +216,11 @@ int adv748x_dai_init(struct adv748x_dai *dai)
> int ret;
> struct adv748x_state *state = adv748x_dai_to_state(dai);
>
> + if (!state->endpoints[ADV748X_PORT_I2S]) {
> + adv_info(state, "no I2S port, DAI disabled\n");
> + ret = 0;
> + goto fail;
How about just 'return 0'?
> + }
And a blank line here ...
Otherwise,
Reviewed-by: Kieran Bingham
This could also be folded into 5/9 too I guess?, though it is easier to
review the sequential additions, rather than the one-big-feature
addition ;-)
> dai->mclk_name = kasprintf(GFP_KERNEL, "%s.%s-i2s-mclk",
> state->dev->driver->name,
> dev_name(state->dev));
>
--
Regards
--
Kieran
Re: [PATCH v6 6/7] clk: mediatek: add UART0 clock support
On Thu, 2020-06-18 at 17:51 +0200, Matthias Brugger wrote:
>
> On 18/06/2020 13:33, Hanks Chen wrote:
> > Add MT6779 UART0 clock support.
> >
>
> Please a dd fixes tag:
>
> Fixes: 710774e04861 ("clk: mediatek: Add MT6779 clock support")
Got it, I'll add it in next version.
>
> > Signed-off-by: Hanks Chen
> > Signed-off-by: mtk01761
>
> Must be a real name not "mtk01761"
Oops, I'll update his name.
Thank you for your comment.
>
> > ---
> > drivers/clk/mediatek/clk-mt6779.c |2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/clk/mediatek/clk-mt6779.c
> > b/drivers/clk/mediatek/clk-mt6779.c
> > index 9766ccc..6e0d3a1 100644
> > --- a/drivers/clk/mediatek/clk-mt6779.c
> > +++ b/drivers/clk/mediatek/clk-mt6779.c
> > @@ -919,6 +919,8 @@
> > "pwm_sel", 19),
> > GATE_INFRA0(CLK_INFRA_PWM, "infra_pwm",
> > "pwm_sel", 21),
> > + GATE_INFRA0(CLK_INFRA_UART0, "infra_uart0",
> > + "uart_sel", 22),
> > GATE_INFRA0(CLK_INFRA_UART1, "infra_uart1",
> > "uart_sel", 23),
> > GATE_INFRA0(CLK_INFRA_UART2, "infra_uart2",
> >
