Re: [RFC 0/1] mconf: Emacs-like isearch
Masahiro Yamada writes: > 2018-06-07 8:54 GMT+09:00 Dirk Gouders : >> Randy Dunlap writes: >> >>> On 06/06/2018 03:32 PM, Dirk Gouders wrote: Randy Dunlap writes: > On 06/06/2018 02:56 PM, Dirk Gouders wrote: >> Hello, >> >> being an Emacs user, I frequently find myself pressing CTRL-s in mconf >> to search for some menu entry, especially in large menus. > > > I use Emacs, but I have never typed Ctrl-s in menuconfig. > > Is it important to use the same key pattern as in Emacs? > > You intercepted Ctrl-* > > Currently, Ctrl-C terminates the program, > but this patch makes it no-op. Yes, Segher also pointed out that I at least should have mentioned this in the cover letter. I simply forgot that, but I was aware that forced raw-mode probably is not acceptable. I'd say it's not important to start isearch with CTRL-s, I am just so used to it. So, I am open for suggestions. Meanwhile, in a V2 I will try to stay with CTRL-s (and probably another unused simpler key) but also create a possibility to toggle raw-mode with default off. >> I decided to implement a basic isearch in mconf and would like to hear >> if others find this functionality useful, as well. >> >> The new functionality is started with pressing CTRL-s followed by >> characters that form the search string. To search for further >> occurences of an entered string, press CTRL-s instead of further >> characters. >> >> For example: to navigate to the USB device drivers, press CTRL-s de >> ENTER ENTER usb ENTER ENTER > > Not being an emacs user, what is the "de" for above? "de" (with my .config) causes a match for "Device Drivers" -- no other menu entry matching the string "de" is befor that entry. >>> >>> Device Drivers is the first match for me also. >>> >>> To get to the USB drivers, I have to enter: >>> CTRL-s de ENTER ENTER CTRL-s usb ENTER ENTER >> >> Yes, I left out the second CTRL-s, thank you! >> >> Oh well, I shouldn't have sent this late at night, then I would probably >> have explained the needed input as: >> >> 1) CTRL-s // start isearch >> 2) de // substring that matches "Device Drivers" >> 3) ENTER // quit isearch >> 4) ENTER // enter Device Drivers menu >> 5) CTRL-s // start isearch >> 6) usb // navigate to USB support >> 7) ENTER // quit isearch >> 8) ENTER // enter USB support menu > > > Hmm. > > I tried this, but I was a bit annoyed. > > > I wonder if this could be more user-friendly. > > For example, I want KEY_UP/DOWN/LEFT/RIGHT > to quit the search mode without pressing ENTER. Thank you for testing it. Concerning annoyance, did you mean the isearch functionality itself or the incomplete implementation, i.e. the missing possibility to use above keys? I will handle the above keys in V2, additionally to fixing a problem with mismatches. Dirk
Re: [RFC 0/1] mconf: Emacs-like isearch
Masahiro Yamada writes: > 2018-06-07 8:54 GMT+09:00 Dirk Gouders : >> Randy Dunlap writes: >> >>> On 06/06/2018 03:32 PM, Dirk Gouders wrote: Randy Dunlap writes: > On 06/06/2018 02:56 PM, Dirk Gouders wrote: >> Hello, >> >> being an Emacs user, I frequently find myself pressing CTRL-s in mconf >> to search for some menu entry, especially in large menus. > > > I use Emacs, but I have never typed Ctrl-s in menuconfig. > > Is it important to use the same key pattern as in Emacs? > > You intercepted Ctrl-* > > Currently, Ctrl-C terminates the program, > but this patch makes it no-op. Yes, Segher also pointed out that I at least should have mentioned this in the cover letter. I simply forgot that, but I was aware that forced raw-mode probably is not acceptable. I'd say it's not important to start isearch with CTRL-s, I am just so used to it. So, I am open for suggestions. Meanwhile, in a V2 I will try to stay with CTRL-s (and probably another unused simpler key) but also create a possibility to toggle raw-mode with default off. >> I decided to implement a basic isearch in mconf and would like to hear >> if others find this functionality useful, as well. >> >> The new functionality is started with pressing CTRL-s followed by >> characters that form the search string. To search for further >> occurences of an entered string, press CTRL-s instead of further >> characters. >> >> For example: to navigate to the USB device drivers, press CTRL-s de >> ENTER ENTER usb ENTER ENTER > > Not being an emacs user, what is the "de" for above? "de" (with my .config) causes a match for "Device Drivers" -- no other menu entry matching the string "de" is befor that entry. >>> >>> Device Drivers is the first match for me also. >>> >>> To get to the USB drivers, I have to enter: >>> CTRL-s de ENTER ENTER CTRL-s usb ENTER ENTER >> >> Yes, I left out the second CTRL-s, thank you! >> >> Oh well, I shouldn't have sent this late at night, then I would probably >> have explained the needed input as: >> >> 1) CTRL-s // start isearch >> 2) de // substring that matches "Device Drivers" >> 3) ENTER // quit isearch >> 4) ENTER // enter Device Drivers menu >> 5) CTRL-s // start isearch >> 6) usb // navigate to USB support >> 7) ENTER // quit isearch >> 8) ENTER // enter USB support menu > > > Hmm. > > I tried this, but I was a bit annoyed. > > > I wonder if this could be more user-friendly. > > For example, I want KEY_UP/DOWN/LEFT/RIGHT > to quit the search mode without pressing ENTER. Thank you for testing it. Concerning annoyance, did you mean the isearch functionality itself or the incomplete implementation, i.e. the missing possibility to use above keys? I will handle the above keys in V2, additionally to fixing a problem with mismatches. Dirk
Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
On Wed 06 Jun 22:29 PDT 2018, Sricharan R wrote: > Hi Bjorn, > > On 6/7/2018 9:54 AM, Bjorn Andersson wrote: > > On Wed 06 Jun 21:11 PDT 2018, Vinod wrote: > > > >> On 06-06-18, 09:17, Bjorn Andersson wrote: > >>> On Tue 05 Jun 05:56 PDT 2018, Sricharan R wrote: > >>> > Hi Vinod, > > On 6/5/2018 11:49 AM, Vinod wrote: > > On 05-06-18, 11:12, Sricharan R wrote: [..] > > If we ignore SMD for a while we have the following combinations: > > > > glink/wcss > > y y - valid > > y m - valid > > y n - valid > > m y - link failure (invalid) > > m m - valid > > m n - valid > > n y - valid (platform uses wcss, but not glink) > > n m - valid (-"-) > > n n - valid > > > > So to distill this we have the two valid cases: > > module/no if RPMSG_QCOM_GLINK_SMEM=m > > yes/module/no if RPMSG_QCOM_GLINK_SMEM=y > > > > and the way you express that in Kconfig is the somewhat awkward > > > > depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > > > > ok, Having "depends on RPMSG_QCOM_GLINK_SMEM" takes care of the > first 6 cases in the above list. > > But just was thinking that by allowing the last three combinations, > there is a chance that some config that really needs GLINK_SMEM and WCSS, > but selects only Q6V5_WCSS and misses to select GLINK_SMEM, > would still built and make it non-functional, right ? > It would allow you to compile a kernel with GLINk disabled that in runtime loads a firmware that depends on GLINK being there. As it would be convenient to thereby state that "WCSS always depends on GLINK" we can make the analog to 410 where "MSS always depends on SMD", which isn't true when the same driver is reused on e.g. 845 - which uses GLINK. So my recommendation is that we stick with Kconfig options that describes the build time dependencies of this particular driver, rather than its runtime dependencies in a particular platform. Regards, Bjorn
Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
On Wed 06 Jun 22:29 PDT 2018, Sricharan R wrote: > Hi Bjorn, > > On 6/7/2018 9:54 AM, Bjorn Andersson wrote: > > On Wed 06 Jun 21:11 PDT 2018, Vinod wrote: > > > >> On 06-06-18, 09:17, Bjorn Andersson wrote: > >>> On Tue 05 Jun 05:56 PDT 2018, Sricharan R wrote: > >>> > Hi Vinod, > > On 6/5/2018 11:49 AM, Vinod wrote: > > On 05-06-18, 11:12, Sricharan R wrote: [..] > > If we ignore SMD for a while we have the following combinations: > > > > glink/wcss > > y y - valid > > y m - valid > > y n - valid > > m y - link failure (invalid) > > m m - valid > > m n - valid > > n y - valid (platform uses wcss, but not glink) > > n m - valid (-"-) > > n n - valid > > > > So to distill this we have the two valid cases: > > module/no if RPMSG_QCOM_GLINK_SMEM=m > > yes/module/no if RPMSG_QCOM_GLINK_SMEM=y > > > > and the way you express that in Kconfig is the somewhat awkward > > > > depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > > > > ok, Having "depends on RPMSG_QCOM_GLINK_SMEM" takes care of the > first 6 cases in the above list. > > But just was thinking that by allowing the last three combinations, > there is a chance that some config that really needs GLINK_SMEM and WCSS, > but selects only Q6V5_WCSS and misses to select GLINK_SMEM, > would still built and make it non-functional, right ? > It would allow you to compile a kernel with GLINk disabled that in runtime loads a firmware that depends on GLINK being there. As it would be convenient to thereby state that "WCSS always depends on GLINK" we can make the analog to 410 where "MSS always depends on SMD", which isn't true when the same driver is reused on e.g. 845 - which uses GLINK. So my recommendation is that we stick with Kconfig options that describes the build time dependencies of this particular driver, rather than its runtime dependencies in a particular platform. Regards, Bjorn
Re: [PATCH] printk/nmi: Prevent deadlock when serializing NMI backtraces
On (06/06/18 13:15), Petr Mladek wrote: > On Wed 2018-06-06 14:10:29, Sergey Senozhatsky wrote: > > On (06/05/18 14:47), Petr Mladek wrote: > > [..] > > > Grr, the ABBA deadlock is still there. NMIs are not sent to the other > > > CPUs atomically. Even if we detect that logbuf_lock is available > > > in printk_nmi_enter() on some CPUs, it might still get locked on > > > another CPU before the other CPU gets NMI. > > > > Can we do something about "B"? :) I mean - any chance we can rework > > locking in nmi_cpu_backtrace()? > > I can't think of any possibility at the moment. Well, it does not mean > that it does not exist. > > The irony is that we need the extra lock in nmi_cpu_backtrace() only > because we try to store the messages into the common log buffer. > If we always use the per-CPU buffers in NMI then the lock would > not cause any problems but it also won't be necessary. Yep. I think we can come up with something. It seems that the only problem is that we want in this particular case to avoid printk_nmi()->logbuf and instead we want to enforce per-CPU printk_nmi buffers. Then we can drop nmi_cpu_backtrace() spinlock, because the messages will be serialized by printk_safe flush spin_lock. That doesn't sound like an impossible thing to do. What am I missing? Could you please check my follow up email? > > > => I suggest to revert the commit 719f6a7040f1bdaf96fcc70 > > > "printk: Use the main logbuf in NMI when logbuf_lock is available" > > > for-4.18 and stable until we get a better solution. > > > > Just random thoughts. > > > > May be we need to revert it, but let's not "panic". I think [but don't > > insist on it] that the patch in question is *probably* "good enough". It > > addresses a bug report after all. > > It was a problem reported by me. I found it when testing other changes. > The patch improved the situation definitely. The question is if it is > enough in practice. Oh, certainly. But I was talking about 719f6a7040f1bdaf96fcc70. We introduced that change in response to a bug report from Steven. He would not be able to debug his kernel otherwise, because per-CPU printk_nmi was too limited in size. So on one hand we have the problem that you reported, which you found while you were hammering/testing NMI printk-s [a valid report on its own]; on the other hand we have the problem that Steven reported, which he triggered while he was debugging the kernel. It might be the case that Steven's problem is more likely to happen in real world. So that's why I proposed to keep 719f6a7040f1bda for the time being [until we come up with another fix]. I may be wrong. -ss
Re: [PATCH] printk/nmi: Prevent deadlock when serializing NMI backtraces
On (06/06/18 13:15), Petr Mladek wrote: > On Wed 2018-06-06 14:10:29, Sergey Senozhatsky wrote: > > On (06/05/18 14:47), Petr Mladek wrote: > > [..] > > > Grr, the ABBA deadlock is still there. NMIs are not sent to the other > > > CPUs atomically. Even if we detect that logbuf_lock is available > > > in printk_nmi_enter() on some CPUs, it might still get locked on > > > another CPU before the other CPU gets NMI. > > > > Can we do something about "B"? :) I mean - any chance we can rework > > locking in nmi_cpu_backtrace()? > > I can't think of any possibility at the moment. Well, it does not mean > that it does not exist. > > The irony is that we need the extra lock in nmi_cpu_backtrace() only > because we try to store the messages into the common log buffer. > If we always use the per-CPU buffers in NMI then the lock would > not cause any problems but it also won't be necessary. Yep. I think we can come up with something. It seems that the only problem is that we want in this particular case to avoid printk_nmi()->logbuf and instead we want to enforce per-CPU printk_nmi buffers. Then we can drop nmi_cpu_backtrace() spinlock, because the messages will be serialized by printk_safe flush spin_lock. That doesn't sound like an impossible thing to do. What am I missing? Could you please check my follow up email? > > > => I suggest to revert the commit 719f6a7040f1bdaf96fcc70 > > > "printk: Use the main logbuf in NMI when logbuf_lock is available" > > > for-4.18 and stable until we get a better solution. > > > > Just random thoughts. > > > > May be we need to revert it, but let's not "panic". I think [but don't > > insist on it] that the patch in question is *probably* "good enough". It > > addresses a bug report after all. > > It was a problem reported by me. I found it when testing other changes. > The patch improved the situation definitely. The question is if it is > enough in practice. Oh, certainly. But I was talking about 719f6a7040f1bdaf96fcc70. We introduced that change in response to a bug report from Steven. He would not be able to debug his kernel otherwise, because per-CPU printk_nmi was too limited in size. So on one hand we have the problem that you reported, which you found while you were hammering/testing NMI printk-s [a valid report on its own]; on the other hand we have the problem that Steven reported, which he triggered while he was debugging the kernel. It might be the case that Steven's problem is more likely to happen in real world. So that's why I proposed to keep 719f6a7040f1bda for the time being [until we come up with another fix]. I may be wrong. -ss
Re: [PATCH 42/46] perf script powerpc: Python script for hypervisor call statistics
Hi Paul, On 06/06/2018 08:23 PM, Paul Clarke wrote: > On 06/05/2018 12:50 PM, Arnaldo Carvalho de Melo wrote: >> From: Ravi Bangoria >> >> Add python script to show hypervisor call statistics. Ex, >> >> # perf record -a -e "{powerpc:hcall_entry,powerpc:hcall_exit}" >> # perf script -s scripts/python/powerpc-hcalls.py >> hcallcount min(ns) max(ns) avg(ns) >> >> H_RANDOM82 838 1164 904 >> H_PUT_TCE 47 1078 5928 2003 >> H_EOI 266 1336 3546 1654 >> H_ENTER 28 1646 4038 1952 >> H_PUT_TCE_INDIRECT 230 2166 18168 6109 >> H_IPI 238 1072 3232 1688 >> H_SEND_LOGICAL_LAN 42 5488 21366 7694 >> H_STUFF_TCE294 986 6210 3591 >> H_XIRR 266 2286 6990 3783 >> H_PROTECT 10 2196 3556 2555 >> H_VIO_SIGNAL 294 1028 2784 1311 >> H_ADD_LOGICAL_LAN_BUFFER53 1978 3450 2600 >> H_SEND_CRQ 77 1762 7240 2447 > > This translation from HCALL code to mnemonic is more generally useful. Is > there a good way to make the "hcall_table_lookup" function more generally > available, like "syscall_name" in > scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py ? It's even simpler > than the syscall ID-to-name mapping, because the HCALL codes are constant, > unlike syscall IDs which vary between arches. > Right, but I don't see any other python script for hcalls. So, I thought to add it in the script itself. Let me know if you are planning to add any :) We will move it in a generic module. Ravi
Re: [PATCH 42/46] perf script powerpc: Python script for hypervisor call statistics
Hi Paul, On 06/06/2018 08:23 PM, Paul Clarke wrote: > On 06/05/2018 12:50 PM, Arnaldo Carvalho de Melo wrote: >> From: Ravi Bangoria >> >> Add python script to show hypervisor call statistics. Ex, >> >> # perf record -a -e "{powerpc:hcall_entry,powerpc:hcall_exit}" >> # perf script -s scripts/python/powerpc-hcalls.py >> hcallcount min(ns) max(ns) avg(ns) >> >> H_RANDOM82 838 1164 904 >> H_PUT_TCE 47 1078 5928 2003 >> H_EOI 266 1336 3546 1654 >> H_ENTER 28 1646 4038 1952 >> H_PUT_TCE_INDIRECT 230 2166 18168 6109 >> H_IPI 238 1072 3232 1688 >> H_SEND_LOGICAL_LAN 42 5488 21366 7694 >> H_STUFF_TCE294 986 6210 3591 >> H_XIRR 266 2286 6990 3783 >> H_PROTECT 10 2196 3556 2555 >> H_VIO_SIGNAL 294 1028 2784 1311 >> H_ADD_LOGICAL_LAN_BUFFER53 1978 3450 2600 >> H_SEND_CRQ 77 1762 7240 2447 > > This translation from HCALL code to mnemonic is more generally useful. Is > there a good way to make the "hcall_table_lookup" function more generally > available, like "syscall_name" in > scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py ? It's even simpler > than the syscall ID-to-name mapping, because the HCALL codes are constant, > unlike syscall IDs which vary between arches. > Right, but I don't see any other python script for hcalls. So, I thought to add it in the script itself. Let me know if you are planning to add any :) We will move it in a generic module. Ravi
Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
Hi Bjorn, On 6/7/2018 9:54 AM, Bjorn Andersson wrote: > On Wed 06 Jun 21:11 PDT 2018, Vinod wrote: > >> On 06-06-18, 09:17, Bjorn Andersson wrote: >>> On Tue 05 Jun 05:56 PDT 2018, Sricharan R wrote: >>> Hi Vinod, On 6/5/2018 11:49 AM, Vinod wrote: > On 05-06-18, 11:12, Sricharan R wrote: > >> +config QCOM_Q6V5_WCSS >> +tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader" >> +depends on OF && ARCH_QCOM >> +depends on QCOM_SMEM >> +depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n) >> +depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > > Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would > happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM > >>> >>> It says that QCOM_Q6V5_WCSS either must have a compatible state (i.e. >>> builtin vs builtin, module vs builtin, but not builtin vs module) or >>> that it's disabled, in which case we will hit the stub functions in >>> qcom_glink.h. >>> >>> I.e. this prevents QCOM_Q6V5_WCSS to be compiled builtin when >>> RPMSG_QCOM_GLINK_SMEM is module, as this would give us both stubs and >>> the module. >> >> IIUC, you want to have QCOM_Q6V5_WCSS and RPMSG_QCOM_GLINK_SMEM as >> modules or builtin >> > > RPMSG_QCOM_SMD, RPMSG_QCOM_GLINK_SMEM and QCOM_Q6V5_WCSS are all > tristate. > >> So, wouldn't Kconfig syntax something like where we say: >> M if RPMSG_QCOM_GLINK_SMEM=m >> bool if RPMSG_QCOM_GLINK_SMEM=y >> > > If we ignore SMD for a while we have the following combinations: > > glink/wcss > y y - valid > y m - valid > y n - valid > m y - link failure (invalid) > m m - valid > m n - valid > n y - valid (platform uses wcss, but not glink) > n m - valid (-"-) > n n - valid > > So to distill this we have the two valid cases: > module/no if RPMSG_QCOM_GLINK_SMEM=m > yes/module/no if RPMSG_QCOM_GLINK_SMEM=y > > and the way you express that in Kconfig is the somewhat awkward > > depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > ok, Having "depends on RPMSG_QCOM_GLINK_SMEM" takes care of the first 6 cases in the above list. But just was thinking that by allowing the last three combinations, there is a chance that some config that really needs GLINK_SMEM and WCSS, but selects only Q6V5_WCSS and misses to select GLINK_SMEM, would still built and make it non-functional, right ? Regards, Sricharan >> Which makes it clear that both these have to be same type? >> > > They don't have to be of the same type, only of a compatible type. > -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
Hi Bjorn, On 6/7/2018 9:54 AM, Bjorn Andersson wrote: > On Wed 06 Jun 21:11 PDT 2018, Vinod wrote: > >> On 06-06-18, 09:17, Bjorn Andersson wrote: >>> On Tue 05 Jun 05:56 PDT 2018, Sricharan R wrote: >>> Hi Vinod, On 6/5/2018 11:49 AM, Vinod wrote: > On 05-06-18, 11:12, Sricharan R wrote: > >> +config QCOM_Q6V5_WCSS >> +tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader" >> +depends on OF && ARCH_QCOM >> +depends on QCOM_SMEM >> +depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n) >> +depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > > Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would > happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM > >>> >>> It says that QCOM_Q6V5_WCSS either must have a compatible state (i.e. >>> builtin vs builtin, module vs builtin, but not builtin vs module) or >>> that it's disabled, in which case we will hit the stub functions in >>> qcom_glink.h. >>> >>> I.e. this prevents QCOM_Q6V5_WCSS to be compiled builtin when >>> RPMSG_QCOM_GLINK_SMEM is module, as this would give us both stubs and >>> the module. >> >> IIUC, you want to have QCOM_Q6V5_WCSS and RPMSG_QCOM_GLINK_SMEM as >> modules or builtin >> > > RPMSG_QCOM_SMD, RPMSG_QCOM_GLINK_SMEM and QCOM_Q6V5_WCSS are all > tristate. > >> So, wouldn't Kconfig syntax something like where we say: >> M if RPMSG_QCOM_GLINK_SMEM=m >> bool if RPMSG_QCOM_GLINK_SMEM=y >> > > If we ignore SMD for a while we have the following combinations: > > glink/wcss > y y - valid > y m - valid > y n - valid > m y - link failure (invalid) > m m - valid > m n - valid > n y - valid (platform uses wcss, but not glink) > n m - valid (-"-) > n n - valid > > So to distill this we have the two valid cases: > module/no if RPMSG_QCOM_GLINK_SMEM=m > yes/module/no if RPMSG_QCOM_GLINK_SMEM=y > > and the way you express that in Kconfig is the somewhat awkward > > depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > ok, Having "depends on RPMSG_QCOM_GLINK_SMEM" takes care of the first 6 cases in the above list. But just was thinking that by allowing the last three combinations, there is a chance that some config that really needs GLINK_SMEM and WCSS, but selects only Q6V5_WCSS and misses to select GLINK_SMEM, would still built and make it non-functional, right ? Regards, Sricharan >> Which makes it clear that both these have to be same type? >> > > They don't have to be of the same type, only of a compatible type. > -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
Re: [GIT PULL 00/46] perf/core fixes and improvements
* Arnaldo Carvalho de Melo wrote: > From: Arnaldo Carvalho de Melo > > Hi Ingo, > > Please consider pulling, > > - Arnaldo > > Test results at the end of this message, as usual. > > The following changes since commit 7869e5889477e4e32e4024d665431b35e8b7b693: > > Merge remote-tracking branch 'tip/perf/urgent' into perf/core (2018-06-04 > 10:28:20 -0300) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git > tags/perf-core-for-mingo-4.18-20180605 > > for you to fetch changes up to 03ac4e71cd120d2c3411d106d00d266114575f74: > > perf intel-pt: Fix "Unexpected indirect branch" error (2018-06-05 12:28:52 > -0300) > > > perf/core improvements and fixes: > > perf stat: > > . Display user and system time for workload targets (Jiri Olsa) > > perf record: > > . Enable arbitrary event names thru name= modifier (Alexey Budankov) > > PowerPC: > > . Add a python script for hypervisor call statistics (Ravi Bangoria) > > Intel PT: (Adrian Hunter) > > . Fix sync_switch INTEL_PT_SS_NOT_TRACING > > . Fix decoding to accept CBR between FUP and corresponding TIP > > . Fix MTC timing after overflow > > . Fix "Unexpected indirect branch" error > > perf test: > > . record+probe_libc_inet_pton: > > . To get the symbol table for dynamic > shared objects on ubuntu we need to pass the -D/--dynamic command line > option, unlike with the fedora distros (Arnaldo Carvalho de Melo) > > . code-reading: > > . Fix perf_env setup for PTI entry trampolines (Adrian Hunter) > > . kmod-path: > > . Add tests for vdso32 and vdsox32 (Adrian Hunter) > > . Use header file util/debug.h (Thomas Richter) > > perf annotate: > > . Make the various UI backends (stdio, TUI, gtk) use more consistently > structs with annotation options as specified by the user (Arnaldo Carvalho > de Melo) > > . Move annotation specific knobs from the symbol_conf global kitchen > sink to the annotation option structs (Arnaldo Carvalho de Melo) > > Core: > > . Fix misleading error for some unparsable events mentioning PMUs when > those are not involved in the problem (Jiri Olsa) > > - Fix symbol and object code resolution for vdso32 and vdsox32 (Adrian Hunter) > > . No need to check for null when passing pointers to foo__get() style > refcount grabbing helpers, just like in the kernel and with free(), > its safe to pass a NULL pointer to avoid having to check it before > each and every foo__get() call (Arnaldo Carvalho de Melo) > > . Remove some dead code (quote.[ch]) (Arnaldo Carvalho de Melo) > > . Remove some needless globals, making them local (Arnaldo Carvalho de Melo) > > . Reduce usage of symbol_conf.use_callchain, using other means of > finding out if callchains are in use or available for specific events, > as we evolved this codebase to allow requesting callchains for just > a subset of the monitored events. In time it will help polish > recording and showing mixed sets accross the various tools: > > perf record -e > cycles/call-graph=fp/,cache-misses/call-graph=dwarf/,instructions > > (Arnaldo Carvalho de Melo) > > . Consider PTI entry trampolines in map__rip_2objdump() (Adrian Hunter) > > Signed-off-by: Arnaldo Carvalho de Melo > > > Adrian Hunter (8): > perf tests kmod-path: Add tests for vdso32 and vdsox32 > perf tools: Fix symbol and object code resolution for vdso32 and vdsox32 > perf test code-reading: Fix perf_env setup for PTI entry trampolines > perf map: Consider PTI entry trampolines in rip_2objdump() > perf intel-pt: Fix sync_switch INTEL_PT_SS_NOT_TRACING > perf intel-pt: Fix decoding to accept CBR between FUP and corresponding > TIP > perf intel-pt: Fix MTC timing after overflow > perf intel-pt: Fix "Unexpected indirect branch" error > > Alexey Budankov (1): > perf record: Enable arbitrary event names thru name= modifier > > Arnaldo Carvalho de Melo (33): > perf tools: Remove dead quote.[ch] code > perf probe: Use return of map__get() to make code more compact > perf cgroup: Make evlist__find_cgroup() more compact > perf tools: No need to check if the argument to __get() function is NULL > perf annotate: Pass perf_evsel instead of just evsel->idx > perf annotate: __symbol__acount_cycles doesn't need notes > perf annotate: Split allocation of annotated_source struct > perf annotate: Introduce constructor/destructor for annotated_source > perf annotate: Introduce annotated_source__alloc_histograms > perf annotate: __symbol__inc_addr_samples() needs just annotated_source > perf annotate: Introduce symbol__hists() > perf annotate: Introduce symbol__cycle_hists() > perf annotate: Stop using symbol_conf.nr_events global in >
Re: [GIT PULL 00/46] perf/core fixes and improvements
* Arnaldo Carvalho de Melo wrote: > From: Arnaldo Carvalho de Melo > > Hi Ingo, > > Please consider pulling, > > - Arnaldo > > Test results at the end of this message, as usual. > > The following changes since commit 7869e5889477e4e32e4024d665431b35e8b7b693: > > Merge remote-tracking branch 'tip/perf/urgent' into perf/core (2018-06-04 > 10:28:20 -0300) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git > tags/perf-core-for-mingo-4.18-20180605 > > for you to fetch changes up to 03ac4e71cd120d2c3411d106d00d266114575f74: > > perf intel-pt: Fix "Unexpected indirect branch" error (2018-06-05 12:28:52 > -0300) > > > perf/core improvements and fixes: > > perf stat: > > . Display user and system time for workload targets (Jiri Olsa) > > perf record: > > . Enable arbitrary event names thru name= modifier (Alexey Budankov) > > PowerPC: > > . Add a python script for hypervisor call statistics (Ravi Bangoria) > > Intel PT: (Adrian Hunter) > > . Fix sync_switch INTEL_PT_SS_NOT_TRACING > > . Fix decoding to accept CBR between FUP and corresponding TIP > > . Fix MTC timing after overflow > > . Fix "Unexpected indirect branch" error > > perf test: > > . record+probe_libc_inet_pton: > > . To get the symbol table for dynamic > shared objects on ubuntu we need to pass the -D/--dynamic command line > option, unlike with the fedora distros (Arnaldo Carvalho de Melo) > > . code-reading: > > . Fix perf_env setup for PTI entry trampolines (Adrian Hunter) > > . kmod-path: > > . Add tests for vdso32 and vdsox32 (Adrian Hunter) > > . Use header file util/debug.h (Thomas Richter) > > perf annotate: > > . Make the various UI backends (stdio, TUI, gtk) use more consistently > structs with annotation options as specified by the user (Arnaldo Carvalho > de Melo) > > . Move annotation specific knobs from the symbol_conf global kitchen > sink to the annotation option structs (Arnaldo Carvalho de Melo) > > Core: > > . Fix misleading error for some unparsable events mentioning PMUs when > those are not involved in the problem (Jiri Olsa) > > - Fix symbol and object code resolution for vdso32 and vdsox32 (Adrian Hunter) > > . No need to check for null when passing pointers to foo__get() style > refcount grabbing helpers, just like in the kernel and with free(), > its safe to pass a NULL pointer to avoid having to check it before > each and every foo__get() call (Arnaldo Carvalho de Melo) > > . Remove some dead code (quote.[ch]) (Arnaldo Carvalho de Melo) > > . Remove some needless globals, making them local (Arnaldo Carvalho de Melo) > > . Reduce usage of symbol_conf.use_callchain, using other means of > finding out if callchains are in use or available for specific events, > as we evolved this codebase to allow requesting callchains for just > a subset of the monitored events. In time it will help polish > recording and showing mixed sets accross the various tools: > > perf record -e > cycles/call-graph=fp/,cache-misses/call-graph=dwarf/,instructions > > (Arnaldo Carvalho de Melo) > > . Consider PTI entry trampolines in map__rip_2objdump() (Adrian Hunter) > > Signed-off-by: Arnaldo Carvalho de Melo > > > Adrian Hunter (8): > perf tests kmod-path: Add tests for vdso32 and vdsox32 > perf tools: Fix symbol and object code resolution for vdso32 and vdsox32 > perf test code-reading: Fix perf_env setup for PTI entry trampolines > perf map: Consider PTI entry trampolines in rip_2objdump() > perf intel-pt: Fix sync_switch INTEL_PT_SS_NOT_TRACING > perf intel-pt: Fix decoding to accept CBR between FUP and corresponding > TIP > perf intel-pt: Fix MTC timing after overflow > perf intel-pt: Fix "Unexpected indirect branch" error > > Alexey Budankov (1): > perf record: Enable arbitrary event names thru name= modifier > > Arnaldo Carvalho de Melo (33): > perf tools: Remove dead quote.[ch] code > perf probe: Use return of map__get() to make code more compact > perf cgroup: Make evlist__find_cgroup() more compact > perf tools: No need to check if the argument to __get() function is NULL > perf annotate: Pass perf_evsel instead of just evsel->idx > perf annotate: __symbol__acount_cycles doesn't need notes > perf annotate: Split allocation of annotated_source struct > perf annotate: Introduce constructor/destructor for annotated_source > perf annotate: Introduce annotated_source__alloc_histograms > perf annotate: __symbol__inc_addr_samples() needs just annotated_source > perf annotate: Introduce symbol__hists() > perf annotate: Introduce symbol__cycle_hists() > perf annotate: Stop using symbol_conf.nr_events global in >
Re: possible deadlock in console_unlock
Cc-ing more people Link: lkml.kernel.org/r/87008b056df8f...@google.com On (06/06/18 06:17), syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:af6c5d5e01ad Merge branch 'for-4.18' of git://git.kernel.o.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=173d93ef80 > kernel config: https://syzkaller.appspot.com/x/.config?x=12ff770540994680 > dashboard link: https://syzkaller.appspot.com/bug?extid=43e93968b964e369db0b > compiler: gcc (GCC) 8.0.1 20180413 (experimental) > userspace arch: i386 > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=16e00bb780 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+43e93968b964e369d...@syzkaller.appspotmail.com Thanks a ton! So tty ioctl is known to be printk-deadlock prone and we don't know how to handle this in printk, as of now. ioctl tty_ioctl tty_port->lock printk call_console_driver console_driver uart_port->lock The problem is that call_console_driver->console_driver also can do this thing uart_port->lock tty_wakeup tty_port->lock So we can have the following: ioctl tty_ioctl tty_port->lock printk call_console_driver console_driver uart_port->lock tty_wakeup tty_port->lock << deadlock But lockdep is complaining about another scenario: > the existing dependency chain (in reverse order) is: > > -> #2 (&(>lock)->rlock){-.-.}: >__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] >_raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152 >tty_port_tty_get+0x20/0x80 drivers/tty/tty_port.c:288 >tty_port_default_wakeup+0x15/0x40 drivers/tty/tty_port.c:47 >tty_port_tty_wakeup+0x5d/0x70 drivers/tty/tty_port.c:390 >uart_write_wakeup+0x44/0x60 drivers/tty/serial/serial_core.c:103 >serial8250_tx_chars+0x4be/0xb60 > drivers/tty/serial/8250/8250_port.c:1808 >serial8250_handle_irq.part.25+0x1ee/0x280 > drivers/tty/serial/8250/8250_port.c:1881 >serial8250_handle_irq drivers/tty/serial/8250/8250_port.c:1867 > [inline] >serial8250_default_handle_irq+0xc8/0x150 > drivers/tty/serial/8250/8250_port.c:1897 >serial8250_interrupt+0xfa/0x1d0 > drivers/tty/serial/8250/8250_core.c:125 >__handle_irq_event_percpu+0x1c0/0xad0 kernel/irq/handle.c:149 >handle_irq_event_percpu+0x98/0x1c0 kernel/irq/handle.c:189 >handle_irq_event+0xa7/0x135 kernel/irq/handle.c:206 >handle_edge_irq+0x20f/0x870 kernel/irq/chip.c:791 >generic_handle_irq_desc include/linux/irqdesc.h:159 [inline] >handle_irq+0x18c/0x2e7 arch/x86/kernel/irq_64.c:77 >do_IRQ+0x78/0x190 arch/x86/kernel/irq.c:245 >ret_from_intr+0x0/0x1e >native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:54 >arch_safe_halt arch/x86/include/asm/paravirt.h:94 [inline] >default_idle+0xc2/0x440 arch/x86/kernel/process.c:500 >arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:491 >default_idle_call+0x6d/0x90 kernel/sched/idle.c:93 >cpuidle_idle_call kernel/sched/idle.c:153 [inline] >do_idle+0x395/0x560 kernel/sched/idle.c:262 >cpu_startup_entry+0x104/0x120 kernel/sched/idle.c:368 >start_secondary+0x42b/0x5c0 arch/x86/kernel/smpboot.c:265 >secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:242 So this one is IRQ ==> uart_port->lock ==> tty_port->lock > -> #1 (_lock_key){-.-.}: >__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] >_raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152 >serial8250_console_write+0x8d5/0xb00 > drivers/tty/serial/8250/8250_port.c:3230 >univ8250_console_write+0x5f/0x70 > drivers/tty/serial/8250/8250_core.c:590 >call_console_drivers kernel/printk/printk.c:1718 [inline] >console_unlock+0xac2/0x1100 kernel/printk/printk.c:2395 >vprintk_emit+0x6ad/0xdd0 kernel/printk/printk.c:1907 >vprintk_default+0x28/0x30 kernel/printk/printk.c:1947 >vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:379 >printk+0x9e/0xba kernel/printk/printk.c:1980 >register_console+0x7e7/0xc00 kernel/printk/printk.c:2714 >univ8250_console_init+0x3f/0x4b > drivers/tty/serial/8250/8250_core.c:685 >console_init+0x6d9/0xa38 kernel/printk/printk.c:2798 >start_kernel+0x608/0x92d init/main.c:661 >x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:452 >x86_64_start_kernel+0x76/0x79 arch/x86/kernel/head64.c:433 >secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:242 This one is console_owner/console_sem ==> uart_port->lock > -> #0 (console_owner){-...}: >lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3924 >console_lock_spinning_enable kernel/printk/printk.c:1581 [inline] >
Re: possible deadlock in console_unlock
Cc-ing more people Link: lkml.kernel.org/r/87008b056df8f...@google.com On (06/06/18 06:17), syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:af6c5d5e01ad Merge branch 'for-4.18' of git://git.kernel.o.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=173d93ef80 > kernel config: https://syzkaller.appspot.com/x/.config?x=12ff770540994680 > dashboard link: https://syzkaller.appspot.com/bug?extid=43e93968b964e369db0b > compiler: gcc (GCC) 8.0.1 20180413 (experimental) > userspace arch: i386 > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=16e00bb780 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+43e93968b964e369d...@syzkaller.appspotmail.com Thanks a ton! So tty ioctl is known to be printk-deadlock prone and we don't know how to handle this in printk, as of now. ioctl tty_ioctl tty_port->lock printk call_console_driver console_driver uart_port->lock The problem is that call_console_driver->console_driver also can do this thing uart_port->lock tty_wakeup tty_port->lock So we can have the following: ioctl tty_ioctl tty_port->lock printk call_console_driver console_driver uart_port->lock tty_wakeup tty_port->lock << deadlock But lockdep is complaining about another scenario: > the existing dependency chain (in reverse order) is: > > -> #2 (&(>lock)->rlock){-.-.}: >__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] >_raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152 >tty_port_tty_get+0x20/0x80 drivers/tty/tty_port.c:288 >tty_port_default_wakeup+0x15/0x40 drivers/tty/tty_port.c:47 >tty_port_tty_wakeup+0x5d/0x70 drivers/tty/tty_port.c:390 >uart_write_wakeup+0x44/0x60 drivers/tty/serial/serial_core.c:103 >serial8250_tx_chars+0x4be/0xb60 > drivers/tty/serial/8250/8250_port.c:1808 >serial8250_handle_irq.part.25+0x1ee/0x280 > drivers/tty/serial/8250/8250_port.c:1881 >serial8250_handle_irq drivers/tty/serial/8250/8250_port.c:1867 > [inline] >serial8250_default_handle_irq+0xc8/0x150 > drivers/tty/serial/8250/8250_port.c:1897 >serial8250_interrupt+0xfa/0x1d0 > drivers/tty/serial/8250/8250_core.c:125 >__handle_irq_event_percpu+0x1c0/0xad0 kernel/irq/handle.c:149 >handle_irq_event_percpu+0x98/0x1c0 kernel/irq/handle.c:189 >handle_irq_event+0xa7/0x135 kernel/irq/handle.c:206 >handle_edge_irq+0x20f/0x870 kernel/irq/chip.c:791 >generic_handle_irq_desc include/linux/irqdesc.h:159 [inline] >handle_irq+0x18c/0x2e7 arch/x86/kernel/irq_64.c:77 >do_IRQ+0x78/0x190 arch/x86/kernel/irq.c:245 >ret_from_intr+0x0/0x1e >native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:54 >arch_safe_halt arch/x86/include/asm/paravirt.h:94 [inline] >default_idle+0xc2/0x440 arch/x86/kernel/process.c:500 >arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:491 >default_idle_call+0x6d/0x90 kernel/sched/idle.c:93 >cpuidle_idle_call kernel/sched/idle.c:153 [inline] >do_idle+0x395/0x560 kernel/sched/idle.c:262 >cpu_startup_entry+0x104/0x120 kernel/sched/idle.c:368 >start_secondary+0x42b/0x5c0 arch/x86/kernel/smpboot.c:265 >secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:242 So this one is IRQ ==> uart_port->lock ==> tty_port->lock > -> #1 (_lock_key){-.-.}: >__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] >_raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152 >serial8250_console_write+0x8d5/0xb00 > drivers/tty/serial/8250/8250_port.c:3230 >univ8250_console_write+0x5f/0x70 > drivers/tty/serial/8250/8250_core.c:590 >call_console_drivers kernel/printk/printk.c:1718 [inline] >console_unlock+0xac2/0x1100 kernel/printk/printk.c:2395 >vprintk_emit+0x6ad/0xdd0 kernel/printk/printk.c:1907 >vprintk_default+0x28/0x30 kernel/printk/printk.c:1947 >vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:379 >printk+0x9e/0xba kernel/printk/printk.c:1980 >register_console+0x7e7/0xc00 kernel/printk/printk.c:2714 >univ8250_console_init+0x3f/0x4b > drivers/tty/serial/8250/8250_core.c:685 >console_init+0x6d9/0xa38 kernel/printk/printk.c:2798 >start_kernel+0x608/0x92d init/main.c:661 >x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:452 >x86_64_start_kernel+0x76/0x79 arch/x86/kernel/head64.c:433 >secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:242 This one is console_owner/console_sem ==> uart_port->lock > -> #0 (console_owner){-...}: >lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3924 >console_lock_spinning_enable kernel/printk/printk.c:1581 [inline] >
Re: [PATCH] pwm: stm32: enforce dependency on CONFIG_MFD_STM32_TIMERS
On Wed, 06 Jun 2018, Thierry Reding wrote: > On Fri, May 25, 2018 at 06:04:54PM +0200, Arnd Bergmann wrote: > > When compile-testing the pwm driver without also enabling the > > stm32_timers MFD, we run into a link error: > > > > drivers/pwm/pwm-stm32.o: In function `stm32_pwm_raw_capture.isra.6': > > pwm-stm32.c:(.text+0xcb0): undefined reference to > > `stm32_timers_dma_burst_read' > > > > We don't need the '|| COMPILE_TEST' here, since stm32_timers itself > > can be built with CONFIG_COMPILE_TEST on all architectures, so we do > > get the coverage through allmodconfig and randconfig builds even > > when we make it a hard dependency. > > > > Fixes: 7edf7369205b ("pwm: Add driver for STM32 plaftorm") > > Signed-off-by: Arnd Bergmann > > --- > > drivers/pwm/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > I think Lee already picked up a patch by Fabrice that fixes this by > adding a dummy implementation, see: > > http://patchwork.ozlabs.org/patch/916435/ > > I prefer your solution, though, because the dummy is pretty redundant if > we can just make this a hard-dependency and still get the same coverage. The two patches aren't mutually exclusive. :) -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH] pwm: stm32: enforce dependency on CONFIG_MFD_STM32_TIMERS
On Wed, 06 Jun 2018, Thierry Reding wrote: > On Fri, May 25, 2018 at 06:04:54PM +0200, Arnd Bergmann wrote: > > When compile-testing the pwm driver without also enabling the > > stm32_timers MFD, we run into a link error: > > > > drivers/pwm/pwm-stm32.o: In function `stm32_pwm_raw_capture.isra.6': > > pwm-stm32.c:(.text+0xcb0): undefined reference to > > `stm32_timers_dma_burst_read' > > > > We don't need the '|| COMPILE_TEST' here, since stm32_timers itself > > can be built with CONFIG_COMPILE_TEST on all architectures, so we do > > get the coverage through allmodconfig and randconfig builds even > > when we make it a hard dependency. > > > > Fixes: 7edf7369205b ("pwm: Add driver for STM32 plaftorm") > > Signed-off-by: Arnd Bergmann > > --- > > drivers/pwm/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > I think Lee already picked up a patch by Fabrice that fixes this by > adding a dummy implementation, see: > > http://patchwork.ozlabs.org/patch/916435/ > > I prefer your solution, though, because the dummy is pretty redundant if > we can just make this a hard-dependency and still get the same coverage. The two patches aren't mutually exclusive. :) -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH] pwm: stm32: fix build warning with CONFIG_DMA_ENGINE disabled
On Wed, 06 Jun 2018, Thierry Reding wrote: > On Fri, May 25, 2018 at 11:08:30PM +0200, Arnd Bergmann wrote: > > Without dmaengine support, we get a harmless warning about an > > unused function: > > > > drivers/pwm/pwm-stm32.c:166:12: error: 'stm32_pwm_capture' defined but not > > used [-Werror=unused-function] > > > > Changing the #ifdef to an IS_ENABLED() check shuts up that warning > > and is slightly nicer to read. > > > > Fixes: 53e38fe73f94 ("pwm: stm32: Add capture support") > > Signed-off-by: Arnd Bergmann > > --- > > drivers/pwm/pwm-stm32.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > Applied, thanks. > > Lee, I applied this on top of your immutable MFD/PWM branch because it > depends on the capture support that you applied. I wasn't sure what your > PR timing was going to be, so I thought I'd do it this way since I'm > pulling in some last minute fixes for v4.18. I don't see a problem with that. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH] pwm: stm32: fix build warning with CONFIG_DMA_ENGINE disabled
On Wed, 06 Jun 2018, Thierry Reding wrote: > On Fri, May 25, 2018 at 11:08:30PM +0200, Arnd Bergmann wrote: > > Without dmaengine support, we get a harmless warning about an > > unused function: > > > > drivers/pwm/pwm-stm32.c:166:12: error: 'stm32_pwm_capture' defined but not > > used [-Werror=unused-function] > > > > Changing the #ifdef to an IS_ENABLED() check shuts up that warning > > and is slightly nicer to read. > > > > Fixes: 53e38fe73f94 ("pwm: stm32: Add capture support") > > Signed-off-by: Arnd Bergmann > > --- > > drivers/pwm/pwm-stm32.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > Applied, thanks. > > Lee, I applied this on top of your immutable MFD/PWM branch because it > depends on the capture support that you applied. I wasn't sure what your > PR timing was going to be, so I thought I'd do it this way since I'm > pulling in some last minute fixes for v4.18. I don't see a problem with that. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH] uapi: Make generic uapi headers compile standalone.
Hi. 2018-06-07 8:16 GMT+09:00 Jayant Chowdhary : > In order for static analysis tools to analyze each of the uapi headers, > we need to enable them to compile stand-alone. Some uapi headers were > missing dependencies which would not make them compile stand-alone in > user-land. This patch adds those dependencies. > > Test: make defconfig; make -j64 > > Test: make ARCH=arm64 defconfig; > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make -j64 all > > Test: ran header-abi-dumper[1] on all the affected headers with > appropriate include paths[2] for arm64. eg: for drm_fourcc.h, > > $HEADER_ABI_DUMPER -o drm_fourcc.h.sdump \ > include/uapi/drm/drm_fourcc.h -- -target aarch64-linux-android -std=gnu99 \ > -isystem $CLANG_INCLUDE_PATH \ > -I include/uapi/ -I arch/arm64/include/generated/uapi \ > -I arch/arm64/include/uapi -I private-compiler-headers > -I include/generated/uapi \ > -I include/uapi/../../arch/arm/include/uapi First of all, is this check sensible? scripts/headers_install.sh manipulates exported headers. So, shouldn't you do this check after "make headers_install"? You are adding around to various UAPI headers. Could you tell me why it was added? The tool rips off anyway when exporting UAPI headers: https://github.com/torvalds/linux/blob/v4.17/scripts/headers_install.sh#L37 Also, you are adding to several headers. Please do not pull it in to the kernel space. The kernel space defines fixed-width types in its headers. For example, you added to include/uapi/scsi/scsi_netlink.h In my opinion, the correct fix is to replace uint8_t with __u8 or __uint8_t. > where > HEADER_ABI_DUMPER=\ > $ANDROID_BUILD_TOP/prebuilts/clang-tools/linux-x86/bin/header-abi-dumper > > CLANG_INCLUDE_PATH=\ > $ANDROID_BUILD_TOP/prebuilts/clang/host/linux-x86/clang-3289846/bin/../lib64/clang/3.8/include > > on a lunched aosp tree [3] > > [1] > header-abi-dumper is a clang based static analysis tool, used by android's > build > system which parses a C/C++ source file and emits information about the data > types included in the source. More information can be found at > https://android.googlesource.com/platform/development/+/master/vndk/tools/header-checker/README.md > > [2] > Some headers, eg: compiler_types.h, compiler.h, sys/socket.h are not in the > set of uapi headers, even though they are included by uapi headers( > include/uapi/linux/types.h, include/uapi/asm-generic/signal-defs.h etc). The > 'private-compiler-headers' in the test clause was included with private copies > of these headers. libc distributions (eg: bionic) often have their > copies of these headers as well. > > [3] > https://source.android.com/setup/build/downloading > > Cc: a...@linux-foundation.org > Cc: kernel-t...@android.com > Cc: linux-kbu...@vger.kernel.org > Signed-off-by: Jayant Chowdhary > --- > include/uapi/asm-generic/ipcbuf.h | 2 ++ > include/uapi/asm-generic/msgbuf.h | 3 +++ > include/uapi/asm-generic/sembuf.h | 2 ++ > include/uapi/asm-generic/shmbuf.h | 2 ++ > include/uapi/asm-generic/ucontext.h| 5 + > include/uapi/linux/agpgart.h | 1 - > include/uapi/linux/android/binder.h| 1 + > include/uapi/linux/chio.h | 6 ++ > include/uapi/linux/coda_psdev.h| 1 + > include/uapi/linux/dvb/dmx.h | 3 --- > include/uapi/linux/dvb/video.h | 3 --- > include/uapi/linux/errqueue.h | 1 + > include/uapi/linux/kexec.h | 1 + > include/uapi/linux/kfd_ioctl.h | 1 + > include/uapi/linux/lightnvm.h | 1 - > include/uapi/linux/ndctl.h | 1 + > include/uapi/linux/netfilter_bridge/ebtables.h | 1 + > include/uapi/linux/nfs4_mount.h| 3 +++ > include/uapi/linux/psp-sev.h | 1 + > include/uapi/linux/scc.h | 2 +- > include/uapi/linux/sctp.h | 3 +++ > include/uapi/linux/sdla.h | 2 ++ > include/uapi/linux/socket.h| 4 > include/uapi/linux/stddef.h| 5 + > include/uapi/linux/sysctl.h| 1 + > include/uapi/linux/types.h | 1 + > include/uapi/linux/vbox_vmmdev_types.h | 1 + > include/uapi/linux/vboxguest.h | 1 + > include/uapi/rdma/hfi/hfi1_user.h | 1 + > include/uapi/scsi/scsi_bsg_fc.h| 2 ++ > include/uapi/scsi/scsi_netlink.h | 1 + > include/uapi/sound/asound.h| 2 +- > 32 files changed, 55 insertions(+), 10 deletions(-) > > diff --git a/include/uapi/asm-generic/ipcbuf.h > b/include/uapi/asm-generic/ipcbuf.h > index 7d80dbd336fb..41a01b494fc7 100644 > --- a/include/uapi/asm-generic/ipcbuf.h > +++ b/include/uapi/asm-generic/ipcbuf.h > @@ -2,6 +2,8 @@ > #ifndef __ASM_GENERIC_IPCBUF_H > #define
Re: [PATCH] uapi: Make generic uapi headers compile standalone.
Hi. 2018-06-07 8:16 GMT+09:00 Jayant Chowdhary : > In order for static analysis tools to analyze each of the uapi headers, > we need to enable them to compile stand-alone. Some uapi headers were > missing dependencies which would not make them compile stand-alone in > user-land. This patch adds those dependencies. > > Test: make defconfig; make -j64 > > Test: make ARCH=arm64 defconfig; > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make -j64 all > > Test: ran header-abi-dumper[1] on all the affected headers with > appropriate include paths[2] for arm64. eg: for drm_fourcc.h, > > $HEADER_ABI_DUMPER -o drm_fourcc.h.sdump \ > include/uapi/drm/drm_fourcc.h -- -target aarch64-linux-android -std=gnu99 \ > -isystem $CLANG_INCLUDE_PATH \ > -I include/uapi/ -I arch/arm64/include/generated/uapi \ > -I arch/arm64/include/uapi -I private-compiler-headers > -I include/generated/uapi \ > -I include/uapi/../../arch/arm/include/uapi First of all, is this check sensible? scripts/headers_install.sh manipulates exported headers. So, shouldn't you do this check after "make headers_install"? You are adding around to various UAPI headers. Could you tell me why it was added? The tool rips off anyway when exporting UAPI headers: https://github.com/torvalds/linux/blob/v4.17/scripts/headers_install.sh#L37 Also, you are adding to several headers. Please do not pull it in to the kernel space. The kernel space defines fixed-width types in its headers. For example, you added to include/uapi/scsi/scsi_netlink.h In my opinion, the correct fix is to replace uint8_t with __u8 or __uint8_t. > where > HEADER_ABI_DUMPER=\ > $ANDROID_BUILD_TOP/prebuilts/clang-tools/linux-x86/bin/header-abi-dumper > > CLANG_INCLUDE_PATH=\ > $ANDROID_BUILD_TOP/prebuilts/clang/host/linux-x86/clang-3289846/bin/../lib64/clang/3.8/include > > on a lunched aosp tree [3] > > [1] > header-abi-dumper is a clang based static analysis tool, used by android's > build > system which parses a C/C++ source file and emits information about the data > types included in the source. More information can be found at > https://android.googlesource.com/platform/development/+/master/vndk/tools/header-checker/README.md > > [2] > Some headers, eg: compiler_types.h, compiler.h, sys/socket.h are not in the > set of uapi headers, even though they are included by uapi headers( > include/uapi/linux/types.h, include/uapi/asm-generic/signal-defs.h etc). The > 'private-compiler-headers' in the test clause was included with private copies > of these headers. libc distributions (eg: bionic) often have their > copies of these headers as well. > > [3] > https://source.android.com/setup/build/downloading > > Cc: a...@linux-foundation.org > Cc: kernel-t...@android.com > Cc: linux-kbu...@vger.kernel.org > Signed-off-by: Jayant Chowdhary > --- > include/uapi/asm-generic/ipcbuf.h | 2 ++ > include/uapi/asm-generic/msgbuf.h | 3 +++ > include/uapi/asm-generic/sembuf.h | 2 ++ > include/uapi/asm-generic/shmbuf.h | 2 ++ > include/uapi/asm-generic/ucontext.h| 5 + > include/uapi/linux/agpgart.h | 1 - > include/uapi/linux/android/binder.h| 1 + > include/uapi/linux/chio.h | 6 ++ > include/uapi/linux/coda_psdev.h| 1 + > include/uapi/linux/dvb/dmx.h | 3 --- > include/uapi/linux/dvb/video.h | 3 --- > include/uapi/linux/errqueue.h | 1 + > include/uapi/linux/kexec.h | 1 + > include/uapi/linux/kfd_ioctl.h | 1 + > include/uapi/linux/lightnvm.h | 1 - > include/uapi/linux/ndctl.h | 1 + > include/uapi/linux/netfilter_bridge/ebtables.h | 1 + > include/uapi/linux/nfs4_mount.h| 3 +++ > include/uapi/linux/psp-sev.h | 1 + > include/uapi/linux/scc.h | 2 +- > include/uapi/linux/sctp.h | 3 +++ > include/uapi/linux/sdla.h | 2 ++ > include/uapi/linux/socket.h| 4 > include/uapi/linux/stddef.h| 5 + > include/uapi/linux/sysctl.h| 1 + > include/uapi/linux/types.h | 1 + > include/uapi/linux/vbox_vmmdev_types.h | 1 + > include/uapi/linux/vboxguest.h | 1 + > include/uapi/rdma/hfi/hfi1_user.h | 1 + > include/uapi/scsi/scsi_bsg_fc.h| 2 ++ > include/uapi/scsi/scsi_netlink.h | 1 + > include/uapi/sound/asound.h| 2 +- > 32 files changed, 55 insertions(+), 10 deletions(-) > > diff --git a/include/uapi/asm-generic/ipcbuf.h > b/include/uapi/asm-generic/ipcbuf.h > index 7d80dbd336fb..41a01b494fc7 100644 > --- a/include/uapi/asm-generic/ipcbuf.h > +++ b/include/uapi/asm-generic/ipcbuf.h > @@ -2,6 +2,8 @@ > #ifndef __ASM_GENERIC_IPCBUF_H > #define
Re: [PATCH][RFC] sched: cpufreq: Fix long idle judgement logic in load calculation
On 07-06-18, 11:17, Chen Yu wrote: > diff --git a/drivers/cpufreq/cpufreq_governor.c > b/drivers/cpufreq/cpufreq_governor.c > index 871bf9c..9792c80 100644 > --- a/drivers/cpufreq/cpufreq_governor.c > +++ b/drivers/cpufreq/cpufreq_governor.c > @@ -165,7 +165,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy) >* calls, so the previous load value can be used then. >*/ > load = j_cdbs->prev_load; > - } else if (unlikely(time_elapsed > 2 * sampling_rate && > + } else if (((int)idle_time > 0) && unlikely(idle_time > 2 * > sampling_rate && Yes the figures are insane, but if the idle time is around 36 minutes, the conversion to int will make a positive value look negative and we will exit the conditional block. And if we don't think that we will ever get such insane idle times or we don't want to care about them, then what about doing this instead: } else if ((unlikely((int)idle_time > 2 * sampling_rate && same below. -- viresh
Re: [PATCH][RFC] sched: cpufreq: Fix long idle judgement logic in load calculation
On 07-06-18, 11:17, Chen Yu wrote: > diff --git a/drivers/cpufreq/cpufreq_governor.c > b/drivers/cpufreq/cpufreq_governor.c > index 871bf9c..9792c80 100644 > --- a/drivers/cpufreq/cpufreq_governor.c > +++ b/drivers/cpufreq/cpufreq_governor.c > @@ -165,7 +165,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy) >* calls, so the previous load value can be used then. >*/ > load = j_cdbs->prev_load; > - } else if (unlikely(time_elapsed > 2 * sampling_rate && > + } else if (((int)idle_time > 0) && unlikely(idle_time > 2 * > sampling_rate && Yes the figures are insane, but if the idle time is around 36 minutes, the conversion to int will make a positive value look negative and we will exit the conditional block. And if we don't think that we will ever get such insane idle times or we don't want to care about them, then what about doing this instead: } else if ((unlikely((int)idle_time > 2 * sampling_rate && same below. -- viresh
Re: possible deadlock in console_unlock
syzbot has found a reproducer for the following crash on: HEAD commit:0ad39cb3d70f Merge tag 'kconfig-v4.18' of git://git.kernel.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=1158868f80 kernel config: https://syzkaller.appspot.com/x/.config?x=b9a1f3aa8b8ddd16 dashboard link: https://syzkaller.appspot.com/bug?extid=43e93968b964e369db0b compiler: gcc (GCC) 8.0.1 20180413 (experimental) syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=14c89b9f80 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=167f596f80 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+43e93968b964e369d...@syzkaller.appspotmail.com R10: R11: 0246 R12: R13: 7f3350380d80 R14: 0004 R15: 6d74702f7665642f CPU: 1 PID: 4456 Comm: syz-executor589 Not tainted 4.17.0+ #87 == WARNING: possible circular locking dependency detected 4.17.0+ #87 Not tainted -- syz-executor589/4455 is trying to acquire lock: (ptrval) (console_owner){-...}, at: log_next kernel/printk/printk.c:496 [inline] (ptrval) (console_owner){-...}, at: console_unlock+0x583/0x1100 kernel/printk/printk.c:2382 but task is already holding lock: (ptrval) (&(>lock)->rlock){-.-.}, at: pty_write+0xf9/0x1f0 drivers/tty/pty.c:119 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (&(>lock)->rlock){-.-.}: __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152 tty_port_tty_get+0x20/0x80 drivers/tty/tty_port.c:288 tty_port_default_wakeup+0x15/0x40 drivers/tty/tty_port.c:47 tty_port_tty_wakeup+0x5d/0x70 drivers/tty/tty_port.c:390 uart_write_wakeup+0x44/0x60 drivers/tty/serial/serial_core.c:103 serial8250_tx_chars+0x4be/0xb60 drivers/tty/serial/8250/8250_port.c:1808 serial8250_handle_irq.part.25+0x1ee/0x280 drivers/tty/serial/8250/8250_port.c:1881 serial8250_handle_irq drivers/tty/serial/8250/8250_port.c:1867 [inline] serial8250_default_handle_irq+0xc8/0x150 drivers/tty/serial/8250/8250_port.c:1897 serial8250_interrupt+0xfa/0x1d0 drivers/tty/serial/8250/8250_core.c:125 __handle_irq_event_percpu+0x1c0/0xad0 kernel/irq/handle.c:149 handle_irq_event_percpu+0x98/0x1c0 kernel/irq/handle.c:189 handle_irq_event+0xa7/0x135 kernel/irq/handle.c:206 handle_edge_irq+0x20f/0x870 kernel/irq/chip.c:791 generic_handle_irq_desc include/linux/irqdesc.h:159 [inline] handle_irq+0x18c/0x2e7 arch/x86/kernel/irq_64.c:77 do_IRQ+0x78/0x190 arch/x86/kernel/irq.c:245 ret_from_intr+0x0/0x1e arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 [inline] __raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] _raw_spin_unlock_irqrestore+0xa1/0xc0 kernel/locking/spinlock.c:184 spin_unlock_irqrestore include/linux/spinlock.h:365 [inline] uart_write+0x3df/0x620 drivers/tty/serial/serial_core.c:591 process_output_block drivers/tty/n_tty.c:579 [inline] n_tty_write+0x6b9/0x1180 drivers/tty/n_tty.c:2308 do_tty_write drivers/tty/tty_io.c:958 [inline] tty_write+0x3f1/0x880 drivers/tty/tty_io.c:1042 redirected_tty_write+0xaf/0xc0 drivers/tty/tty_io.c:1063 __vfs_write+0x10b/0x960 fs/read_write.c:485 vfs_write+0x1f8/0x560 fs/read_write.c:549 ksys_write+0xf9/0x250 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x73/0xb0 fs/read_write.c:607 do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x49/0xbe -> #1 (_lock_key){-.-.}: __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152 serial8250_console_write+0x8d5/0xb00 drivers/tty/serial/8250/8250_port.c:3230 univ8250_console_write+0x5f/0x70 drivers/tty/serial/8250/8250_core.c:590 call_console_drivers kernel/printk/printk.c:1718 [inline] console_unlock+0xac2/0x1100 kernel/printk/printk.c:2395 vprintk_emit+0x6ad/0xdd0 kernel/printk/printk.c:1907 vprintk_default+0x28/0x30 kernel/printk/printk.c:1947 vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:379 printk+0x9e/0xba kernel/printk/printk.c:1980 register_console+0x7e7/0xc00 kernel/printk/printk.c:2714 univ8250_console_init+0x3f/0x4b drivers/tty/serial/8250/8250_core.c:685 console_init+0x6d9/0xa38 kernel/printk/printk.c:2798 start_kernel+0x608/0x92d init/main.c:661 x86_64_start_reservations+0x29/0x2b
Re: possible deadlock in console_unlock
syzbot has found a reproducer for the following crash on: HEAD commit:0ad39cb3d70f Merge tag 'kconfig-v4.18' of git://git.kernel.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=1158868f80 kernel config: https://syzkaller.appspot.com/x/.config?x=b9a1f3aa8b8ddd16 dashboard link: https://syzkaller.appspot.com/bug?extid=43e93968b964e369db0b compiler: gcc (GCC) 8.0.1 20180413 (experimental) syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=14c89b9f80 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=167f596f80 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+43e93968b964e369d...@syzkaller.appspotmail.com R10: R11: 0246 R12: R13: 7f3350380d80 R14: 0004 R15: 6d74702f7665642f CPU: 1 PID: 4456 Comm: syz-executor589 Not tainted 4.17.0+ #87 == WARNING: possible circular locking dependency detected 4.17.0+ #87 Not tainted -- syz-executor589/4455 is trying to acquire lock: (ptrval) (console_owner){-...}, at: log_next kernel/printk/printk.c:496 [inline] (ptrval) (console_owner){-...}, at: console_unlock+0x583/0x1100 kernel/printk/printk.c:2382 but task is already holding lock: (ptrval) (&(>lock)->rlock){-.-.}, at: pty_write+0xf9/0x1f0 drivers/tty/pty.c:119 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (&(>lock)->rlock){-.-.}: __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152 tty_port_tty_get+0x20/0x80 drivers/tty/tty_port.c:288 tty_port_default_wakeup+0x15/0x40 drivers/tty/tty_port.c:47 tty_port_tty_wakeup+0x5d/0x70 drivers/tty/tty_port.c:390 uart_write_wakeup+0x44/0x60 drivers/tty/serial/serial_core.c:103 serial8250_tx_chars+0x4be/0xb60 drivers/tty/serial/8250/8250_port.c:1808 serial8250_handle_irq.part.25+0x1ee/0x280 drivers/tty/serial/8250/8250_port.c:1881 serial8250_handle_irq drivers/tty/serial/8250/8250_port.c:1867 [inline] serial8250_default_handle_irq+0xc8/0x150 drivers/tty/serial/8250/8250_port.c:1897 serial8250_interrupt+0xfa/0x1d0 drivers/tty/serial/8250/8250_core.c:125 __handle_irq_event_percpu+0x1c0/0xad0 kernel/irq/handle.c:149 handle_irq_event_percpu+0x98/0x1c0 kernel/irq/handle.c:189 handle_irq_event+0xa7/0x135 kernel/irq/handle.c:206 handle_edge_irq+0x20f/0x870 kernel/irq/chip.c:791 generic_handle_irq_desc include/linux/irqdesc.h:159 [inline] handle_irq+0x18c/0x2e7 arch/x86/kernel/irq_64.c:77 do_IRQ+0x78/0x190 arch/x86/kernel/irq.c:245 ret_from_intr+0x0/0x1e arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 [inline] __raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] _raw_spin_unlock_irqrestore+0xa1/0xc0 kernel/locking/spinlock.c:184 spin_unlock_irqrestore include/linux/spinlock.h:365 [inline] uart_write+0x3df/0x620 drivers/tty/serial/serial_core.c:591 process_output_block drivers/tty/n_tty.c:579 [inline] n_tty_write+0x6b9/0x1180 drivers/tty/n_tty.c:2308 do_tty_write drivers/tty/tty_io.c:958 [inline] tty_write+0x3f1/0x880 drivers/tty/tty_io.c:1042 redirected_tty_write+0xaf/0xc0 drivers/tty/tty_io.c:1063 __vfs_write+0x10b/0x960 fs/read_write.c:485 vfs_write+0x1f8/0x560 fs/read_write.c:549 ksys_write+0xf9/0x250 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x73/0xb0 fs/read_write.c:607 do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x49/0xbe -> #1 (_lock_key){-.-.}: __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152 serial8250_console_write+0x8d5/0xb00 drivers/tty/serial/8250/8250_port.c:3230 univ8250_console_write+0x5f/0x70 drivers/tty/serial/8250/8250_core.c:590 call_console_drivers kernel/printk/printk.c:1718 [inline] console_unlock+0xac2/0x1100 kernel/printk/printk.c:2395 vprintk_emit+0x6ad/0xdd0 kernel/printk/printk.c:1907 vprintk_default+0x28/0x30 kernel/printk/printk.c:1947 vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:379 printk+0x9e/0xba kernel/printk/printk.c:1980 register_console+0x7e7/0xc00 kernel/printk/printk.c:2714 univ8250_console_init+0x3f/0x4b drivers/tty/serial/8250/8250_core.c:685 console_init+0x6d9/0xa38 kernel/printk/printk.c:2798 start_kernel+0x608/0x92d init/main.c:661 x86_64_start_reservations+0x29/0x2b
Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
On Wed 06 Jun 21:11 PDT 2018, Vinod wrote: > On 06-06-18, 09:17, Bjorn Andersson wrote: > > On Tue 05 Jun 05:56 PDT 2018, Sricharan R wrote: > > > > > Hi Vinod, > > > > > > On 6/5/2018 11:49 AM, Vinod wrote: > > > > On 05-06-18, 11:12, Sricharan R wrote: > > > > > > > >> +config QCOM_Q6V5_WCSS > > > >> + tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader" > > > >> + depends on OF && ARCH_QCOM > > > >> + depends on QCOM_SMEM > > > >> + depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n) > > > >> + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > > > > > > > > Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would > > > > happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM > > > > > > > > It says that QCOM_Q6V5_WCSS either must have a compatible state (i.e. > > builtin vs builtin, module vs builtin, but not builtin vs module) or > > that it's disabled, in which case we will hit the stub functions in > > qcom_glink.h. > > > > I.e. this prevents QCOM_Q6V5_WCSS to be compiled builtin when > > RPMSG_QCOM_GLINK_SMEM is module, as this would give us both stubs and > > the module. > > IIUC, you want to have QCOM_Q6V5_WCSS and RPMSG_QCOM_GLINK_SMEM as > modules or builtin > RPMSG_QCOM_SMD, RPMSG_QCOM_GLINK_SMEM and QCOM_Q6V5_WCSS are all tristate. > So, wouldn't Kconfig syntax something like where we say: > M if RPMSG_QCOM_GLINK_SMEM=m > bool if RPMSG_QCOM_GLINK_SMEM=y > If we ignore SMD for a while we have the following combinations: glink/wcss y y - valid y m - valid y n - valid m y - link failure (invalid) m m - valid m n - valid n y - valid (platform uses wcss, but not glink) n m - valid (-"-) n n - valid So to distill this we have the two valid cases: module/no if RPMSG_QCOM_GLINK_SMEM=m yes/module/no if RPMSG_QCOM_GLINK_SMEM=y and the way you express that in Kconfig is the somewhat awkward depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > Which makes it clear that both these have to be same type? > They don't have to be of the same type, only of a compatible type. Regards, Bjorn
Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
On Wed 06 Jun 21:11 PDT 2018, Vinod wrote: > On 06-06-18, 09:17, Bjorn Andersson wrote: > > On Tue 05 Jun 05:56 PDT 2018, Sricharan R wrote: > > > > > Hi Vinod, > > > > > > On 6/5/2018 11:49 AM, Vinod wrote: > > > > On 05-06-18, 11:12, Sricharan R wrote: > > > > > > > >> +config QCOM_Q6V5_WCSS > > > >> + tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader" > > > >> + depends on OF && ARCH_QCOM > > > >> + depends on QCOM_SMEM > > > >> + depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n) > > > >> + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > > > > > > > > Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would > > > > happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM > > > > > > > > It says that QCOM_Q6V5_WCSS either must have a compatible state (i.e. > > builtin vs builtin, module vs builtin, but not builtin vs module) or > > that it's disabled, in which case we will hit the stub functions in > > qcom_glink.h. > > > > I.e. this prevents QCOM_Q6V5_WCSS to be compiled builtin when > > RPMSG_QCOM_GLINK_SMEM is module, as this would give us both stubs and > > the module. > > IIUC, you want to have QCOM_Q6V5_WCSS and RPMSG_QCOM_GLINK_SMEM as > modules or builtin > RPMSG_QCOM_SMD, RPMSG_QCOM_GLINK_SMEM and QCOM_Q6V5_WCSS are all tristate. > So, wouldn't Kconfig syntax something like where we say: > M if RPMSG_QCOM_GLINK_SMEM=m > bool if RPMSG_QCOM_GLINK_SMEM=y > If we ignore SMD for a while we have the following combinations: glink/wcss y y - valid y m - valid y n - valid m y - link failure (invalid) m m - valid m n - valid n y - valid (platform uses wcss, but not glink) n m - valid (-"-) n n - valid So to distill this we have the two valid cases: module/no if RPMSG_QCOM_GLINK_SMEM=m yes/module/no if RPMSG_QCOM_GLINK_SMEM=y and the way you express that in Kconfig is the somewhat awkward depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > Which makes it clear that both these have to be same type? > They don't have to be of the same type, only of a compatible type. Regards, Bjorn
linux-next: manual merge of the nvdimm tree with the tip tree
Hi all, Today's linux-next merge of the nvdimm tree got a conflict in: kernel/Makefile between commit: d7822b1e24f2 ("rseq: Introduce restartable sequences system call") from the tip tree and commit: 5981690ddb8f ("memremap: split devm_memremap_pages() and memremap() infrastructure") from the nvdimm tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc kernel/Makefile index 7085c841c413,9b9241361311.. --- a/kernel/Makefile +++ b/kernel/Makefile @@@ -112,8 -112,8 +112,9 @@@ obj-$(CONFIG_JUMP_LABEL) += jump_label. obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o obj-$(CONFIG_TORTURE_TEST) += torture.o - obj-$(CONFIG_HAS_IOMEM) += memremap.o + obj-$(CONFIG_HAS_IOMEM) += iomem.o + obj-$(CONFIG_ZONE_DEVICE) += memremap.o +obj-$(CONFIG_RSEQ) += rseq.o $(obj)/configs.o: $(obj)/config_data.h pgphsiKSy5d3l.pgp Description: OpenPGP digital signature
linux-next: manual merge of the nvdimm tree with the tip tree
Hi all, Today's linux-next merge of the nvdimm tree got a conflict in: kernel/Makefile between commit: d7822b1e24f2 ("rseq: Introduce restartable sequences system call") from the tip tree and commit: 5981690ddb8f ("memremap: split devm_memremap_pages() and memremap() infrastructure") from the nvdimm tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc kernel/Makefile index 7085c841c413,9b9241361311.. --- a/kernel/Makefile +++ b/kernel/Makefile @@@ -112,8 -112,8 +112,9 @@@ obj-$(CONFIG_JUMP_LABEL) += jump_label. obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o obj-$(CONFIG_TORTURE_TEST) += torture.o - obj-$(CONFIG_HAS_IOMEM) += memremap.o + obj-$(CONFIG_HAS_IOMEM) += iomem.o + obj-$(CONFIG_ZONE_DEVICE) += memremap.o +obj-$(CONFIG_RSEQ) += rseq.o $(obj)/configs.o: $(obj)/config_data.h pgphsiKSy5d3l.pgp Description: OpenPGP digital signature
Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
+Greg/Alex, @Fegguang/build-bot: I do see mention of Greg and /me in your initial email's body saying TO: Viresh, CC: Greg, but I don't see any of us getting cc'd in your email. Bug ? On 06-06-18, 14:26, Steven Rostedt wrote: > On Wed, 6 Jun 2018 16:26:00 +0200 > Johan Hovold wrote: > > > Looks like the greybus code above is working as intended by checking for > > unterminated string after the strncpy, even if this does now triggers > > the truncation warning. So why exactly are we generating a warning here ? Is it because it is possible that the first n bytes of src may not have the null terminating byte and the dest may not be null terminated eventually ? Maybe I should just use memcpy here then ? But AFAIR, I used strncpy() specifically because it also sets all the remaining bytes after the null terminating byte with the null terminating byte. And so it is pretty easy for me to check if the final string is null terminated by checking [max - 1] byte against '\0', which the code is doing right now. I am not sure what would the best way to get around this incorrect-warning. And I am wondering on why buildbot reported the warning only for two instances in that file, while I have done the same thing at 4 places. > Ah, yes I now see that. Thanks for pointing it out. But perhaps it > should also add the "- 1" to the strncpy() so that gcc doesn't think > it's a mistake. The src string is passed on from a firmware entity and we need to make sure the protocol (greybus) is implemented properly by the other end. For example, in the current case if the firmware sends "HELLOWORLD", its an error as it should have sent "HELLWORLD\0". But with what you are saying we will forcefully make dest as "HELLWORLD\0", which wouldn't be the right thing to do as we will miss the bug present in firmware. -- viresh
Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
+Greg/Alex, @Fegguang/build-bot: I do see mention of Greg and /me in your initial email's body saying TO: Viresh, CC: Greg, but I don't see any of us getting cc'd in your email. Bug ? On 06-06-18, 14:26, Steven Rostedt wrote: > On Wed, 6 Jun 2018 16:26:00 +0200 > Johan Hovold wrote: > > > Looks like the greybus code above is working as intended by checking for > > unterminated string after the strncpy, even if this does now triggers > > the truncation warning. So why exactly are we generating a warning here ? Is it because it is possible that the first n bytes of src may not have the null terminating byte and the dest may not be null terminated eventually ? Maybe I should just use memcpy here then ? But AFAIR, I used strncpy() specifically because it also sets all the remaining bytes after the null terminating byte with the null terminating byte. And so it is pretty easy for me to check if the final string is null terminated by checking [max - 1] byte against '\0', which the code is doing right now. I am not sure what would the best way to get around this incorrect-warning. And I am wondering on why buildbot reported the warning only for two instances in that file, while I have done the same thing at 4 places. > Ah, yes I now see that. Thanks for pointing it out. But perhaps it > should also add the "- 1" to the strncpy() so that gcc doesn't think > it's a mistake. The src string is passed on from a firmware entity and we need to make sure the protocol (greybus) is implemented properly by the other end. For example, in the current case if the firmware sends "HELLOWORLD", its an error as it should have sent "HELLWORLD\0". But with what you are saying we will forcefully make dest as "HELLWORLD\0", which wouldn't be the right thing to do as we will miss the bug present in firmware. -- viresh
Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
On 06-06-18, 09:17, Bjorn Andersson wrote: > On Tue 05 Jun 05:56 PDT 2018, Sricharan R wrote: > > > Hi Vinod, > > > > On 6/5/2018 11:49 AM, Vinod wrote: > > > On 05-06-18, 11:12, Sricharan R wrote: > > > > > >> +config QCOM_Q6V5_WCSS > > >> +tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader" > > >> +depends on OF && ARCH_QCOM > > >> +depends on QCOM_SMEM > > >> +depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n) > > >> +depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > > > > > > Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would > > > happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM > > > > > It says that QCOM_Q6V5_WCSS either must have a compatible state (i.e. > builtin vs builtin, module vs builtin, but not builtin vs module) or > that it's disabled, in which case we will hit the stub functions in > qcom_glink.h. > > I.e. this prevents QCOM_Q6V5_WCSS to be compiled builtin when > RPMSG_QCOM_GLINK_SMEM is module, as this would give us both stubs and > the module. IIUC, you want to have QCOM_Q6V5_WCSS and RPMSG_QCOM_GLINK_SMEM as modules or builtin So, wouldn't Kconfig syntax something like where we say: M if RPMSG_QCOM_GLINK_SMEM=m bool if RPMSG_QCOM_GLINK_SMEM=y Which makes it clear that both these have to be same type? -- ~Vinod
Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
On 06-06-18, 09:17, Bjorn Andersson wrote: > On Tue 05 Jun 05:56 PDT 2018, Sricharan R wrote: > > > Hi Vinod, > > > > On 6/5/2018 11:49 AM, Vinod wrote: > > > On 05-06-18, 11:12, Sricharan R wrote: > > > > > >> +config QCOM_Q6V5_WCSS > > >> +tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader" > > >> +depends on OF && ARCH_QCOM > > >> +depends on QCOM_SMEM > > >> +depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n) > > >> +depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n > > > > > > Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would > > > happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM > > > > > It says that QCOM_Q6V5_WCSS either must have a compatible state (i.e. > builtin vs builtin, module vs builtin, but not builtin vs module) or > that it's disabled, in which case we will hit the stub functions in > qcom_glink.h. > > I.e. this prevents QCOM_Q6V5_WCSS to be compiled builtin when > RPMSG_QCOM_GLINK_SMEM is module, as this would give us both stubs and > the module. IIUC, you want to have QCOM_Q6V5_WCSS and RPMSG_QCOM_GLINK_SMEM as modules or builtin So, wouldn't Kconfig syntax something like where we say: M if RPMSG_QCOM_GLINK_SMEM=m bool if RPMSG_QCOM_GLINK_SMEM=y Which makes it clear that both these have to be same type? -- ~Vinod
[PATCH][RFC] sched: cpufreq: Fix long idle judgement logic in load calculation
According to current code implementation, detecting the long idle period is done by checking if the interval between two adjacent utilization update handers is long enough. Although this mechanism can detect if the idle period is long enough (no utilization hooks invoked during idle period), it might not contain a corner case: if the task has occupied the cpu for too long which causes no context switch during that period, then no utilization handler will be launched until this high prio task is switched out. As a result, the idle_periods field might be calculated incorrectly because it regards the 100% load as 0% and makes the conservative governor who uses this field confusing. Change the judgement to compare the idle_time with sampling_rate directly. Reported-by: Artem S. Tashkinov Cc: Artem S Tashkinov Cc: "Rafael J. Wysocki" Cc: Viresh Kumar Cc: linux...@vger.kernel.org Signed-off-by: Chen Yu --- drivers/cpufreq/cpufreq_governor.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 871bf9c..9792c80 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -165,7 +165,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy) * calls, so the previous load value can be used then. */ load = j_cdbs->prev_load; - } else if (unlikely(time_elapsed > 2 * sampling_rate && + } else if (((int)idle_time > 0) && unlikely(idle_time > 2 * sampling_rate && j_cdbs->prev_load)) { /* * If the CPU had gone completely idle and a task has @@ -185,10 +185,8 @@ unsigned int dbs_update(struct cpufreq_policy *policy) * clear prev_load to guarantee that the load will be * computed again next time. * -* Detecting this situation is easy: the governor's -* utilization update handler would not have run during -* CPU-idle periods. Hence, an unusually large -* 'time_elapsed' (as compared to the sampling rate) +* Detecting this situation is easy: an unusually large +* 'idle_time' (as compared to the sampling rate) * indicates this scenario. */ load = j_cdbs->prev_load; @@ -217,8 +215,8 @@ unsigned int dbs_update(struct cpufreq_policy *policy) j_cdbs->prev_load = load; } - if (time_elapsed > 2 * sampling_rate) { - unsigned int periods = time_elapsed / sampling_rate; + if (((int)idle_time > 0) && (idle_time > 2 * sampling_rate)) { + unsigned int periods = idle_time / sampling_rate; if (periods < idle_periods) idle_periods = periods; -- 2.7.4
[PATCH][RFC] sched: cpufreq: Fix long idle judgement logic in load calculation
According to current code implementation, detecting the long idle period is done by checking if the interval between two adjacent utilization update handers is long enough. Although this mechanism can detect if the idle period is long enough (no utilization hooks invoked during idle period), it might not contain a corner case: if the task has occupied the cpu for too long which causes no context switch during that period, then no utilization handler will be launched until this high prio task is switched out. As a result, the idle_periods field might be calculated incorrectly because it regards the 100% load as 0% and makes the conservative governor who uses this field confusing. Change the judgement to compare the idle_time with sampling_rate directly. Reported-by: Artem S. Tashkinov Cc: Artem S Tashkinov Cc: "Rafael J. Wysocki" Cc: Viresh Kumar Cc: linux...@vger.kernel.org Signed-off-by: Chen Yu --- drivers/cpufreq/cpufreq_governor.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 871bf9c..9792c80 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -165,7 +165,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy) * calls, so the previous load value can be used then. */ load = j_cdbs->prev_load; - } else if (unlikely(time_elapsed > 2 * sampling_rate && + } else if (((int)idle_time > 0) && unlikely(idle_time > 2 * sampling_rate && j_cdbs->prev_load)) { /* * If the CPU had gone completely idle and a task has @@ -185,10 +185,8 @@ unsigned int dbs_update(struct cpufreq_policy *policy) * clear prev_load to guarantee that the load will be * computed again next time. * -* Detecting this situation is easy: the governor's -* utilization update handler would not have run during -* CPU-idle periods. Hence, an unusually large -* 'time_elapsed' (as compared to the sampling rate) +* Detecting this situation is easy: an unusually large +* 'idle_time' (as compared to the sampling rate) * indicates this scenario. */ load = j_cdbs->prev_load; @@ -217,8 +215,8 @@ unsigned int dbs_update(struct cpufreq_policy *policy) j_cdbs->prev_load = load; } - if (time_elapsed > 2 * sampling_rate) { - unsigned int periods = time_elapsed / sampling_rate; + if (((int)idle_time > 0) && (idle_time > 2 * sampling_rate)) { + unsigned int periods = idle_time / sampling_rate; if (periods < idle_periods) idle_periods = periods; -- 2.7.4
Re: Is this a kernel BUG? ///Re: [Question] Can we use SIGRTMIN when vdso disabled on X86?
On 2018/6/7 10:39, Andy Lutomirski wrote: > > >> On Jun 6, 2018, at 7:05 PM, Leizhen (ThunderTown) >> wrote: >> >> >> >>> On 2018/6/7 1:01, Andy Lutomirski wrote: >>> On Wed, Jun 6, 2018 at 2:18 AM Leizhen (ThunderTown) >>> wrote: I found that glibc has already dealt with this case. So this issue must have been met before, should it be maintained by libc/user? if (GLRO(dl_sysinfo_dso) == NULL) { kact.sa_flags |= SA_RESTORER; kact.sa_restorer = ((act->sa_flags & SA_SIGINFO) ? _rt : ); } > On 2018/6/6 15:52, Leizhen (ThunderTown) wrote: > > >> On 2018/6/5 19:24, Leizhen (ThunderTown) wrote: >> After I executed "echo 0 > /proc/sys/abi/vsyscall32" to disable vdso, >> the rt_sigaction01 test case from ltp_2015 failed. >> The test case source code please refer to the attachment, and the output >> as blow: >> >> - >> ./rt_sigaction01 >> rt_sigaction010 TINFO : signal: 34 >> rt_sigaction011 TPASS : rt_sigaction call succeeded: result = 0 >> rt_sigaction010 TINFO : sa.sa_flags = SA_RESETHAND|SA_SIGINFO >> rt_sigaction010 TINFO : Signal Handler Called with signal number >> 34 >> >> Segmentation fault >> -- >> >> >> Is this the desired result? In function ia32_setup_rt_frame, I found >> below code: >> >> if (ksig->ka.sa.sa_flags & SA_RESTORER) >> restorer = ksig->ka.sa.sa_restorer; >> else >> restorer = current->mm->context.vdso + >> vdso_image_32.sym___kernel_rt_sigreturn; >> put_user_ex(ptr_to_compat(restorer), >pretcode); >> >> Because the vdso is disabled, so current->mm->context.vdso is NULL, >> which cause the result of frame->pretcode invalid. >> >> I'm not sure whether this is a kernel bug or just an error of test case >> itself. Can anyone help me? >> > >>> >>> I can't tell from your email what you're testing, what behavior you >>> expect, and what you saw. A program that sets up a signal handler >>> without supplying a restorer will not work if the vDSO is off, and >>> this is by design. >> OK, so that the user should take care whether the vDSO is disabled by itself >> or not, and use different strategies to process it appropriately, like glibc. >> >>> >>> (FWIW, there is a very longstanding libc bug that causes this case to >>> get severely screwed up if the user's SS is not the expected value, >>> and that bug was just fixed very recently. But I doubt this is what >>> you're seeing.) >>> >>> I suppose we could improve the kernel to at least push NULL instead of >>> some random address a bit above 0, but it'll still crash. >> Should we add a warning? Which may help the user to aware this error in time. >> > > It’s entirely valid to have a non working restorer if you never plan to > return from a signal handler. And anyone who writes their own libc should be > able to figure this out on their own, I think. OK. Thanks a lot. > >>> >>> . >>> >> >> -- >> Thanks! >> BestRegards >> > > . > -- Thanks! BestRegards
Re: Is this a kernel BUG? ///Re: [Question] Can we use SIGRTMIN when vdso disabled on X86?
On 2018/6/7 10:39, Andy Lutomirski wrote: > > >> On Jun 6, 2018, at 7:05 PM, Leizhen (ThunderTown) >> wrote: >> >> >> >>> On 2018/6/7 1:01, Andy Lutomirski wrote: >>> On Wed, Jun 6, 2018 at 2:18 AM Leizhen (ThunderTown) >>> wrote: I found that glibc has already dealt with this case. So this issue must have been met before, should it be maintained by libc/user? if (GLRO(dl_sysinfo_dso) == NULL) { kact.sa_flags |= SA_RESTORER; kact.sa_restorer = ((act->sa_flags & SA_SIGINFO) ? _rt : ); } > On 2018/6/6 15:52, Leizhen (ThunderTown) wrote: > > >> On 2018/6/5 19:24, Leizhen (ThunderTown) wrote: >> After I executed "echo 0 > /proc/sys/abi/vsyscall32" to disable vdso, >> the rt_sigaction01 test case from ltp_2015 failed. >> The test case source code please refer to the attachment, and the output >> as blow: >> >> - >> ./rt_sigaction01 >> rt_sigaction010 TINFO : signal: 34 >> rt_sigaction011 TPASS : rt_sigaction call succeeded: result = 0 >> rt_sigaction010 TINFO : sa.sa_flags = SA_RESETHAND|SA_SIGINFO >> rt_sigaction010 TINFO : Signal Handler Called with signal number >> 34 >> >> Segmentation fault >> -- >> >> >> Is this the desired result? In function ia32_setup_rt_frame, I found >> below code: >> >> if (ksig->ka.sa.sa_flags & SA_RESTORER) >> restorer = ksig->ka.sa.sa_restorer; >> else >> restorer = current->mm->context.vdso + >> vdso_image_32.sym___kernel_rt_sigreturn; >> put_user_ex(ptr_to_compat(restorer), >pretcode); >> >> Because the vdso is disabled, so current->mm->context.vdso is NULL, >> which cause the result of frame->pretcode invalid. >> >> I'm not sure whether this is a kernel bug or just an error of test case >> itself. Can anyone help me? >> > >>> >>> I can't tell from your email what you're testing, what behavior you >>> expect, and what you saw. A program that sets up a signal handler >>> without supplying a restorer will not work if the vDSO is off, and >>> this is by design. >> OK, so that the user should take care whether the vDSO is disabled by itself >> or not, and use different strategies to process it appropriately, like glibc. >> >>> >>> (FWIW, there is a very longstanding libc bug that causes this case to >>> get severely screwed up if the user's SS is not the expected value, >>> and that bug was just fixed very recently. But I doubt this is what >>> you're seeing.) >>> >>> I suppose we could improve the kernel to at least push NULL instead of >>> some random address a bit above 0, but it'll still crash. >> Should we add a warning? Which may help the user to aware this error in time. >> > > It’s entirely valid to have a non working restorer if you never plan to > return from a signal handler. And anyone who writes their own libc should be > able to figure this out on their own, I think. OK. Thanks a lot. > >>> >>> . >>> >> >> -- >> Thanks! >> BestRegards >> > > . > -- Thanks! BestRegards
[lkp-robot] [x86] 1a39381d70: WARNING:at_kernel/locking/mutex.c:#__mutex_unlock_slowpath
FYI, we noticed the following commit (built with gcc-7): commit: 1a39381d7f0097ec6e2ceb75812d6c00b2f1 ("x86: alternatives: macrofy locks for better inlining") url: https://github.com/0day-ci/linux/commits/Nadav-Amit/x86-macrofying-inline-asm-for-better-compilation/20180605-124313 in testcase: trinity with following parameters: runtime: 300s test-description: Trinity is a linux system call fuzz tester. test-url: http://codemonkey.org.uk/projects/trinity/ on test machine: qemu-system-x86_64 -enable-kvm -cpu Haswell,+smep,+smap -m 512M caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace): +-+++ | | 2c1316126e | 1a39381d70 | +-+++ | boot_successes | 0 | 0 | | boot_failures | 18 | 17 | | WARNING:at_lib/debugobjects.c:#__debug_object_init | 18 || | RIP:__debug_object_init | 18 || | WARNING:at_kernel/locking/mutex.c:#__mutex_unlock_slowpath | 0 | 17 | | RIP:__mutex_unlock_slowpath | 0 | 17 | | WARNING:at_arch/x86/kernel/idt.c:#update_intr_gate | 0 | 17 | | RIP:update_intr_gate | 0 | 17 | | page_allocation_failure:order:#,mode:#(),nodemask=(null) | 0 | 17 | | Mem-Info | 0 | 17 | | Kernel_panic-not_syncing:kmem_cache_create:Failed_to_create_slab'radix_tree_node'.Error | 0 | 17 | +-+++ [0.00] WARNING: CPU: 0 PID: 0 at kernel/locking/mutex.c:1032 __mutex_unlock_slowpath+0x1ff/0x2a0 [0.00] Modules linked in: [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0-4-g1a39381 #2 [0.00] RIP: 0010:__mutex_unlock_slowpath+0x1ff/0x2a0 [0.00] RSP: :b4a03df0 EFLAGS: 00010082 ORIG_RAX: [0.00] RAX: 0033 RBX: RCX: b4a797c0 [0.00] RDX: b36b41de RSI: 0001 RDI: 0046 [0.00] RBP: b4a03e30 R08: 0001 R09: [0.00] R10: R11: R12: b4c789e0 [0.00] R13: b4a03df8 R14: b48dcfb0 R15: [0.00] FS: () GS:b4a8e000() knlGS: [0.00] CS: 0010 DS: ES: CR0: 80050033 [0.00] CR2: a2951a312000 CR3: 18a74000 CR4: 000606b0 [0.00] Call Trace: [0.00] mutex_unlock+0x12/0x20 [0.00] __clocksource_register_scale+0xda/0x120 [0.00] kvmclock_init+0x22d/0x23f [0.00] setup_arch+0xa20/0xaf6 [0.00] start_kernel+0x6a/0x4dc [0.00] ? copy_bootdata+0x1f/0xb8 [0.00] x86_64_start_reservations+0x24/0x26 [0.00] x86_64_start_kernel+0x73/0x76 [0.00] secondary_startup_64+0xa5/0xb0 [0.00] Code: 0f 0b e9 ae fe ff ff e8 d0 15 a5 ff 85 c0 74 1d 44 8b 15 1d 0c 8f 01 45 85 d2 75 11 4c 89 f6 48 c7 c7 53 b4 8c b4 e8 81 3a 5c ff <0f> 0b 44 8b 0d c0 b2 7c 01 45 85 c9 0f 84 6f fe ff ff e9 73 fe [0.00] random: get_random_bytes called from print_oops_end_marker+0x3f/0x60 with crng_init=0 [0.00] ---[ end trace a9c261981ad74140 ]--- To reproduce: git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k job-script # job-script is attached in this email Thanks, Xiaolong # # Automatically generated file; DO NOT EDIT. # Linux/x86_64 4.17.0 Kernel Configuration # CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT="elf64-x86-64" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_MMU=y CONFIG_ARCH_MMAP_RND_BITS_MIN=28 CONFIG_ARCH_MMAP_RND_BITS_MAX=32 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16 CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y
[lkp-robot] [x86] 1a39381d70: WARNING:at_kernel/locking/mutex.c:#__mutex_unlock_slowpath
FYI, we noticed the following commit (built with gcc-7): commit: 1a39381d7f0097ec6e2ceb75812d6c00b2f1 ("x86: alternatives: macrofy locks for better inlining") url: https://github.com/0day-ci/linux/commits/Nadav-Amit/x86-macrofying-inline-asm-for-better-compilation/20180605-124313 in testcase: trinity with following parameters: runtime: 300s test-description: Trinity is a linux system call fuzz tester. test-url: http://codemonkey.org.uk/projects/trinity/ on test machine: qemu-system-x86_64 -enable-kvm -cpu Haswell,+smep,+smap -m 512M caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace): +-+++ | | 2c1316126e | 1a39381d70 | +-+++ | boot_successes | 0 | 0 | | boot_failures | 18 | 17 | | WARNING:at_lib/debugobjects.c:#__debug_object_init | 18 || | RIP:__debug_object_init | 18 || | WARNING:at_kernel/locking/mutex.c:#__mutex_unlock_slowpath | 0 | 17 | | RIP:__mutex_unlock_slowpath | 0 | 17 | | WARNING:at_arch/x86/kernel/idt.c:#update_intr_gate | 0 | 17 | | RIP:update_intr_gate | 0 | 17 | | page_allocation_failure:order:#,mode:#(),nodemask=(null) | 0 | 17 | | Mem-Info | 0 | 17 | | Kernel_panic-not_syncing:kmem_cache_create:Failed_to_create_slab'radix_tree_node'.Error | 0 | 17 | +-+++ [0.00] WARNING: CPU: 0 PID: 0 at kernel/locking/mutex.c:1032 __mutex_unlock_slowpath+0x1ff/0x2a0 [0.00] Modules linked in: [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0-4-g1a39381 #2 [0.00] RIP: 0010:__mutex_unlock_slowpath+0x1ff/0x2a0 [0.00] RSP: :b4a03df0 EFLAGS: 00010082 ORIG_RAX: [0.00] RAX: 0033 RBX: RCX: b4a797c0 [0.00] RDX: b36b41de RSI: 0001 RDI: 0046 [0.00] RBP: b4a03e30 R08: 0001 R09: [0.00] R10: R11: R12: b4c789e0 [0.00] R13: b4a03df8 R14: b48dcfb0 R15: [0.00] FS: () GS:b4a8e000() knlGS: [0.00] CS: 0010 DS: ES: CR0: 80050033 [0.00] CR2: a2951a312000 CR3: 18a74000 CR4: 000606b0 [0.00] Call Trace: [0.00] mutex_unlock+0x12/0x20 [0.00] __clocksource_register_scale+0xda/0x120 [0.00] kvmclock_init+0x22d/0x23f [0.00] setup_arch+0xa20/0xaf6 [0.00] start_kernel+0x6a/0x4dc [0.00] ? copy_bootdata+0x1f/0xb8 [0.00] x86_64_start_reservations+0x24/0x26 [0.00] x86_64_start_kernel+0x73/0x76 [0.00] secondary_startup_64+0xa5/0xb0 [0.00] Code: 0f 0b e9 ae fe ff ff e8 d0 15 a5 ff 85 c0 74 1d 44 8b 15 1d 0c 8f 01 45 85 d2 75 11 4c 89 f6 48 c7 c7 53 b4 8c b4 e8 81 3a 5c ff <0f> 0b 44 8b 0d c0 b2 7c 01 45 85 c9 0f 84 6f fe ff ff e9 73 fe [0.00] random: get_random_bytes called from print_oops_end_marker+0x3f/0x60 with crng_init=0 [0.00] ---[ end trace a9c261981ad74140 ]--- To reproduce: git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k job-script # job-script is attached in this email Thanks, Xiaolong # # Automatically generated file; DO NOT EDIT. # Linux/x86_64 4.17.0 Kernel Configuration # CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT="elf64-x86-64" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_MMU=y CONFIG_ARCH_MMAP_RND_BITS_MIN=28 CONFIG_ARCH_MMAP_RND_BITS_MAX=32 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16 CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y
[PATCH v1 0/4] clk: rockchip: support clock controller for px30 SoC
Elaine Zhang (4): dt-bindings: add bindings for px30 clock controller clk: rockchip: add dt-binding header for px30 clk: rockchip: add support for half divider clk: rockchip: add clock controller for px30 .../bindings/clock/rockchip,px30-cru.txt | 67 ++ drivers/clk/rockchip/Makefile |2 + drivers/clk/rockchip/clk-half-divider.c| 235 + drivers/clk/rockchip/clk-px30.c| 1080 drivers/clk/rockchip/clk.c | 10 + drivers/clk/rockchip/clk.h | 86 +- include/dt-bindings/clock/px30-cru.h | 402 7 files changed, 1881 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt create mode 100644 drivers/clk/rockchip/clk-half-divider.c create mode 100644 drivers/clk/rockchip/clk-px30.c create mode 100644 include/dt-bindings/clock/px30-cru.h -- 1.9.1
[PATCH v1 2/4] clk: rockchip: add dt-binding header for px30
Add the dt-bindings header for the px30, that gets shared between the clock controller and the clock references in the dts. Add softreset ID for px30. Signed-off-by: Elaine Zhang --- include/dt-bindings/clock/px30-cru.h | 402 +++ 1 file changed, 402 insertions(+) create mode 100644 include/dt-bindings/clock/px30-cru.h diff --git a/include/dt-bindings/clock/px30-cru.h b/include/dt-bindings/clock/px30-cru.h new file mode 100644 index ..db9fc2a0bb21 --- /dev/null +++ b/include/dt-bindings/clock/px30-cru.h @@ -0,0 +1,402 @@ +/* + * Copyright (c) 2017 Rockchip Electronics Co. Ltd. + * Author: Elaine + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _DT_BINDINGS_CLK_ROCKCHIP_PX30_H +#define _DT_BINDINGS_CLK_ROCKCHIP_PX30_H + +/* core clocks */ +#define PLL_APLL 1 +#define PLL_DPLL 2 +#define PLL_CPLL 3 +#define PLL_NPLL 4 +#define APLL_BOOST_H 5 +#define APLL_BOOST_L 6 +#define ARMCLK 7 + +/* sclk gates (special clocks) */ +#define USB480M14 +#define SCLK_PDM 15 +#define SCLK_I2S0_TX 16 +#define SCLK_I2S0_TX_OUT 17 +#define SCLK_I2S0_RX 18 +#define SCLK_I2S0_RX_OUT 19 +#define SCLK_I2S1 20 +#define SCLK_I2S1_OUT 21 +#define SCLK_I2S2 22 +#define SCLK_I2S2_OUT 23 +#define SCLK_UART1 24 +#define SCLK_UART2 25 +#define SCLK_UART3 26 +#define SCLK_UART4 27 +#define SCLK_UART5 28 +#define SCLK_I2C0 29 +#define SCLK_I2C1 30 +#define SCLK_I2C2 31 +#define SCLK_I2C3 32 +#define SCLK_I2C4 33 +#define SCLK_PWM0 34 +#define SCLK_PWM1 35 +#define SCLK_SPI0 36 +#define SCLK_SPI1 37 +#define SCLK_TIMER038 +#define SCLK_TIMER139 +#define SCLK_TIMER240 +#define SCLK_TIMER341 +#define SCLK_TIMER442 +#define SCLK_TIMER543 +#define SCLK_TSADC 44 +#define SCLK_SARADC45 +#define SCLK_OTP 46 +#define SCLK_OTP_USR 47 +#define SCLK_CRYPTO48 +#define SCLK_CRYPTO_APK49 +#define SCLK_DDRC 50 +#define SCLK_ISP 51 +#define SCLK_CIF_OUT 52 +#define SCLK_RGA_CORE 53 +#define SCLK_VOPB_PWM 54 +#define SCLK_NANDC 55 +#define SCLK_SDIO 56 +#define SCLK_EMMC 57 +#define SCLK_SFC 58 +#define SCLK_SDMMC 59 +#define SCLK_OTG_ADP 60 +#define SCLK_GMAC_SRC 61 +#define SCLK_GMAC 62 +#define SCLK_GMAC_RX_TX63 +#define SCLK_MAC_REF 64 +#define SCLK_MAC_REFOUT65 +#define SCLK_MAC_OUT 66 +#define SCLK_SDMMC_DRV 67 +#define SCLK_SDMMC_SAMPLE 68 +#define SCLK_SDIO_DRV 69 +#define SCLK_SDIO_SAMPLE 70 +#define SCLK_EMMC_DRV 71 +#define SCLK_EMMC_SAMPLE 72 +#define SCLK_GPU 73 +#define SCLK_PVTM 74 +#define SCLK_CORE_VPU 75 +#define SCLK_GMAC_RMII 76 +#define SCLK_UART2_SRC 77 +#define SCLK_NANDC_DIV 78 +#define SCLK_NANDC_DIV50 79 +#define SCLK_SDIO_DIV 80 +#define SCLK_SDIO_DIV5081 +#define SCLK_EMMC_DIV 82 +#define SCLK_EMMC_DIV5083 +#define SCLK_DDRCLK84 +#define SCLK_UART1_SRC 85 + +/* dclk gates */ +#define DCLK_VOPB 150 +#define DCLK_VOPL 151 + +/* aclk gates */ +#define ACLK_GPU 170 +#define ACLK_BUS_PRE 171 +#define ACLK_CRYPTO172 +#define ACLK_VI_PRE173 +#define ACLK_VO_PRE174 +#define ACLK_VPU 175 +#define ACLK_PERI_PRE 176 +#define ACLK_GMAC 178 +#define ACLK_CIF 179 +#define ACLK_ISP 180 +#define ACLK_VOPB 181 +#define ACLK_VOPL 182 +#define ACLK_RGA 183 +#define ACLK_GIC 184 +#define ACLK_DCF 186 +#define ACLK_DMAC 187 +#define ACLK_BUS_SRC 188 +#define ACLK_PERI_SRC 189 + +/* hclk gates */ +#define HCLK_BUS_PRE 240 +#define HCLK_CRYPTO241 +#define HCLK_VI_PRE
[PATCH v1 0/4] clk: rockchip: support clock controller for px30 SoC
Elaine Zhang (4): dt-bindings: add bindings for px30 clock controller clk: rockchip: add dt-binding header for px30 clk: rockchip: add support for half divider clk: rockchip: add clock controller for px30 .../bindings/clock/rockchip,px30-cru.txt | 67 ++ drivers/clk/rockchip/Makefile |2 + drivers/clk/rockchip/clk-half-divider.c| 235 + drivers/clk/rockchip/clk-px30.c| 1080 drivers/clk/rockchip/clk.c | 10 + drivers/clk/rockchip/clk.h | 86 +- include/dt-bindings/clock/px30-cru.h | 402 7 files changed, 1881 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt create mode 100644 drivers/clk/rockchip/clk-half-divider.c create mode 100644 drivers/clk/rockchip/clk-px30.c create mode 100644 include/dt-bindings/clock/px30-cru.h -- 1.9.1
[PATCH v1 2/4] clk: rockchip: add dt-binding header for px30
Add the dt-bindings header for the px30, that gets shared between the clock controller and the clock references in the dts. Add softreset ID for px30. Signed-off-by: Elaine Zhang --- include/dt-bindings/clock/px30-cru.h | 402 +++ 1 file changed, 402 insertions(+) create mode 100644 include/dt-bindings/clock/px30-cru.h diff --git a/include/dt-bindings/clock/px30-cru.h b/include/dt-bindings/clock/px30-cru.h new file mode 100644 index ..db9fc2a0bb21 --- /dev/null +++ b/include/dt-bindings/clock/px30-cru.h @@ -0,0 +1,402 @@ +/* + * Copyright (c) 2017 Rockchip Electronics Co. Ltd. + * Author: Elaine + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _DT_BINDINGS_CLK_ROCKCHIP_PX30_H +#define _DT_BINDINGS_CLK_ROCKCHIP_PX30_H + +/* core clocks */ +#define PLL_APLL 1 +#define PLL_DPLL 2 +#define PLL_CPLL 3 +#define PLL_NPLL 4 +#define APLL_BOOST_H 5 +#define APLL_BOOST_L 6 +#define ARMCLK 7 + +/* sclk gates (special clocks) */ +#define USB480M14 +#define SCLK_PDM 15 +#define SCLK_I2S0_TX 16 +#define SCLK_I2S0_TX_OUT 17 +#define SCLK_I2S0_RX 18 +#define SCLK_I2S0_RX_OUT 19 +#define SCLK_I2S1 20 +#define SCLK_I2S1_OUT 21 +#define SCLK_I2S2 22 +#define SCLK_I2S2_OUT 23 +#define SCLK_UART1 24 +#define SCLK_UART2 25 +#define SCLK_UART3 26 +#define SCLK_UART4 27 +#define SCLK_UART5 28 +#define SCLK_I2C0 29 +#define SCLK_I2C1 30 +#define SCLK_I2C2 31 +#define SCLK_I2C3 32 +#define SCLK_I2C4 33 +#define SCLK_PWM0 34 +#define SCLK_PWM1 35 +#define SCLK_SPI0 36 +#define SCLK_SPI1 37 +#define SCLK_TIMER038 +#define SCLK_TIMER139 +#define SCLK_TIMER240 +#define SCLK_TIMER341 +#define SCLK_TIMER442 +#define SCLK_TIMER543 +#define SCLK_TSADC 44 +#define SCLK_SARADC45 +#define SCLK_OTP 46 +#define SCLK_OTP_USR 47 +#define SCLK_CRYPTO48 +#define SCLK_CRYPTO_APK49 +#define SCLK_DDRC 50 +#define SCLK_ISP 51 +#define SCLK_CIF_OUT 52 +#define SCLK_RGA_CORE 53 +#define SCLK_VOPB_PWM 54 +#define SCLK_NANDC 55 +#define SCLK_SDIO 56 +#define SCLK_EMMC 57 +#define SCLK_SFC 58 +#define SCLK_SDMMC 59 +#define SCLK_OTG_ADP 60 +#define SCLK_GMAC_SRC 61 +#define SCLK_GMAC 62 +#define SCLK_GMAC_RX_TX63 +#define SCLK_MAC_REF 64 +#define SCLK_MAC_REFOUT65 +#define SCLK_MAC_OUT 66 +#define SCLK_SDMMC_DRV 67 +#define SCLK_SDMMC_SAMPLE 68 +#define SCLK_SDIO_DRV 69 +#define SCLK_SDIO_SAMPLE 70 +#define SCLK_EMMC_DRV 71 +#define SCLK_EMMC_SAMPLE 72 +#define SCLK_GPU 73 +#define SCLK_PVTM 74 +#define SCLK_CORE_VPU 75 +#define SCLK_GMAC_RMII 76 +#define SCLK_UART2_SRC 77 +#define SCLK_NANDC_DIV 78 +#define SCLK_NANDC_DIV50 79 +#define SCLK_SDIO_DIV 80 +#define SCLK_SDIO_DIV5081 +#define SCLK_EMMC_DIV 82 +#define SCLK_EMMC_DIV5083 +#define SCLK_DDRCLK84 +#define SCLK_UART1_SRC 85 + +/* dclk gates */ +#define DCLK_VOPB 150 +#define DCLK_VOPL 151 + +/* aclk gates */ +#define ACLK_GPU 170 +#define ACLK_BUS_PRE 171 +#define ACLK_CRYPTO172 +#define ACLK_VI_PRE173 +#define ACLK_VO_PRE174 +#define ACLK_VPU 175 +#define ACLK_PERI_PRE 176 +#define ACLK_GMAC 178 +#define ACLK_CIF 179 +#define ACLK_ISP 180 +#define ACLK_VOPB 181 +#define ACLK_VOPL 182 +#define ACLK_RGA 183 +#define ACLK_GIC 184 +#define ACLK_DCF 186 +#define ACLK_DMAC 187 +#define ACLK_BUS_SRC 188 +#define ACLK_PERI_SRC 189 + +/* hclk gates */ +#define HCLK_BUS_PRE 240 +#define HCLK_CRYPTO241 +#define HCLK_VI_PRE
[PATCH v1 3/4] clk: rockchip: add support for half divider
The new Rockchip socs have optional half divider, so we use "branch_half_divider" + "COMPOSITE_NOMUX_HALFDIV \ DIV_HALF" to hook that special divider clock-type into our clock-tree. Signed-off-by: Elaine Zhang --- drivers/clk/rockchip/Makefile | 1 + drivers/clk/rockchip/clk-half-divider.c | 235 drivers/clk/rockchip/clk.c | 10 ++ drivers/clk/rockchip/clk.h | 45 ++ 4 files changed, 291 insertions(+) create mode 100644 drivers/clk/rockchip/clk-half-divider.c diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile index 59b8d320960a..023f83ad3429 100644 --- a/drivers/clk/rockchip/Makefile +++ b/drivers/clk/rockchip/Makefile @@ -7,6 +7,7 @@ obj-y += clk-rockchip.o obj-y += clk.o obj-y += clk-pll.o obj-y += clk-cpu.o +obj-y += clk-half-divider.o obj-y += clk-inverter.o obj-y += clk-mmc-phase.o obj-y += clk-muxgrf.o diff --git a/drivers/clk/rockchip/clk-half-divider.c b/drivers/clk/rockchip/clk-half-divider.c new file mode 100644 index ..23830de254ec --- /dev/null +++ b/drivers/clk/rockchip/clk-half-divider.c @@ -0,0 +1,235 @@ +/* + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include "clk.h" + +#define div_mask(width)((1 << (width)) - 1) + +static bool _is_best_half_div(unsigned long rate, unsigned long now, + unsigned long best, unsigned long flags) +{ + if (flags & CLK_DIVIDER_ROUND_CLOSEST) + return abs(rate - now) < abs(rate - best); + + return now <= rate && now > best; +} + +static unsigned long clk_half_divider_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct clk_divider *divider = to_clk_divider(hw); + unsigned int val; + + val = clk_readl(divider->reg) >> divider->shift; + val &= div_mask(divider->width); + val = val * 2 + 3; + + return DIV_ROUND_UP_ULL((u64)(parent_rate * 2), val); +} + +static int clk_half_divider_bestdiv(struct clk_hw *hw, unsigned long rate, + unsigned long *best_parent_rate, u8 width, + unsigned long flags) +{ + int i, bestdiv = 0; + unsigned long parent_rate, best = 0, now, maxdiv; + unsigned long parent_rate_saved = *best_parent_rate; + + if (!rate) + rate = 1; + + maxdiv = div_mask(width); + + if (!(clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) { + parent_rate = *best_parent_rate; + bestdiv = DIV_ROUND_UP_ULL((u64)(parent_rate * 2), rate); + if (bestdiv < 3) + bestdiv = 0; + else + bestdiv = (bestdiv - 3) / 2; + bestdiv = bestdiv > maxdiv ? maxdiv : bestdiv; + return bestdiv; + } + + /* +* The maximum divider we can use without overflowing +* unsigned long in rate * i below +*/ + maxdiv = min(ULONG_MAX / rate, maxdiv); + + for (i = 0; i <= maxdiv; i++) { + if ((rate * (i * 2 + 3)) == (parent_rate_saved * 2)) { + /* +* It's the most ideal case if the requested rate can be +* divided from parent clock without needing to change +* parent rate, so return the divider immediately. +*/ + *best_parent_rate = parent_rate_saved; + return i; + } + parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), + (rate * (i * 2 + 3)) / 2); + now = DIV_ROUND_UP_ULL((u64)(parent_rate * 2), (i * 2 + 3)); + if (_is_best_half_div(rate, now, best, flags)) { + bestdiv = i; + best = now; + *best_parent_rate = parent_rate; + } + } + + if (!bestdiv) { + bestdiv = div_mask(width); + *best_parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), 1); + } + + return bestdiv; +} + +static long clk_half_divider_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + struct clk_divider *divider = to_clk_divider(hw); + int div; + + div = clk_half_divider_bestdiv(hw, rate, prate,
[PATCH v1 4/4] clk: rockchip: add clock controller for px30
Add the clock tree definition for the new px30 SoC. Signed-off-by: Elaine Zhang --- drivers/clk/rockchip/Makefile |1 + drivers/clk/rockchip/clk-px30.c | 1080 +++ drivers/clk/rockchip/clk.h | 41 +- 3 files changed, 1121 insertions(+), 1 deletion(-) create mode 100644 drivers/clk/rockchip/clk-px30.c diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile index 023f83ad3429..82ab6f515179 100644 --- a/drivers/clk/rockchip/Makefile +++ b/drivers/clk/rockchip/Makefile @@ -14,6 +14,7 @@ obj-y += clk-muxgrf.o obj-y += clk-ddr.o obj-$(CONFIG_RESET_CONTROLLER) += softrst.o +obj-y += clk-px30.o obj-y += clk-rv1108.o obj-y += clk-rk3036.o obj-y += clk-rk3128.o diff --git a/drivers/clk/rockchip/clk-px30.c b/drivers/clk/rockchip/clk-px30.c new file mode 100644 index ..07105fe1ff6c --- /dev/null +++ b/drivers/clk/rockchip/clk-px30.c @@ -0,0 +1,1080 @@ +/* + * Copyright (c) 2016 Rockchip Electronics Co. Ltd. + * Author: Elaine + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include "clk.h" + +#define PX30_GRF_SOC_STATUS0 0x480 + +enum px30_plls { + apll, dpll, cpll, npll, apll_b_h, apll_b_l, +}; + +enum px30_pmu_plls { + gpll, +}; + +static struct rockchip_pll_rate_table px30_pll_rates[] = { + /* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */ + RK3036_PLL_RATE(160800, 1, 67, 1, 1, 1, 0), + RK3036_PLL_RATE(158400, 1, 66, 1, 1, 1, 0), + RK3036_PLL_RATE(156000, 1, 65, 1, 1, 1, 0), + RK3036_PLL_RATE(153600, 1, 64, 1, 1, 1, 0), + RK3036_PLL_RATE(151200, 1, 63, 1, 1, 1, 0), + RK3036_PLL_RATE(148800, 1, 62, 1, 1, 1, 0), + RK3036_PLL_RATE(146400, 1, 61, 1, 1, 1, 0), + RK3036_PLL_RATE(144000, 1, 60, 1, 1, 1, 0), + RK3036_PLL_RATE(141600, 1, 59, 1, 1, 1, 0), + RK3036_PLL_RATE(139200, 1, 58, 1, 1, 1, 0), + RK3036_PLL_RATE(136800, 1, 57, 1, 1, 1, 0), + RK3036_PLL_RATE(134400, 1, 56, 1, 1, 1, 0), + RK3036_PLL_RATE(132000, 1, 55, 1, 1, 1, 0), + RK3036_PLL_RATE(129600, 1, 54, 1, 1, 1, 0), + RK3036_PLL_RATE(127200, 1, 53, 1, 1, 1, 0), + RK3036_PLL_RATE(124800, 1, 52, 1, 1, 1, 0), + RK3036_PLL_RATE(12, 1, 50, 1, 1, 1, 0), + RK3036_PLL_RATE(118800, 2, 99, 1, 1, 1, 0), + RK3036_PLL_RATE(110400, 1, 46, 1, 1, 1, 0), + RK3036_PLL_RATE(11, 12, 550, 1, 1, 1, 0), + RK3036_PLL_RATE(100800, 1, 84, 2, 1, 1, 0), + RK3036_PLL_RATE(10, 6, 500, 2, 1, 1, 0), + RK3036_PLL_RATE(98400, 1, 82, 2, 1, 1, 0), + RK3036_PLL_RATE(96000, 1, 80, 2, 1, 1, 0), + RK3036_PLL_RATE(93600, 1, 78, 2, 1, 1, 0), + RK3036_PLL_RATE(91200, 1, 76, 2, 1, 1, 0), + RK3036_PLL_RATE(9, 4, 300, 2, 1, 1, 0), + RK3036_PLL_RATE(88800, 1, 74, 2, 1, 1, 0), + RK3036_PLL_RATE(86400, 1, 72, 2, 1, 1, 0), + RK3036_PLL_RATE(84000, 1, 70, 2, 1, 1, 0), + RK3036_PLL_RATE(81600, 1, 68, 2, 1, 1, 0), + RK3036_PLL_RATE(8, 6, 400, 2, 1, 1, 0), + RK3036_PLL_RATE(7, 6, 350, 2, 1, 1, 0), + RK3036_PLL_RATE(69600, 1, 58, 2, 1, 1, 0), + RK3036_PLL_RATE(62400, 1, 52, 2, 1, 1, 0), + RK3036_PLL_RATE(6, 1, 75, 3, 1, 1, 0), + RK3036_PLL_RATE(59400, 2, 99, 2, 1, 1, 0), + RK3036_PLL_RATE(50400, 1, 63, 3, 1, 1, 0), + RK3036_PLL_RATE(5, 6, 250, 2, 1, 1, 0), + RK3036_PLL_RATE(40800, 1, 68, 2, 2, 1, 0), + RK3036_PLL_RATE(31200, 1, 52, 2, 2, 1, 0), + RK3036_PLL_RATE(21600, 1, 72, 4, 2, 1, 0), + RK3036_PLL_RATE(9600, 1, 64, 4, 4, 1, 0), + { /* sentinel */ }, +}; + +#define PX30_DIV_ACLKM_MASK0x7 +#define PX30_DIV_ACLKM_SHIFT 12 +#define PX30_DIV_PCLK_DBG_MASK 0xf +#define PX30_DIV_PCLK_DBG_SHIFT8 + +#define PX30_CLKSEL0(_aclk_core, _pclk_dbg)\ +{ \ + .reg = PX30_CLKSEL_CON(0), \ + .val = HIWORD_UPDATE(_aclk_core, PX30_DIV_ACLKM_MASK, \ +PX30_DIV_ACLKM_SHIFT) |\ + HIWORD_UPDATE(_pclk_dbg, PX30_DIV_PCLK_DBG_MASK, \ +
[PATCH v1 1/4] dt-bindings: add bindings for px30 clock controller
Add devicetree bindings for Rockchip cru which found on Rockchip SoCs. Signed-off-by: Elaine Zhang --- .../bindings/clock/rockchip,px30-cru.txt | 67 ++ 1 file changed, 67 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt diff --git a/Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt b/Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt new file mode 100644 index ..af5a45b680d0 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt @@ -0,0 +1,67 @@ +* Rockchip PX30 Clock and Reset Unit + +The PX30 clock controller generates and supplies clock to various +controllers within the SoC and also implements a reset controller for SoC +peripherals. + +Required Properties: + +- compatible: PMU for CRU should be "rockchip,px30-pmu-cru" +- compatible: CRU should be "rockchip,px30-cru" +- reg: physical base address of the controller and length of memory mapped + region. +- #clock-cells: should be 1. +- #reset-cells: should be 1. + +Optional Properties: + +- rockchip,grf: phandle to the syscon managing the "general register files" + If missing, pll rates are not changeable, due to the missing pll lock status. + +Each clock is assigned an identifier and client nodes can use this identifier +to specify the clock which they consume. All available clocks are defined as +preprocessor macros in the dt-bindings/clock/px30-cru.h headers and can be +used in device tree sources. Similar macros exist for the reset sources in +these files. + +External clocks: + +There are several clocks that are generated outside the SoC. It is expected +that they are defined using standard clock bindings with following +clock-output-names: + - "xin24m" - crystal input - required, + - "xin32k" - rtc clock - optional, + - "i2sx_clkin" - external I2S clock - optional, + - "gmac_clkin" - external GMAC clock - optional + +Example: Clock controller node: + + pmucru: pmu-clock-controller@ff2bc000 { + compatible = "rockchip,px30-pmucru"; + reg = <0x0 0xff2bc000 0x0 0x1000>; + #clock-cells = <1>; + #reset-cells = <1>; + }; + + cru: clock-controller@ff2b { + compatible = "rockchip,px30-cru"; + reg = <0x0 0xff2b 0x0 0x1000>; + rockchip,grf = <>; + #clock-cells = <1>; + #reset-cells = <1>; + }; + +Example: UART controller node that consumes the clock generated by the clock + controller: + + uart0: serial@ff03 { + compatible = "rockchip,px30-uart", "snps,dw-apb-uart"; + reg = <0x0 0xff03 0x0 0x100>; + interrupts = ; + clocks = < SCLK_UART0_PMU>, < PCLK_UART0_PMU>; + clock-names = "baudclk", "apb_pclk"; + reg-shift = <2>; + reg-io-width = <4>; + status = "disabled"; + }; + -- 1.9.1
[PATCH v1 3/4] clk: rockchip: add support for half divider
The new Rockchip socs have optional half divider, so we use "branch_half_divider" + "COMPOSITE_NOMUX_HALFDIV \ DIV_HALF" to hook that special divider clock-type into our clock-tree. Signed-off-by: Elaine Zhang --- drivers/clk/rockchip/Makefile | 1 + drivers/clk/rockchip/clk-half-divider.c | 235 drivers/clk/rockchip/clk.c | 10 ++ drivers/clk/rockchip/clk.h | 45 ++ 4 files changed, 291 insertions(+) create mode 100644 drivers/clk/rockchip/clk-half-divider.c diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile index 59b8d320960a..023f83ad3429 100644 --- a/drivers/clk/rockchip/Makefile +++ b/drivers/clk/rockchip/Makefile @@ -7,6 +7,7 @@ obj-y += clk-rockchip.o obj-y += clk.o obj-y += clk-pll.o obj-y += clk-cpu.o +obj-y += clk-half-divider.o obj-y += clk-inverter.o obj-y += clk-mmc-phase.o obj-y += clk-muxgrf.o diff --git a/drivers/clk/rockchip/clk-half-divider.c b/drivers/clk/rockchip/clk-half-divider.c new file mode 100644 index ..23830de254ec --- /dev/null +++ b/drivers/clk/rockchip/clk-half-divider.c @@ -0,0 +1,235 @@ +/* + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include "clk.h" + +#define div_mask(width)((1 << (width)) - 1) + +static bool _is_best_half_div(unsigned long rate, unsigned long now, + unsigned long best, unsigned long flags) +{ + if (flags & CLK_DIVIDER_ROUND_CLOSEST) + return abs(rate - now) < abs(rate - best); + + return now <= rate && now > best; +} + +static unsigned long clk_half_divider_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct clk_divider *divider = to_clk_divider(hw); + unsigned int val; + + val = clk_readl(divider->reg) >> divider->shift; + val &= div_mask(divider->width); + val = val * 2 + 3; + + return DIV_ROUND_UP_ULL((u64)(parent_rate * 2), val); +} + +static int clk_half_divider_bestdiv(struct clk_hw *hw, unsigned long rate, + unsigned long *best_parent_rate, u8 width, + unsigned long flags) +{ + int i, bestdiv = 0; + unsigned long parent_rate, best = 0, now, maxdiv; + unsigned long parent_rate_saved = *best_parent_rate; + + if (!rate) + rate = 1; + + maxdiv = div_mask(width); + + if (!(clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) { + parent_rate = *best_parent_rate; + bestdiv = DIV_ROUND_UP_ULL((u64)(parent_rate * 2), rate); + if (bestdiv < 3) + bestdiv = 0; + else + bestdiv = (bestdiv - 3) / 2; + bestdiv = bestdiv > maxdiv ? maxdiv : bestdiv; + return bestdiv; + } + + /* +* The maximum divider we can use without overflowing +* unsigned long in rate * i below +*/ + maxdiv = min(ULONG_MAX / rate, maxdiv); + + for (i = 0; i <= maxdiv; i++) { + if ((rate * (i * 2 + 3)) == (parent_rate_saved * 2)) { + /* +* It's the most ideal case if the requested rate can be +* divided from parent clock without needing to change +* parent rate, so return the divider immediately. +*/ + *best_parent_rate = parent_rate_saved; + return i; + } + parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), + (rate * (i * 2 + 3)) / 2); + now = DIV_ROUND_UP_ULL((u64)(parent_rate * 2), (i * 2 + 3)); + if (_is_best_half_div(rate, now, best, flags)) { + bestdiv = i; + best = now; + *best_parent_rate = parent_rate; + } + } + + if (!bestdiv) { + bestdiv = div_mask(width); + *best_parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), 1); + } + + return bestdiv; +} + +static long clk_half_divider_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + struct clk_divider *divider = to_clk_divider(hw); + int div; + + div = clk_half_divider_bestdiv(hw, rate, prate,
[PATCH v1 4/4] clk: rockchip: add clock controller for px30
Add the clock tree definition for the new px30 SoC. Signed-off-by: Elaine Zhang --- drivers/clk/rockchip/Makefile |1 + drivers/clk/rockchip/clk-px30.c | 1080 +++ drivers/clk/rockchip/clk.h | 41 +- 3 files changed, 1121 insertions(+), 1 deletion(-) create mode 100644 drivers/clk/rockchip/clk-px30.c diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile index 023f83ad3429..82ab6f515179 100644 --- a/drivers/clk/rockchip/Makefile +++ b/drivers/clk/rockchip/Makefile @@ -14,6 +14,7 @@ obj-y += clk-muxgrf.o obj-y += clk-ddr.o obj-$(CONFIG_RESET_CONTROLLER) += softrst.o +obj-y += clk-px30.o obj-y += clk-rv1108.o obj-y += clk-rk3036.o obj-y += clk-rk3128.o diff --git a/drivers/clk/rockchip/clk-px30.c b/drivers/clk/rockchip/clk-px30.c new file mode 100644 index ..07105fe1ff6c --- /dev/null +++ b/drivers/clk/rockchip/clk-px30.c @@ -0,0 +1,1080 @@ +/* + * Copyright (c) 2016 Rockchip Electronics Co. Ltd. + * Author: Elaine + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include "clk.h" + +#define PX30_GRF_SOC_STATUS0 0x480 + +enum px30_plls { + apll, dpll, cpll, npll, apll_b_h, apll_b_l, +}; + +enum px30_pmu_plls { + gpll, +}; + +static struct rockchip_pll_rate_table px30_pll_rates[] = { + /* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */ + RK3036_PLL_RATE(160800, 1, 67, 1, 1, 1, 0), + RK3036_PLL_RATE(158400, 1, 66, 1, 1, 1, 0), + RK3036_PLL_RATE(156000, 1, 65, 1, 1, 1, 0), + RK3036_PLL_RATE(153600, 1, 64, 1, 1, 1, 0), + RK3036_PLL_RATE(151200, 1, 63, 1, 1, 1, 0), + RK3036_PLL_RATE(148800, 1, 62, 1, 1, 1, 0), + RK3036_PLL_RATE(146400, 1, 61, 1, 1, 1, 0), + RK3036_PLL_RATE(144000, 1, 60, 1, 1, 1, 0), + RK3036_PLL_RATE(141600, 1, 59, 1, 1, 1, 0), + RK3036_PLL_RATE(139200, 1, 58, 1, 1, 1, 0), + RK3036_PLL_RATE(136800, 1, 57, 1, 1, 1, 0), + RK3036_PLL_RATE(134400, 1, 56, 1, 1, 1, 0), + RK3036_PLL_RATE(132000, 1, 55, 1, 1, 1, 0), + RK3036_PLL_RATE(129600, 1, 54, 1, 1, 1, 0), + RK3036_PLL_RATE(127200, 1, 53, 1, 1, 1, 0), + RK3036_PLL_RATE(124800, 1, 52, 1, 1, 1, 0), + RK3036_PLL_RATE(12, 1, 50, 1, 1, 1, 0), + RK3036_PLL_RATE(118800, 2, 99, 1, 1, 1, 0), + RK3036_PLL_RATE(110400, 1, 46, 1, 1, 1, 0), + RK3036_PLL_RATE(11, 12, 550, 1, 1, 1, 0), + RK3036_PLL_RATE(100800, 1, 84, 2, 1, 1, 0), + RK3036_PLL_RATE(10, 6, 500, 2, 1, 1, 0), + RK3036_PLL_RATE(98400, 1, 82, 2, 1, 1, 0), + RK3036_PLL_RATE(96000, 1, 80, 2, 1, 1, 0), + RK3036_PLL_RATE(93600, 1, 78, 2, 1, 1, 0), + RK3036_PLL_RATE(91200, 1, 76, 2, 1, 1, 0), + RK3036_PLL_RATE(9, 4, 300, 2, 1, 1, 0), + RK3036_PLL_RATE(88800, 1, 74, 2, 1, 1, 0), + RK3036_PLL_RATE(86400, 1, 72, 2, 1, 1, 0), + RK3036_PLL_RATE(84000, 1, 70, 2, 1, 1, 0), + RK3036_PLL_RATE(81600, 1, 68, 2, 1, 1, 0), + RK3036_PLL_RATE(8, 6, 400, 2, 1, 1, 0), + RK3036_PLL_RATE(7, 6, 350, 2, 1, 1, 0), + RK3036_PLL_RATE(69600, 1, 58, 2, 1, 1, 0), + RK3036_PLL_RATE(62400, 1, 52, 2, 1, 1, 0), + RK3036_PLL_RATE(6, 1, 75, 3, 1, 1, 0), + RK3036_PLL_RATE(59400, 2, 99, 2, 1, 1, 0), + RK3036_PLL_RATE(50400, 1, 63, 3, 1, 1, 0), + RK3036_PLL_RATE(5, 6, 250, 2, 1, 1, 0), + RK3036_PLL_RATE(40800, 1, 68, 2, 2, 1, 0), + RK3036_PLL_RATE(31200, 1, 52, 2, 2, 1, 0), + RK3036_PLL_RATE(21600, 1, 72, 4, 2, 1, 0), + RK3036_PLL_RATE(9600, 1, 64, 4, 4, 1, 0), + { /* sentinel */ }, +}; + +#define PX30_DIV_ACLKM_MASK0x7 +#define PX30_DIV_ACLKM_SHIFT 12 +#define PX30_DIV_PCLK_DBG_MASK 0xf +#define PX30_DIV_PCLK_DBG_SHIFT8 + +#define PX30_CLKSEL0(_aclk_core, _pclk_dbg)\ +{ \ + .reg = PX30_CLKSEL_CON(0), \ + .val = HIWORD_UPDATE(_aclk_core, PX30_DIV_ACLKM_MASK, \ +PX30_DIV_ACLKM_SHIFT) |\ + HIWORD_UPDATE(_pclk_dbg, PX30_DIV_PCLK_DBG_MASK, \ +
[PATCH v1 1/4] dt-bindings: add bindings for px30 clock controller
Add devicetree bindings for Rockchip cru which found on Rockchip SoCs. Signed-off-by: Elaine Zhang --- .../bindings/clock/rockchip,px30-cru.txt | 67 ++ 1 file changed, 67 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt diff --git a/Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt b/Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt new file mode 100644 index ..af5a45b680d0 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt @@ -0,0 +1,67 @@ +* Rockchip PX30 Clock and Reset Unit + +The PX30 clock controller generates and supplies clock to various +controllers within the SoC and also implements a reset controller for SoC +peripherals. + +Required Properties: + +- compatible: PMU for CRU should be "rockchip,px30-pmu-cru" +- compatible: CRU should be "rockchip,px30-cru" +- reg: physical base address of the controller and length of memory mapped + region. +- #clock-cells: should be 1. +- #reset-cells: should be 1. + +Optional Properties: + +- rockchip,grf: phandle to the syscon managing the "general register files" + If missing, pll rates are not changeable, due to the missing pll lock status. + +Each clock is assigned an identifier and client nodes can use this identifier +to specify the clock which they consume. All available clocks are defined as +preprocessor macros in the dt-bindings/clock/px30-cru.h headers and can be +used in device tree sources. Similar macros exist for the reset sources in +these files. + +External clocks: + +There are several clocks that are generated outside the SoC. It is expected +that they are defined using standard clock bindings with following +clock-output-names: + - "xin24m" - crystal input - required, + - "xin32k" - rtc clock - optional, + - "i2sx_clkin" - external I2S clock - optional, + - "gmac_clkin" - external GMAC clock - optional + +Example: Clock controller node: + + pmucru: pmu-clock-controller@ff2bc000 { + compatible = "rockchip,px30-pmucru"; + reg = <0x0 0xff2bc000 0x0 0x1000>; + #clock-cells = <1>; + #reset-cells = <1>; + }; + + cru: clock-controller@ff2b { + compatible = "rockchip,px30-cru"; + reg = <0x0 0xff2b 0x0 0x1000>; + rockchip,grf = <>; + #clock-cells = <1>; + #reset-cells = <1>; + }; + +Example: UART controller node that consumes the clock generated by the clock + controller: + + uart0: serial@ff03 { + compatible = "rockchip,px30-uart", "snps,dw-apb-uart"; + reg = <0x0 0xff03 0x0 0x100>; + interrupts = ; + clocks = < SCLK_UART0_PMU>, < PCLK_UART0_PMU>; + clock-names = "baudclk", "apb_pclk"; + reg-shift = <2>; + reg-io-width = <4>; + status = "disabled"; + }; + -- 1.9.1
[GIT PULL] DeviceTree updates for 4.18
Hi Linus, Please pull DT updates for 4.18. Details below. Rob The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338: Linux 4.17-rc1 (2018-04-15 18:24:20 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git tags/devicetree-for-4.18 for you to fetch changes up to 44acf10587907ff77c28fd97906220b2d8eb4f05: dt-bindings: submitting-patches: add guidance on patch content and subject (2018-06-05 16:37:47 -0600) DeviceTree updates for v4.18: - Sync dtc with upstream version v1.4.6-21-g84e414b0b5bc. This adds new warnings which are either fixed or disabled by default (enabled with W=1). - Validate an untrusted offset in DT overlay function update_usages_of_a_phandle_reference - Fix a use after free error of_platform_device_destroy - Fix an off by 1 string errors in unittest - Avoid creating a struct device for OPP nodes - Update DT specific submitting-patches.txt with patch content and subject requirements. - Move some bindings to their proper subsystem locations - Add vendor prefixes for Kaohsiung, SiFive, Avnet, Wi2Wi, Logic PD, and ArcherMind - Add documentation for "no-gpio-delays" property in FSI bus GPIO master - Add compatible for r8a77990 SoC ravb ethernet block - More wack-a-mole removal of 'status' property in examples Benjamin Herrenschmidt (1): dt-bindings: fsi-master-gpio: Document "no-gpio-delays" property Frank Rowand (2): MAINTAINERS: add keyword for devicetree overlay notifiers of: overlay: validate offset from property fixups Geert Uytterhoeven (3): dt-bindings: meson-uart: DT fix s/clocks-names/clock-names/ dt-bindings: mvebu-uart: DT fix s/interrupts-names/interrupt-names/ dt-bindings: panel: lvds: Fix path to display timing bindings H. Nikolaus Schaller (1): dt-bindings: define vendor prefix for Wi2Wi, Inc. Jacopo Mondi (3): dt-bindings: net: ravb: Add support for r8a77965 SoC dt-bindings: serial: sh-sci: Add support for r8a77965 (H)SCIF dt-bindings: dmaengine: rcar-dmac: document R8A77965 support Jan Kiszka (1): of: overlay: Stop leaking resources on overlay removal Lukasz Majewski (2): doc: Add vendor prefix for Kieback & Peter GmbH doc: Add vendor prefix for Kaohsiung Manivannan Sadhasivam (1): dt-bindings: Add vendor prefix for ArcherMind Matheus Castello (1): dt-bindings: pinctrl: sunxi: Fix reference to driver Michal Simek (1): dt-bindings: Add vendor prefix for Avnet, Inc. Niklas Söderlund (1): dt-bindings: thermal: rcar-gen3-thermal: update register size in example Rob Herring (12): of/numa: drop export of of_node_to_nid dt-bindings: more status property removal from examples dtc: checks: drop warning for missing PCI bridge bus-range dt-bindings: move various timer bindings to timer/ directory dt-bindings: move various RNG bindings to rng/ directory dt-bindings: powerpc/4xx: move 4xx NDFC and EMAC bindings to subsystem directories dt-bindings: exynos: move ADC binding to iio/adc/ directory Merge tag 'devicetree-fixes-for-4.17' into dt/next to pick-up fixes scripts/dtc: Update to upstream version v1.4.6-21-g84e414b0b5bc kbuild: disable new dtc graph and unit-address warnings drm: rcar-du: disable dtc graph-endpoint warnings on DT overlays dt-bindings: submitting-patches: add guidance on patch content and subject Srinivas Kandagatla (1): of: platform: stop accessing invalid dev in of_platform_device_destroy Stefan M Schaeckeler (1): of: unittest: for strings, account for trailing \0 in property length field Thierry Reding (1): dt-bindings: Relocate Tegra20 memory controller bindings Viresh Kumar (1): of: Don't create device for OPP tables Vladimir Zapolskiy (1): dt-bindings: Add vendor prefix for Logic PD Wesley W. Terpstra (1): dt-bindings: Add "sifive" vendor prefix Yoshihiro Shimoda (1): dt-bindings: net: ravb: Add support for r8a77990 SoC .../devicetree/bindings/arm/ux500/boards.txt | 2 +- .../bindings/display/panel/panel-common.txt| 2 +- Documentation/devicetree/bindings/dma/k3dma.txt| 1 - .../devicetree/bindings/dma/renesas,rcar-dmac.txt | 1 + Documentation/devicetree/bindings/dma/ti-edma.txt | 1 - .../devicetree/bindings/fsi/fsi-master-gpio.txt| 4 + .../adc/samsung,exynos-adc.txt}| 0 .../nvidia,tegra20-mc.txt | 0 .../devicetree/bindings/mips/lantiq/rcu.txt| 2 - Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 4 - .../{powerpc/4xx/ndfc.txt => mtd/ibm,ndfc.txt} | 0 Documentation/devicetree/bindings/mtd/mtk-nand.txt | 4 - .../{powerpc/4xx/emac.txt => net/ibm,emac.txt} | 0
[GIT PULL] DeviceTree updates for 4.18
Hi Linus, Please pull DT updates for 4.18. Details below. Rob The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338: Linux 4.17-rc1 (2018-04-15 18:24:20 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git tags/devicetree-for-4.18 for you to fetch changes up to 44acf10587907ff77c28fd97906220b2d8eb4f05: dt-bindings: submitting-patches: add guidance on patch content and subject (2018-06-05 16:37:47 -0600) DeviceTree updates for v4.18: - Sync dtc with upstream version v1.4.6-21-g84e414b0b5bc. This adds new warnings which are either fixed or disabled by default (enabled with W=1). - Validate an untrusted offset in DT overlay function update_usages_of_a_phandle_reference - Fix a use after free error of_platform_device_destroy - Fix an off by 1 string errors in unittest - Avoid creating a struct device for OPP nodes - Update DT specific submitting-patches.txt with patch content and subject requirements. - Move some bindings to their proper subsystem locations - Add vendor prefixes for Kaohsiung, SiFive, Avnet, Wi2Wi, Logic PD, and ArcherMind - Add documentation for "no-gpio-delays" property in FSI bus GPIO master - Add compatible for r8a77990 SoC ravb ethernet block - More wack-a-mole removal of 'status' property in examples Benjamin Herrenschmidt (1): dt-bindings: fsi-master-gpio: Document "no-gpio-delays" property Frank Rowand (2): MAINTAINERS: add keyword for devicetree overlay notifiers of: overlay: validate offset from property fixups Geert Uytterhoeven (3): dt-bindings: meson-uart: DT fix s/clocks-names/clock-names/ dt-bindings: mvebu-uart: DT fix s/interrupts-names/interrupt-names/ dt-bindings: panel: lvds: Fix path to display timing bindings H. Nikolaus Schaller (1): dt-bindings: define vendor prefix for Wi2Wi, Inc. Jacopo Mondi (3): dt-bindings: net: ravb: Add support for r8a77965 SoC dt-bindings: serial: sh-sci: Add support for r8a77965 (H)SCIF dt-bindings: dmaengine: rcar-dmac: document R8A77965 support Jan Kiszka (1): of: overlay: Stop leaking resources on overlay removal Lukasz Majewski (2): doc: Add vendor prefix for Kieback & Peter GmbH doc: Add vendor prefix for Kaohsiung Manivannan Sadhasivam (1): dt-bindings: Add vendor prefix for ArcherMind Matheus Castello (1): dt-bindings: pinctrl: sunxi: Fix reference to driver Michal Simek (1): dt-bindings: Add vendor prefix for Avnet, Inc. Niklas Söderlund (1): dt-bindings: thermal: rcar-gen3-thermal: update register size in example Rob Herring (12): of/numa: drop export of of_node_to_nid dt-bindings: more status property removal from examples dtc: checks: drop warning for missing PCI bridge bus-range dt-bindings: move various timer bindings to timer/ directory dt-bindings: move various RNG bindings to rng/ directory dt-bindings: powerpc/4xx: move 4xx NDFC and EMAC bindings to subsystem directories dt-bindings: exynos: move ADC binding to iio/adc/ directory Merge tag 'devicetree-fixes-for-4.17' into dt/next to pick-up fixes scripts/dtc: Update to upstream version v1.4.6-21-g84e414b0b5bc kbuild: disable new dtc graph and unit-address warnings drm: rcar-du: disable dtc graph-endpoint warnings on DT overlays dt-bindings: submitting-patches: add guidance on patch content and subject Srinivas Kandagatla (1): of: platform: stop accessing invalid dev in of_platform_device_destroy Stefan M Schaeckeler (1): of: unittest: for strings, account for trailing \0 in property length field Thierry Reding (1): dt-bindings: Relocate Tegra20 memory controller bindings Viresh Kumar (1): of: Don't create device for OPP tables Vladimir Zapolskiy (1): dt-bindings: Add vendor prefix for Logic PD Wesley W. Terpstra (1): dt-bindings: Add "sifive" vendor prefix Yoshihiro Shimoda (1): dt-bindings: net: ravb: Add support for r8a77990 SoC .../devicetree/bindings/arm/ux500/boards.txt | 2 +- .../bindings/display/panel/panel-common.txt| 2 +- Documentation/devicetree/bindings/dma/k3dma.txt| 1 - .../devicetree/bindings/dma/renesas,rcar-dmac.txt | 1 + Documentation/devicetree/bindings/dma/ti-edma.txt | 1 - .../devicetree/bindings/fsi/fsi-master-gpio.txt| 4 + .../adc/samsung,exynos-adc.txt}| 0 .../nvidia,tegra20-mc.txt | 0 .../devicetree/bindings/mips/lantiq/rcu.txt| 2 - Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 4 - .../{powerpc/4xx/ndfc.txt => mtd/ibm,ndfc.txt} | 0 Documentation/devicetree/bindings/mtd/mtk-nand.txt | 4 - .../{powerpc/4xx/emac.txt => net/ibm,emac.txt} | 0
Re: Is this a kernel BUG? ///Re: [Question] Can we use SIGRTMIN when vdso disabled on X86?
> On Jun 6, 2018, at 7:05 PM, Leizhen (ThunderTown) > wrote: > > > >> On 2018/6/7 1:01, Andy Lutomirski wrote: >> On Wed, Jun 6, 2018 at 2:18 AM Leizhen (ThunderTown) >> wrote: >>> >>> I found that glibc has already dealt with this case. So this issue must >>> have been met before, should it be maintained by libc/user? >>> >>>if (GLRO(dl_sysinfo_dso) == NULL) >>>{ >>>kact.sa_flags |= SA_RESTORER; >>> >>>kact.sa_restorer = ((act->sa_flags & SA_SIGINFO) >>>? _rt : ); >>>} >>> >>> On 2018/6/6 15:52, Leizhen (ThunderTown) wrote: > On 2018/6/5 19:24, Leizhen (ThunderTown) wrote: > After I executed "echo 0 > /proc/sys/abi/vsyscall32" to disable vdso, the > rt_sigaction01 test case from ltp_2015 failed. > The test case source code please refer to the attachment, and the output > as blow: > > - > ./rt_sigaction01 > rt_sigaction010 TINFO : signal: 34 > rt_sigaction011 TPASS : rt_sigaction call succeeded: result = 0 > rt_sigaction010 TINFO : sa.sa_flags = SA_RESETHAND|SA_SIGINFO > rt_sigaction010 TINFO : Signal Handler Called with signal number 34 > > Segmentation fault > -- > > > Is this the desired result? In function ia32_setup_rt_frame, I found > below code: > > if (ksig->ka.sa.sa_flags & SA_RESTORER) > restorer = ksig->ka.sa.sa_restorer; > else > restorer = current->mm->context.vdso + > vdso_image_32.sym___kernel_rt_sigreturn; > put_user_ex(ptr_to_compat(restorer), >pretcode); > > Because the vdso is disabled, so current->mm->context.vdso is NULL, which > cause the result of frame->pretcode invalid. > > I'm not sure whether this is a kernel bug or just an error of test case > itself. Can anyone help me? > >>> >>> >> >> I can't tell from your email what you're testing, what behavior you >> expect, and what you saw. A program that sets up a signal handler >> without supplying a restorer will not work if the vDSO is off, and >> this is by design. > OK, so that the user should take care whether the vDSO is disabled by itself > or not, and use different strategies to process it appropriately, like glibc. > >> >> (FWIW, there is a very longstanding libc bug that causes this case to >> get severely screwed up if the user's SS is not the expected value, >> and that bug was just fixed very recently. But I doubt this is what >> you're seeing.) >> >> I suppose we could improve the kernel to at least push NULL instead of >> some random address a bit above 0, but it'll still crash. > Should we add a warning? Which may help the user to aware this error in time. > It’s entirely valid to have a non working restorer if you never plan to return from a signal handler. And anyone who writes their own libc should be able to figure this out on their own, I think. >> >> . >> > > -- > Thanks! > BestRegards >
Re: Is this a kernel BUG? ///Re: [Question] Can we use SIGRTMIN when vdso disabled on X86?
> On Jun 6, 2018, at 7:05 PM, Leizhen (ThunderTown) > wrote: > > > >> On 2018/6/7 1:01, Andy Lutomirski wrote: >> On Wed, Jun 6, 2018 at 2:18 AM Leizhen (ThunderTown) >> wrote: >>> >>> I found that glibc has already dealt with this case. So this issue must >>> have been met before, should it be maintained by libc/user? >>> >>>if (GLRO(dl_sysinfo_dso) == NULL) >>>{ >>>kact.sa_flags |= SA_RESTORER; >>> >>>kact.sa_restorer = ((act->sa_flags & SA_SIGINFO) >>>? _rt : ); >>>} >>> >>> On 2018/6/6 15:52, Leizhen (ThunderTown) wrote: > On 2018/6/5 19:24, Leizhen (ThunderTown) wrote: > After I executed "echo 0 > /proc/sys/abi/vsyscall32" to disable vdso, the > rt_sigaction01 test case from ltp_2015 failed. > The test case source code please refer to the attachment, and the output > as blow: > > - > ./rt_sigaction01 > rt_sigaction010 TINFO : signal: 34 > rt_sigaction011 TPASS : rt_sigaction call succeeded: result = 0 > rt_sigaction010 TINFO : sa.sa_flags = SA_RESETHAND|SA_SIGINFO > rt_sigaction010 TINFO : Signal Handler Called with signal number 34 > > Segmentation fault > -- > > > Is this the desired result? In function ia32_setup_rt_frame, I found > below code: > > if (ksig->ka.sa.sa_flags & SA_RESTORER) > restorer = ksig->ka.sa.sa_restorer; > else > restorer = current->mm->context.vdso + > vdso_image_32.sym___kernel_rt_sigreturn; > put_user_ex(ptr_to_compat(restorer), >pretcode); > > Because the vdso is disabled, so current->mm->context.vdso is NULL, which > cause the result of frame->pretcode invalid. > > I'm not sure whether this is a kernel bug or just an error of test case > itself. Can anyone help me? > >>> >>> >> >> I can't tell from your email what you're testing, what behavior you >> expect, and what you saw. A program that sets up a signal handler >> without supplying a restorer will not work if the vDSO is off, and >> this is by design. > OK, so that the user should take care whether the vDSO is disabled by itself > or not, and use different strategies to process it appropriately, like glibc. > >> >> (FWIW, there is a very longstanding libc bug that causes this case to >> get severely screwed up if the user's SS is not the expected value, >> and that bug was just fixed very recently. But I doubt this is what >> you're seeing.) >> >> I suppose we could improve the kernel to at least push NULL instead of >> some random address a bit above 0, but it'll still crash. > Should we add a warning? Which may help the user to aware this error in time. > It’s entirely valid to have a non working restorer if you never plan to return from a signal handler. And anyone who writes their own libc should be able to figure this out on their own, I think. >> >> . >> > > -- > Thanks! > BestRegards >
Re: [PATCH v5 01/28] docs: fpga: add a document for FPGA Device Feature List (DFL) Framework Overview
On Wed, Jun 06, 2018 at 11:16:03AM -0500, Alan Tull wrote: > On Tue, May 1, 2018 at 9:50 PM, Wu Hao wrote: > > Hi Hao, > > I've acked the remaining patches with some changes requested. In your > v6, please add a patch to add yourself to the MAINTAINERS file or > whoever is planning on maintaining fpga/drivers/*dfl* Hi Alan, Sure, I will fix these in the next version. Thanks a lot for the review. Hao > > > Add a document for FPGA Device Feature List (DFL) Framework Overview. > > > > Signed-off-by: Enno Luebbers > > Signed-off-by: Xiao Guangrong > > Signed-off-by: Wu Hao > Acked-by: Alan Tull > > Thanks, > Alan
Re: [PATCH v5 01/28] docs: fpga: add a document for FPGA Device Feature List (DFL) Framework Overview
On Wed, Jun 06, 2018 at 11:16:03AM -0500, Alan Tull wrote: > On Tue, May 1, 2018 at 9:50 PM, Wu Hao wrote: > > Hi Hao, > > I've acked the remaining patches with some changes requested. In your > v6, please add a patch to add yourself to the MAINTAINERS file or > whoever is planning on maintaining fpga/drivers/*dfl* Hi Alan, Sure, I will fix these in the next version. Thanks a lot for the review. Hao > > > Add a document for FPGA Device Feature List (DFL) Framework Overview. > > > > Signed-off-by: Enno Luebbers > > Signed-off-by: Xiao Guangrong > > Signed-off-by: Wu Hao > Acked-by: Alan Tull > > Thanks, > Alan
Re: [PATCH 26/32] afs: Use fs_context to pass parameters over automount [ver #8]
On 05/24/2018 07:08 PM, David Howells wrote: > Alter the AFS automounting code to create and modify an fs_context struct > when parameterising a new mount triggered by an AFS mountpoint rather than > constructing device name and option strings. > > Also remove the cell=, vol= and rwpath options as they are then redundant. > The reason they existed is because the 'device name' may be derived > literally from a mountpoint object in the filesystem, so default cell and > parent-type information needed to be passed in by some other method from > the automount routines. The vol= option didn't end up being used. > > Signed-off-by: David Howells > cc: Eric W. Biederman > --- > > fs/afs/internal.h |1 > fs/afs/mntpt.c| 148 > +++-- > fs/afs/super.c| 43 +-- > 3 files changed, 79 insertions(+), 113 deletions(-) > > diff --git a/fs/afs/internal.h b/fs/afs/internal.h > index eb6e75e00181..90af5001f8c8 100644 > --- a/fs/afs/internal.h > +++ b/fs/afs/internal.h > @@ -35,7 +35,6 @@ struct pagevec; > struct afs_call; > > struct afs_fs_context { > - boolrwpath; /* T if the parent should be > considered R/W */ > boolforce; /* T to force cell type */ > boolautocell; /* T if set auto mount > operation */ > booldyn_root; /* T if dynamic root */ > diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c > index c45aa1776591..fc383d727552 100644 > --- a/fs/afs/mntpt.c > +++ b/fs/afs/mntpt.c > @@ -47,6 +47,8 @@ static DECLARE_DELAYED_WORK(afs_mntpt_expiry_timer, > afs_mntpt_expiry_timed_out); > > static unsigned long afs_mntpt_expiry_timeout = 10 * 60; > > +static const char afs_root_volume[] = "root.cell"; > + > /* > * no valid lookup procedure on this sort of dir > */ > @@ -68,107 +70,107 @@ static int afs_mntpt_open(struct inode *inode, struct > file *file) > } > > /* > - * create a vfsmount to be automounted > + * Set the parameters for the proposed superblock. > */ > -static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt) > +static int afs_mntpt_set_params(struct fs_context *fc, struct dentry *mntpt) > { > - struct afs_super_info *as; > - struct vfsmount *mnt; > - struct afs_vnode *vnode; > - struct page *page; > - char *devname, *options; > - bool rwpath = false; > + struct afs_fs_context *ctx = fc->fs_private; > + struct afs_vnode *vnode = AFS_FS_I(d_inode(mntpt)); > + struct afs_cell *cell; > + const char *p; > int ret; > > - _enter("{%pd}", mntpt); > - > - BUG_ON(!d_inode(mntpt)); > - > - ret = -ENOMEM; > - devname = (char *) get_zeroed_page(GFP_KERNEL); > - if (!devname) > - goto error_no_devname; > - > - options = (char *) get_zeroed_page(GFP_KERNEL); > - if (!options) > - goto error_no_options; > - > - vnode = AFS_FS_I(d_inode(mntpt)); > if (test_bit(AFS_VNODE_PSEUDODIR, >flags)) { > /* if the directory is a pseudo directory, use the d_name */ > - static const char afs_root_cell[] = ":root.cell."; > unsigned size = mntpt->d_name.len; > > - ret = -ENOENT; > - if (size < 2 || size > AFS_MAXCELLNAME) > - goto error_no_page; > + if (size < 2) > + return -ENOENT; > > + p = mntpt->d_name.name; > if (mntpt->d_name.name[0] == '.') { > - devname[0] = '%'; > - memcpy(devname + 1, mntpt->d_name.name + 1, size - 1); > - memcpy(devname + size, afs_root_cell, > -sizeof(afs_root_cell)); > - rwpath = true; > - } else { > - devname[0] = '#'; > - memcpy(devname + 1, mntpt->d_name.name, size); > - memcpy(devname + size + 1, afs_root_cell, > -sizeof(afs_root_cell)); > + size--; > + p++; > + ctx->type = AFSVL_RWVOL; > + ctx->force = true; > + } > + if (size > AFS_MAXCELLNAME) > + return -ENAMETOOLONG; > + > + cell = afs_lookup_cell(ctx->net, p, size, NULL, false); > + if (IS_ERR(cell)) { > + pr_err("kAFS: unable to lookup cell '%pd'\n", mntpt); > + return PTR_ERR(cell); > } > + afs_put_cell(ctx->net, ctx->cell); > + ctx->cell = cell; > + > + ctx->volname = afs_root_volume; > + ctx->volnamesz = sizeof(afs_root_volume) - 1; > } else { > /* read the contents of the AFS special symlink */ > + struct page *page; > loff_t size = i_size_read(d_inode(mntpt));
Re: [PATCH 26/32] afs: Use fs_context to pass parameters over automount [ver #8]
On 05/24/2018 07:08 PM, David Howells wrote: > Alter the AFS automounting code to create and modify an fs_context struct > when parameterising a new mount triggered by an AFS mountpoint rather than > constructing device name and option strings. > > Also remove the cell=, vol= and rwpath options as they are then redundant. > The reason they existed is because the 'device name' may be derived > literally from a mountpoint object in the filesystem, so default cell and > parent-type information needed to be passed in by some other method from > the automount routines. The vol= option didn't end up being used. > > Signed-off-by: David Howells > cc: Eric W. Biederman > --- > > fs/afs/internal.h |1 > fs/afs/mntpt.c| 148 > +++-- > fs/afs/super.c| 43 +-- > 3 files changed, 79 insertions(+), 113 deletions(-) > > diff --git a/fs/afs/internal.h b/fs/afs/internal.h > index eb6e75e00181..90af5001f8c8 100644 > --- a/fs/afs/internal.h > +++ b/fs/afs/internal.h > @@ -35,7 +35,6 @@ struct pagevec; > struct afs_call; > > struct afs_fs_context { > - boolrwpath; /* T if the parent should be > considered R/W */ > boolforce; /* T to force cell type */ > boolautocell; /* T if set auto mount > operation */ > booldyn_root; /* T if dynamic root */ > diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c > index c45aa1776591..fc383d727552 100644 > --- a/fs/afs/mntpt.c > +++ b/fs/afs/mntpt.c > @@ -47,6 +47,8 @@ static DECLARE_DELAYED_WORK(afs_mntpt_expiry_timer, > afs_mntpt_expiry_timed_out); > > static unsigned long afs_mntpt_expiry_timeout = 10 * 60; > > +static const char afs_root_volume[] = "root.cell"; > + > /* > * no valid lookup procedure on this sort of dir > */ > @@ -68,107 +70,107 @@ static int afs_mntpt_open(struct inode *inode, struct > file *file) > } > > /* > - * create a vfsmount to be automounted > + * Set the parameters for the proposed superblock. > */ > -static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt) > +static int afs_mntpt_set_params(struct fs_context *fc, struct dentry *mntpt) > { > - struct afs_super_info *as; > - struct vfsmount *mnt; > - struct afs_vnode *vnode; > - struct page *page; > - char *devname, *options; > - bool rwpath = false; > + struct afs_fs_context *ctx = fc->fs_private; > + struct afs_vnode *vnode = AFS_FS_I(d_inode(mntpt)); > + struct afs_cell *cell; > + const char *p; > int ret; > > - _enter("{%pd}", mntpt); > - > - BUG_ON(!d_inode(mntpt)); > - > - ret = -ENOMEM; > - devname = (char *) get_zeroed_page(GFP_KERNEL); > - if (!devname) > - goto error_no_devname; > - > - options = (char *) get_zeroed_page(GFP_KERNEL); > - if (!options) > - goto error_no_options; > - > - vnode = AFS_FS_I(d_inode(mntpt)); > if (test_bit(AFS_VNODE_PSEUDODIR, >flags)) { > /* if the directory is a pseudo directory, use the d_name */ > - static const char afs_root_cell[] = ":root.cell."; > unsigned size = mntpt->d_name.len; > > - ret = -ENOENT; > - if (size < 2 || size > AFS_MAXCELLNAME) > - goto error_no_page; > + if (size < 2) > + return -ENOENT; > > + p = mntpt->d_name.name; > if (mntpt->d_name.name[0] == '.') { > - devname[0] = '%'; > - memcpy(devname + 1, mntpt->d_name.name + 1, size - 1); > - memcpy(devname + size, afs_root_cell, > -sizeof(afs_root_cell)); > - rwpath = true; > - } else { > - devname[0] = '#'; > - memcpy(devname + 1, mntpt->d_name.name, size); > - memcpy(devname + size + 1, afs_root_cell, > -sizeof(afs_root_cell)); > + size--; > + p++; > + ctx->type = AFSVL_RWVOL; > + ctx->force = true; > + } > + if (size > AFS_MAXCELLNAME) > + return -ENAMETOOLONG; > + > + cell = afs_lookup_cell(ctx->net, p, size, NULL, false); > + if (IS_ERR(cell)) { > + pr_err("kAFS: unable to lookup cell '%pd'\n", mntpt); > + return PTR_ERR(cell); > } > + afs_put_cell(ctx->net, ctx->cell); > + ctx->cell = cell; > + > + ctx->volname = afs_root_volume; > + ctx->volnamesz = sizeof(afs_root_volume) - 1; > } else { > /* read the contents of the AFS special symlink */ > + struct page *page; > loff_t size = i_size_read(d_inode(mntpt));
Re: Is this a kernel BUG? ///Re: [Question] Can we use SIGRTMIN when vdso disabled on X86?
On 2018/6/7 1:01, Andy Lutomirski wrote: > On Wed, Jun 6, 2018 at 2:18 AM Leizhen (ThunderTown) > wrote: >> >> I found that glibc has already dealt with this case. So this issue must have >> been met before, should it be maintained by libc/user? >> >> if (GLRO(dl_sysinfo_dso) == NULL) >> { >> kact.sa_flags |= SA_RESTORER; >> >> kact.sa_restorer = ((act->sa_flags & SA_SIGINFO) >> ? _rt : ); >> } >> >> >> On 2018/6/6 15:52, Leizhen (ThunderTown) wrote: >>> >>> >>> On 2018/6/5 19:24, Leizhen (ThunderTown) wrote: After I executed "echo 0 > /proc/sys/abi/vsyscall32" to disable vdso, the rt_sigaction01 test case from ltp_2015 failed. The test case source code please refer to the attachment, and the output as blow: - ./rt_sigaction01 rt_sigaction010 TINFO : signal: 34 rt_sigaction011 TPASS : rt_sigaction call succeeded: result = 0 rt_sigaction010 TINFO : sa.sa_flags = SA_RESETHAND|SA_SIGINFO rt_sigaction010 TINFO : Signal Handler Called with signal number 34 Segmentation fault -- Is this the desired result? In function ia32_setup_rt_frame, I found below code: if (ksig->ka.sa.sa_flags & SA_RESTORER) restorer = ksig->ka.sa.sa_restorer; else restorer = current->mm->context.vdso + vdso_image_32.sym___kernel_rt_sigreturn; put_user_ex(ptr_to_compat(restorer), >pretcode); Because the vdso is disabled, so current->mm->context.vdso is NULL, which cause the result of frame->pretcode invalid. I'm not sure whether this is a kernel bug or just an error of test case itself. Can anyone help me? >>> >> >> > > I can't tell from your email what you're testing, what behavior you > expect, and what you saw. A program that sets up a signal handler > without supplying a restorer will not work if the vDSO is off, and > this is by design. OK, so that the user should take care whether the vDSO is disabled by itself or not, and use different strategies to process it appropriately, like glibc. > > (FWIW, there is a very longstanding libc bug that causes this case to > get severely screwed up if the user's SS is not the expected value, > and that bug was just fixed very recently. But I doubt this is what > you're seeing.) > > I suppose we could improve the kernel to at least push NULL instead of > some random address a bit above 0, but it'll still crash. Should we add a warning? Which may help the user to aware this error in time. > > . > -- Thanks! BestRegards
Re: Is this a kernel BUG? ///Re: [Question] Can we use SIGRTMIN when vdso disabled on X86?
On 2018/6/7 1:01, Andy Lutomirski wrote: > On Wed, Jun 6, 2018 at 2:18 AM Leizhen (ThunderTown) > wrote: >> >> I found that glibc has already dealt with this case. So this issue must have >> been met before, should it be maintained by libc/user? >> >> if (GLRO(dl_sysinfo_dso) == NULL) >> { >> kact.sa_flags |= SA_RESTORER; >> >> kact.sa_restorer = ((act->sa_flags & SA_SIGINFO) >> ? _rt : ); >> } >> >> >> On 2018/6/6 15:52, Leizhen (ThunderTown) wrote: >>> >>> >>> On 2018/6/5 19:24, Leizhen (ThunderTown) wrote: After I executed "echo 0 > /proc/sys/abi/vsyscall32" to disable vdso, the rt_sigaction01 test case from ltp_2015 failed. The test case source code please refer to the attachment, and the output as blow: - ./rt_sigaction01 rt_sigaction010 TINFO : signal: 34 rt_sigaction011 TPASS : rt_sigaction call succeeded: result = 0 rt_sigaction010 TINFO : sa.sa_flags = SA_RESETHAND|SA_SIGINFO rt_sigaction010 TINFO : Signal Handler Called with signal number 34 Segmentation fault -- Is this the desired result? In function ia32_setup_rt_frame, I found below code: if (ksig->ka.sa.sa_flags & SA_RESTORER) restorer = ksig->ka.sa.sa_restorer; else restorer = current->mm->context.vdso + vdso_image_32.sym___kernel_rt_sigreturn; put_user_ex(ptr_to_compat(restorer), >pretcode); Because the vdso is disabled, so current->mm->context.vdso is NULL, which cause the result of frame->pretcode invalid. I'm not sure whether this is a kernel bug or just an error of test case itself. Can anyone help me? >>> >> >> > > I can't tell from your email what you're testing, what behavior you > expect, and what you saw. A program that sets up a signal handler > without supplying a restorer will not work if the vDSO is off, and > this is by design. OK, so that the user should take care whether the vDSO is disabled by itself or not, and use different strategies to process it appropriately, like glibc. > > (FWIW, there is a very longstanding libc bug that causes this case to > get severely screwed up if the user's SS is not the expected value, > and that bug was just fixed very recently. But I doubt this is what > you're seeing.) > > I suppose we could improve the kernel to at least push NULL instead of > some random address a bit above 0, but it'll still crash. Should we add a warning? Which may help the user to aware this error in time. > > . > -- Thanks! BestRegards
Re: [RFC 0/1] mconf: Emacs-like isearch
2018-06-07 8:54 GMT+09:00 Dirk Gouders : > Randy Dunlap writes: > >> On 06/06/2018 03:32 PM, Dirk Gouders wrote: >>> Randy Dunlap writes: >>> On 06/06/2018 02:56 PM, Dirk Gouders wrote: > Hello, > > being an Emacs user, I frequently find myself pressing CTRL-s in mconf > to search for some menu entry, especially in large menus. I use Emacs, but I have never typed Ctrl-s in menuconfig. Is it important to use the same key pattern as in Emacs? You intercepted Ctrl-* Currently, Ctrl-C terminates the program, but this patch makes it no-op. > I decided to implement a basic isearch in mconf and would like to hear > if others find this functionality useful, as well. > > The new functionality is started with pressing CTRL-s followed by > characters that form the search string. To search for further > occurences of an entered string, press CTRL-s instead of further > characters. > > For example: to navigate to the USB device drivers, press CTRL-s de ENTER > ENTER usb ENTER ENTER Not being an emacs user, what is the "de" for above? >>> >>> "de" (with my .config) causes a match for "Device Drivers" -- >>> no other menu entry matching the string "de" is befor that entry. >>> >> >> Device Drivers is the first match for me also. >> >> To get to the USB drivers, I have to enter: >> CTRL-s de ENTER ENTER CTRL-s usb ENTER ENTER > > Yes, I left out the second CTRL-s, thank you! > > Oh well, I shouldn't have sent this late at night, then I would probably > have explained the needed input as: > > 1) CTRL-s // start isearch > 2) de // substring that matches "Device Drivers" > 3) ENTER // quit isearch > 4) ENTER // enter Device Drivers menu > 5) CTRL-s // start isearch > 6) usb // navigate to USB support > 7) ENTER // quit isearch > 8) ENTER // enter USB support menu Hmm. I tried this, but I was a bit annoyed. I wonder if this could be more user-friendly. For example, I want KEY_UP/DOWN/LEFT/RIGHT to quit the search mode without pressing ENTER. > Again, I'm really sorry for the confusion. > > Dirk > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html You in -- Best Regards Masahiro Yamada
Re: [RFC 0/1] mconf: Emacs-like isearch
2018-06-07 8:54 GMT+09:00 Dirk Gouders : > Randy Dunlap writes: > >> On 06/06/2018 03:32 PM, Dirk Gouders wrote: >>> Randy Dunlap writes: >>> On 06/06/2018 02:56 PM, Dirk Gouders wrote: > Hello, > > being an Emacs user, I frequently find myself pressing CTRL-s in mconf > to search for some menu entry, especially in large menus. I use Emacs, but I have never typed Ctrl-s in menuconfig. Is it important to use the same key pattern as in Emacs? You intercepted Ctrl-* Currently, Ctrl-C terminates the program, but this patch makes it no-op. > I decided to implement a basic isearch in mconf and would like to hear > if others find this functionality useful, as well. > > The new functionality is started with pressing CTRL-s followed by > characters that form the search string. To search for further > occurences of an entered string, press CTRL-s instead of further > characters. > > For example: to navigate to the USB device drivers, press CTRL-s de ENTER > ENTER usb ENTER ENTER Not being an emacs user, what is the "de" for above? >>> >>> "de" (with my .config) causes a match for "Device Drivers" -- >>> no other menu entry matching the string "de" is befor that entry. >>> >> >> Device Drivers is the first match for me also. >> >> To get to the USB drivers, I have to enter: >> CTRL-s de ENTER ENTER CTRL-s usb ENTER ENTER > > Yes, I left out the second CTRL-s, thank you! > > Oh well, I shouldn't have sent this late at night, then I would probably > have explained the needed input as: > > 1) CTRL-s // start isearch > 2) de // substring that matches "Device Drivers" > 3) ENTER // quit isearch > 4) ENTER // enter Device Drivers menu > 5) CTRL-s // start isearch > 6) usb // navigate to USB support > 7) ENTER // quit isearch > 8) ENTER // enter USB support menu Hmm. I tried this, but I was a bit annoyed. I wonder if this could be more user-friendly. For example, I want KEY_UP/DOWN/LEFT/RIGHT to quit the search mode without pressing ENTER. > Again, I'm really sorry for the confusion. > > Dirk > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html You in -- Best Regards Masahiro Yamada
linux-next: manual merge of the tip tree with Linus' tree
Hi all, Today's linux-next merge of the tip tree got a conflict in: tools/testing/selftests/Makefile between commit: a12ab9e125f1 ("selftests: move RTC tests to rtc subfolder") from Linus' tree and commit: ccba8b64452b ("rseq/selftests: Provide Makefile, scripts, gitignore") from the tip tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc tools/testing/selftests/Makefile index 4cd339b5366a,593fb44c9cd4.. --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@@ -29,7 -28,7 +29,8 @@@ TARGETS += powerp TARGETS += proc TARGETS += pstore TARGETS += ptrace + TARGETS += rseq +TARGETS += rtc TARGETS += seccomp TARGETS += sigaltstack TARGETS += size pgp1EdnUyas_k.pgp Description: OpenPGP digital signature
linux-next: manual merge of the tip tree with Linus' tree
Hi all, Today's linux-next merge of the tip tree got a conflict in: tools/testing/selftests/Makefile between commit: a12ab9e125f1 ("selftests: move RTC tests to rtc subfolder") from Linus' tree and commit: ccba8b64452b ("rseq/selftests: Provide Makefile, scripts, gitignore") from the tip tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc tools/testing/selftests/Makefile index 4cd339b5366a,593fb44c9cd4.. --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@@ -29,7 -28,7 +29,8 @@@ TARGETS += powerp TARGETS += proc TARGETS += pstore TARGETS += ptrace + TARGETS += rseq +TARGETS += rtc TARGETS += seccomp TARGETS += sigaltstack TARGETS += size pgp1EdnUyas_k.pgp Description: OpenPGP digital signature
Re: Is this a kernel BUG? ///Re: [Question] Can we use SIGRTMIN when vdso disabled on X86?
On 2018/6/7 1:48, h...@zytor.com wrote: > On June 6, 2018 2:17:42 AM PDT, "Leizhen (ThunderTown)" > wrote: >> I found that glibc has already dealt with this case. So this issue must >> have been met before, should it be maintained by libc/user? >> >> if (GLRO(dl_sysinfo_dso) == NULL) >> { >> kact.sa_flags |= SA_RESTORER; >> >> kact.sa_restorer = ((act->sa_flags & SA_SIGINFO) >> ? _rt : ); >> } >> >> >> On 2018/6/6 15:52, Leizhen (ThunderTown) wrote: >>> >>> >>> On 2018/6/5 19:24, Leizhen (ThunderTown) wrote: After I executed "echo 0 > /proc/sys/abi/vsyscall32" to disable >> vdso, the rt_sigaction01 test case from ltp_2015 failed. The test case source code please refer to the attachment, and the >> output as blow: - ./rt_sigaction01 rt_sigaction010 TINFO : signal: 34 rt_sigaction011 TPASS : rt_sigaction call succeeded: result = >> 0 rt_sigaction010 TINFO : sa.sa_flags = SA_RESETHAND|SA_SIGINFO rt_sigaction010 TINFO : Signal Handler Called with signal >> number 34 Segmentation fault -- Is this the desired result? In function ia32_setup_rt_frame, I found >> below code: if (ksig->ka.sa.sa_flags & SA_RESTORER) restorer = ksig->ka.sa.sa_restorer; else restorer = current->mm->context.vdso + vdso_image_32.sym___kernel_rt_sigreturn; put_user_ex(ptr_to_compat(restorer), >pretcode); Because the vdso is disabled, so current->mm->context.vdso is NULL, >> which cause the result of frame->pretcode invalid. I'm not sure whether this is a kernel bug or just an error of test >> case itself. Can anyone help me? >>> > > The use of signals without SA_RESTORER is considered obsolete, but it's > somewhat surprising that the vdso isn't there; it should be mapped even for > static binaries esp. on i386 since it is the preferred way to do system calls > (you don't need to parse the ELF for that.) Are you explicitly disabling the > VDSO? If so, Don't Do That. Yes, the vdso was explicitly disabled by the tester. Thanks. > -- Thanks! BestRegards
Re: Is this a kernel BUG? ///Re: [Question] Can we use SIGRTMIN when vdso disabled on X86?
On 2018/6/7 1:48, h...@zytor.com wrote: > On June 6, 2018 2:17:42 AM PDT, "Leizhen (ThunderTown)" > wrote: >> I found that glibc has already dealt with this case. So this issue must >> have been met before, should it be maintained by libc/user? >> >> if (GLRO(dl_sysinfo_dso) == NULL) >> { >> kact.sa_flags |= SA_RESTORER; >> >> kact.sa_restorer = ((act->sa_flags & SA_SIGINFO) >> ? _rt : ); >> } >> >> >> On 2018/6/6 15:52, Leizhen (ThunderTown) wrote: >>> >>> >>> On 2018/6/5 19:24, Leizhen (ThunderTown) wrote: After I executed "echo 0 > /proc/sys/abi/vsyscall32" to disable >> vdso, the rt_sigaction01 test case from ltp_2015 failed. The test case source code please refer to the attachment, and the >> output as blow: - ./rt_sigaction01 rt_sigaction010 TINFO : signal: 34 rt_sigaction011 TPASS : rt_sigaction call succeeded: result = >> 0 rt_sigaction010 TINFO : sa.sa_flags = SA_RESETHAND|SA_SIGINFO rt_sigaction010 TINFO : Signal Handler Called with signal >> number 34 Segmentation fault -- Is this the desired result? In function ia32_setup_rt_frame, I found >> below code: if (ksig->ka.sa.sa_flags & SA_RESTORER) restorer = ksig->ka.sa.sa_restorer; else restorer = current->mm->context.vdso + vdso_image_32.sym___kernel_rt_sigreturn; put_user_ex(ptr_to_compat(restorer), >pretcode); Because the vdso is disabled, so current->mm->context.vdso is NULL, >> which cause the result of frame->pretcode invalid. I'm not sure whether this is a kernel bug or just an error of test >> case itself. Can anyone help me? >>> > > The use of signals without SA_RESTORER is considered obsolete, but it's > somewhat surprising that the vdso isn't there; it should be mapped even for > static binaries esp. on i386 since it is the preferred way to do system calls > (you don't need to parse the ELF for that.) Are you explicitly disabling the > VDSO? If so, Don't Do That. Yes, the vdso was explicitly disabled by the tester. Thanks. > -- Thanks! BestRegards
Re: [PATCH v3 2/5] gpio: syscon: rockchip: add GPIO_MUTE support for rk3328
Rob Herring writes: > On Sat, Jun 02, 2018 at 04:40:09PM +0800, Levin Du wrote: >> >> Rob Herring writes: >> >> > On Thu, May 31, 2018 at 9:05 PM, Levin wrote: >> > > Hi Rob, >> > > >> > > >> > > On 2018-05-31 10:45 PM, Rob Herring wrote: >> > > > >> > > > On Wed, May 30, 2018 at 10:27 PM, wrote: >> > > > > >> > > > > From: Levin Du >> > > > > >> > > > > In Rockchip RK3328, the output only GPIO_MUTE pin, >> > > > > originally for codec >> > > > > mute control, can also be used for general purpose. It is >> > > > > manipulated by >> > > > > the GRF_SOC_CON10 register. >> > > > > >> > > > > Signed-off-by: Levin Du >> > > > > >> > > > > --- >> > > > > >> > > > > Changes in v3: >> > > > > - Change from general gpio-syscon to specific >> > > > > rk3328-gpio-mute >> > > > > >> > > > > Changes in v2: >> > > > > - Rename gpio_syscon10 to gpio_mute in doc >> > > > > >> > > > > Changes in v1: >> > > > > - Refactured for general gpio-syscon usage for Rockchip SoCs. >> > > > > - Add doc rockchip,gpio-syscon.txt >> > > > > >> > > > > .../bindings/gpio/rockchip,rk3328-gpio-mute.txt| 28 >> > > > > +++ >> > > > > drivers/gpio/gpio-syscon.c | 31 >> > > > > ++ >> > > > > 2 files changed, 59 insertions(+) >> > > > > create mode 100644 >> > > > > Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt >> > > > > >> > > > > diff --git >> > > > > a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt >> > > > > b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt >> > > > > new file mode 100644 >> > > > > index 000..10bc632 >> > > > > --- /dev/null >> > > > > +++ >> > > > > b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt >> > > > > @@ -0,0 +1,28 @@ >> > > > > +Rockchip RK3328 GPIO controller dedicated for the GPIO_MUTE >> > > > > pin. >> > > > > + >> > > > > +In Rockchip RK3328, the output only GPIO_MUTE pin, >> > > > > originally for codec >> > > > > mute >> > > > > +control, can also be used for general purpose. It is >> > > > > manipulated by the >> > > > > +GRF_SOC_CON10 register. >> > > > > + >> > > > > +Required properties: >> > > > > +- compatible: Should contain "rockchip,rk3328-gpio-mute". >> > > > > +- gpio-controller: Marks the device node as a gpio >> > > > > controller. >> > > > > +- #gpio-cells: Should be 2. The first cell is the pin >> > > > > number and >> > > > > + the second cell is used to specify the gpio polarity: >> > > > > +0 = Active high, >> > > > > +1 = Active low. >> > > > > + >> > > > > +Example: >> > > > > + >> > > > > + grf: syscon@ff10 { >> > > > > + compatible = "rockchip,rk3328-grf", "syscon", >> > > > > "simple-mfd"; >> > > > > + >> > > > > + gpio_mute: gpio-mute { >> > > > >> > > > Node names should be generic: >> > > > >> > > > gpio { >> > > > >> > > > This also means you can't add another GPIO node in the future >> > > > and >> > > > you'll have to live with "rockchip,rk3328-gpio-mute" covering >> > > > more >> > > > than 1 GPIO if you do need to add more GPIOs. >> > > >> > > >> > > As the first line describes, this GPIO controller is dedicated for >> > > the >> > > GPIO_MUTE pin. >> > > There's only one GPIO pin in the GRF_SOC_CON10 register. Therefore >> > > the >> > > gpio_mute >> > > name is proper IMHO. >> > >> > It's how many GPIOs in the GRF, not this register. What I'm saying is >> > when you come along later to add another GPIO in the GRF, you had >> > better just add it to this same node. I'm not going to accept another >> > GPIO controller node within the GRF. You have the cells to support >> > more than 1, so it would only be a driver change. The compatible >> > string would then not be ideally named at that point. But compatible >> > strings are just unique identifiers, so it doesn't really matter what >> > the string is. >> > >> >> I'll try my best to introduce the situation here. The GRF, GPIO0~GPIO3 >> are register blocks in the RK3328 Soc. The GPIO0~GPIO3 contain registers >> for GPIO operations like reading/writing data, setting direction, >> interruption etc, which corresponds to the GPIO banks (gpio0~gpio3) >> defined in rk3328.dtsi: > > I'm only talking about GRF functions, not "regular" GPIOs. > >> pinctrl: pinctrl { >> compatible = "rockchip,rk3328-pinctrl"; >> rockchip,grf = <>; >> #address-cells = <2>; >> #size-cells = <2>; >> ranges; >> >> gpio0: gpio0@ff21 { >> compatible = "rockchip,gpio-bank"; >> reg = <0x0 0xff21 0x0 0x100>; >> interrupts = > IRQ_TYPE_LEVEL_HIGH>; >> clocks = < PCLK_GPIO0>; >> >> gpio-controller; >> #gpio-cells = <2>; >> >> interrupt-controller; >> #interrupt-cells =
Re: [PATCH v3 2/5] gpio: syscon: rockchip: add GPIO_MUTE support for rk3328
Rob Herring writes: > On Sat, Jun 02, 2018 at 04:40:09PM +0800, Levin Du wrote: >> >> Rob Herring writes: >> >> > On Thu, May 31, 2018 at 9:05 PM, Levin wrote: >> > > Hi Rob, >> > > >> > > >> > > On 2018-05-31 10:45 PM, Rob Herring wrote: >> > > > >> > > > On Wed, May 30, 2018 at 10:27 PM, wrote: >> > > > > >> > > > > From: Levin Du >> > > > > >> > > > > In Rockchip RK3328, the output only GPIO_MUTE pin, >> > > > > originally for codec >> > > > > mute control, can also be used for general purpose. It is >> > > > > manipulated by >> > > > > the GRF_SOC_CON10 register. >> > > > > >> > > > > Signed-off-by: Levin Du >> > > > > >> > > > > --- >> > > > > >> > > > > Changes in v3: >> > > > > - Change from general gpio-syscon to specific >> > > > > rk3328-gpio-mute >> > > > > >> > > > > Changes in v2: >> > > > > - Rename gpio_syscon10 to gpio_mute in doc >> > > > > >> > > > > Changes in v1: >> > > > > - Refactured for general gpio-syscon usage for Rockchip SoCs. >> > > > > - Add doc rockchip,gpio-syscon.txt >> > > > > >> > > > > .../bindings/gpio/rockchip,rk3328-gpio-mute.txt| 28 >> > > > > +++ >> > > > > drivers/gpio/gpio-syscon.c | 31 >> > > > > ++ >> > > > > 2 files changed, 59 insertions(+) >> > > > > create mode 100644 >> > > > > Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt >> > > > > >> > > > > diff --git >> > > > > a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt >> > > > > b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt >> > > > > new file mode 100644 >> > > > > index 000..10bc632 >> > > > > --- /dev/null >> > > > > +++ >> > > > > b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt >> > > > > @@ -0,0 +1,28 @@ >> > > > > +Rockchip RK3328 GPIO controller dedicated for the GPIO_MUTE >> > > > > pin. >> > > > > + >> > > > > +In Rockchip RK3328, the output only GPIO_MUTE pin, >> > > > > originally for codec >> > > > > mute >> > > > > +control, can also be used for general purpose. It is >> > > > > manipulated by the >> > > > > +GRF_SOC_CON10 register. >> > > > > + >> > > > > +Required properties: >> > > > > +- compatible: Should contain "rockchip,rk3328-gpio-mute". >> > > > > +- gpio-controller: Marks the device node as a gpio >> > > > > controller. >> > > > > +- #gpio-cells: Should be 2. The first cell is the pin >> > > > > number and >> > > > > + the second cell is used to specify the gpio polarity: >> > > > > +0 = Active high, >> > > > > +1 = Active low. >> > > > > + >> > > > > +Example: >> > > > > + >> > > > > + grf: syscon@ff10 { >> > > > > + compatible = "rockchip,rk3328-grf", "syscon", >> > > > > "simple-mfd"; >> > > > > + >> > > > > + gpio_mute: gpio-mute { >> > > > >> > > > Node names should be generic: >> > > > >> > > > gpio { >> > > > >> > > > This also means you can't add another GPIO node in the future >> > > > and >> > > > you'll have to live with "rockchip,rk3328-gpio-mute" covering >> > > > more >> > > > than 1 GPIO if you do need to add more GPIOs. >> > > >> > > >> > > As the first line describes, this GPIO controller is dedicated for >> > > the >> > > GPIO_MUTE pin. >> > > There's only one GPIO pin in the GRF_SOC_CON10 register. Therefore >> > > the >> > > gpio_mute >> > > name is proper IMHO. >> > >> > It's how many GPIOs in the GRF, not this register. What I'm saying is >> > when you come along later to add another GPIO in the GRF, you had >> > better just add it to this same node. I'm not going to accept another >> > GPIO controller node within the GRF. You have the cells to support >> > more than 1, so it would only be a driver change. The compatible >> > string would then not be ideally named at that point. But compatible >> > strings are just unique identifiers, so it doesn't really matter what >> > the string is. >> > >> >> I'll try my best to introduce the situation here. The GRF, GPIO0~GPIO3 >> are register blocks in the RK3328 Soc. The GPIO0~GPIO3 contain registers >> for GPIO operations like reading/writing data, setting direction, >> interruption etc, which corresponds to the GPIO banks (gpio0~gpio3) >> defined in rk3328.dtsi: > > I'm only talking about GRF functions, not "regular" GPIOs. > >> pinctrl: pinctrl { >> compatible = "rockchip,rk3328-pinctrl"; >> rockchip,grf = <>; >> #address-cells = <2>; >> #size-cells = <2>; >> ranges; >> >> gpio0: gpio0@ff21 { >> compatible = "rockchip,gpio-bank"; >> reg = <0x0 0xff21 0x0 0x100>; >> interrupts = > IRQ_TYPE_LEVEL_HIGH>; >> clocks = < PCLK_GPIO0>; >> >> gpio-controller; >> #gpio-cells = <2>; >> >> interrupt-controller; >> #interrupt-cells =
Re: [RFC 2/2] x86, tsc: Enable clock for ealry printk timestamp
Hi Pavel, Thanks for the revew. On Wed, Jun 06, 2018 at 11:25:22AM -0400, Pavel Tatashin wrote: > Hi Feng, > > Using a global variable for this is not going to work, because you are adding > a conditional branch and a load to a very hot path for the live of the > system, not only for the duration of the boot. Exactly. As I explained, I wanted to use the "__use_tsc" and "__sched_clock_stable" without creating new gloabl variables, but the problem is jump_label_init() can't be called that early, so I used "tsc_inited" temply just to show tsc_init() could be call early, and the printk timestamp could work much earlier. Thanks, Feng > > Pavel > > > > > +int tsc_inited; > > /* > > * TSC can be unstable due to cpufreq or due to unsynced TSCs > > */ > > @@ -192,7 +193,7 @@ static void set_cyc2ns_scale(unsigned long khz, int > > cpu, unsigned long long tsc_ > > */ > > u64 native_sched_clock(void) > > { > > - if (static_branch_likely(&__use_tsc)) { > > + if (static_branch_likely(&__use_tsc) || tsc_inited) { > > u64 tsc_now = rdtsc(); > > > > /* return the value in ns */ > > @@ -1387,30 +1391,16 @@ static int __init init_tsc_clocksource(void) > > */ > > device_initcall(init_tsc_clocksource); > > > > -void __init tsc_early_delay_calibrate(void) > > -{ > > - unsigned long lpj; > > - > > - if (!boot_cpu_has(X86_FEATURE_TSC)) > > - return; > > - > > - cpu_khz = x86_platform.calibrate_cpu(); > > - tsc_khz = x86_platform.calibrate_tsc(); > > - > > - tsc_khz = tsc_khz ? : cpu_khz; > > - if (!tsc_khz) > > - return; > > - > > - lpj = tsc_khz * 1000; > > - do_div(lpj, HZ); > > - loops_per_jiffy = lpj; > > -} > > - > > void __init tsc_init(void) > > { > > u64 lpj, cyc; > > int cpu; > > > > + if (tsc_inited) > > + return; > > + > > + tsc_inited = 1; > > + > > if (!boot_cpu_has(X86_FEATURE_TSC)) { > > setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER); > > return; > > @@ -1474,11 +1464,15 @@ void __init tsc_init(void) > > lpj = ((u64)tsc_khz * 1000); > > do_div(lpj, HZ); > > lpj_fine = lpj; > > + loops_per_jiffy = lpj; > > > > use_tsc_delay(); > > > > check_system_tsc_reliable(); > > > > + extern void early_set_sched_clock_stable(u64 sched_clock_offset); > > + early_set_sched_clock_stable(div64_u64(rdtsc() * 1000, tsc_khz)); > > + > > if (unsynchronized_tsc()) { > > mark_tsc_unstable("TSCs unsynchronized"); > > return; > > diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c > > index 10c83e7..6c5c22d 100644 > > --- a/kernel/sched/clock.c > > +++ b/kernel/sched/clock.c > > @@ -119,6 +119,13 @@ static void __scd_stamp(struct sched_clock_data *scd) > > scd->tick_raw = sched_clock(); > > } > > > > + > > +void early_set_sched_clock_stable(u64 sched_clock_offset) > > +{ > > + __sched_clock_offset = sched_clock_offset; > > + static_branch_enable(&__sched_clock_stable); > > +} > > + > > static void __set_sched_clock_stable(void) > > { > > struct sched_clock_data *scd; > > @@ -342,12 +349,14 @@ static u64 sched_clock_remote(struct sched_clock_data > > *scd) > > * > > * See cpu_clock(). > > */ > > + > > +extern int tsc_inited; > > u64 sched_clock_cpu(int cpu) > > { > > struct sched_clock_data *scd; > > u64 clock; > > > > - if (sched_clock_stable()) > > + if (sched_clock_stable() || tsc_inited) > > return sched_clock() + __sched_clock_offset; > > > > if (unlikely(!sched_clock_running)) > > > > > > > > > >>> > >>> If you have a dodgy part (sorry SKX), you'll just have to live with > >>> sched_clock starting late(r). > >>> > >>> Do not cobble things on the side, try and get the normal things running > >>> earlier.
Re: [RFC 2/2] x86, tsc: Enable clock for ealry printk timestamp
Hi Pavel, Thanks for the revew. On Wed, Jun 06, 2018 at 11:25:22AM -0400, Pavel Tatashin wrote: > Hi Feng, > > Using a global variable for this is not going to work, because you are adding > a conditional branch and a load to a very hot path for the live of the > system, not only for the duration of the boot. Exactly. As I explained, I wanted to use the "__use_tsc" and "__sched_clock_stable" without creating new gloabl variables, but the problem is jump_label_init() can't be called that early, so I used "tsc_inited" temply just to show tsc_init() could be call early, and the printk timestamp could work much earlier. Thanks, Feng > > Pavel > > > > > +int tsc_inited; > > /* > > * TSC can be unstable due to cpufreq or due to unsynced TSCs > > */ > > @@ -192,7 +193,7 @@ static void set_cyc2ns_scale(unsigned long khz, int > > cpu, unsigned long long tsc_ > > */ > > u64 native_sched_clock(void) > > { > > - if (static_branch_likely(&__use_tsc)) { > > + if (static_branch_likely(&__use_tsc) || tsc_inited) { > > u64 tsc_now = rdtsc(); > > > > /* return the value in ns */ > > @@ -1387,30 +1391,16 @@ static int __init init_tsc_clocksource(void) > > */ > > device_initcall(init_tsc_clocksource); > > > > -void __init tsc_early_delay_calibrate(void) > > -{ > > - unsigned long lpj; > > - > > - if (!boot_cpu_has(X86_FEATURE_TSC)) > > - return; > > - > > - cpu_khz = x86_platform.calibrate_cpu(); > > - tsc_khz = x86_platform.calibrate_tsc(); > > - > > - tsc_khz = tsc_khz ? : cpu_khz; > > - if (!tsc_khz) > > - return; > > - > > - lpj = tsc_khz * 1000; > > - do_div(lpj, HZ); > > - loops_per_jiffy = lpj; > > -} > > - > > void __init tsc_init(void) > > { > > u64 lpj, cyc; > > int cpu; > > > > + if (tsc_inited) > > + return; > > + > > + tsc_inited = 1; > > + > > if (!boot_cpu_has(X86_FEATURE_TSC)) { > > setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER); > > return; > > @@ -1474,11 +1464,15 @@ void __init tsc_init(void) > > lpj = ((u64)tsc_khz * 1000); > > do_div(lpj, HZ); > > lpj_fine = lpj; > > + loops_per_jiffy = lpj; > > > > use_tsc_delay(); > > > > check_system_tsc_reliable(); > > > > + extern void early_set_sched_clock_stable(u64 sched_clock_offset); > > + early_set_sched_clock_stable(div64_u64(rdtsc() * 1000, tsc_khz)); > > + > > if (unsynchronized_tsc()) { > > mark_tsc_unstable("TSCs unsynchronized"); > > return; > > diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c > > index 10c83e7..6c5c22d 100644 > > --- a/kernel/sched/clock.c > > +++ b/kernel/sched/clock.c > > @@ -119,6 +119,13 @@ static void __scd_stamp(struct sched_clock_data *scd) > > scd->tick_raw = sched_clock(); > > } > > > > + > > +void early_set_sched_clock_stable(u64 sched_clock_offset) > > +{ > > + __sched_clock_offset = sched_clock_offset; > > + static_branch_enable(&__sched_clock_stable); > > +} > > + > > static void __set_sched_clock_stable(void) > > { > > struct sched_clock_data *scd; > > @@ -342,12 +349,14 @@ static u64 sched_clock_remote(struct sched_clock_data > > *scd) > > * > > * See cpu_clock(). > > */ > > + > > +extern int tsc_inited; > > u64 sched_clock_cpu(int cpu) > > { > > struct sched_clock_data *scd; > > u64 clock; > > > > - if (sched_clock_stable()) > > + if (sched_clock_stable() || tsc_inited) > > return sched_clock() + __sched_clock_offset; > > > > if (unlikely(!sched_clock_running)) > > > > > > > > > >>> > >>> If you have a dodgy part (sorry SKX), you'll just have to live with > >>> sched_clock starting late(r). > >>> > >>> Do not cobble things on the side, try and get the normal things running > >>> earlier.
Hi
Am a United State Army here in Afghanistan, am seeking your help to evacuate the sum of $ 7,000,000 to you as long as I am assured it will be safe That in your care Until I complete my service here in Afghanistan. This is not stolen money and there are no dangers involved. I count on your understanding. Please get back for more information :. rob322...@gmail.com
Hi
Am a United State Army here in Afghanistan, am seeking your help to evacuate the sum of $ 7,000,000 to you as long as I am assured it will be safe That in your care Until I complete my service here in Afghanistan. This is not stolen money and there are no dangers involved. I count on your understanding. Please get back for more information :. rob322...@gmail.com
Re: [PATCH v4 1/7] interconnect: Add generic on-chip interconnect API
On Wed, Jun 6, 2018 at 11:09 AM Georgi Djakov wrote: > > Hi Evan, > > On 06/06/2018 05:59 PM, Georgi Djakov wrote: > >>> + > >>> +/** > >>> + * icc_node_create() - create a node > >>> + * @id: node id > >>> + * > >>> + * Return: icc_node pointer on success, or ERR_PTR() on error > >>> + */ > >>> +struct icc_node *icc_node_create(int id) > >>> +{ > >>> + struct icc_node *node; > >>> + > >>> + /* check if node already exists */ > >>> + node = node_find(id); > >>> + if (node) > >>> + return node; > >> > >> This is probably going to do more harm than good once icc_node_delete comes > >> in, since it almost certainly indicates a programmer error or ID collision, > >> and will likely result in a double free. We should probably fail with > >> EEXIST instead. > > > > In the current approach we create the nodes one by one, and the linked > > nodes are created when they are referenced. The other way around would > > be to create first all the nodes and then populate the links to avoid > > the "chicken and egg" problem. > > > > Just to elaborate a bit more on that: We can't actually register all the > nodes in advance, as we might have multiple interconnect providers > probing in different order. Each provider may have nodes linking to > nodes belonging to other providers (not probed yet). That's why we > create the nodes on the first reference and then, when the actual > provider driver is probed, the rest of the node data is filled. > Ah ok, the extra explanation helped a lot. This makes sense to me. Thanks. -Evan
Re: [PATCH v4 1/7] interconnect: Add generic on-chip interconnect API
On Wed, Jun 6, 2018 at 11:09 AM Georgi Djakov wrote: > > Hi Evan, > > On 06/06/2018 05:59 PM, Georgi Djakov wrote: > >>> + > >>> +/** > >>> + * icc_node_create() - create a node > >>> + * @id: node id > >>> + * > >>> + * Return: icc_node pointer on success, or ERR_PTR() on error > >>> + */ > >>> +struct icc_node *icc_node_create(int id) > >>> +{ > >>> + struct icc_node *node; > >>> + > >>> + /* check if node already exists */ > >>> + node = node_find(id); > >>> + if (node) > >>> + return node; > >> > >> This is probably going to do more harm than good once icc_node_delete comes > >> in, since it almost certainly indicates a programmer error or ID collision, > >> and will likely result in a double free. We should probably fail with > >> EEXIST instead. > > > > In the current approach we create the nodes one by one, and the linked > > nodes are created when they are referenced. The other way around would > > be to create first all the nodes and then populate the links to avoid > > the "chicken and egg" problem. > > > > Just to elaborate a bit more on that: We can't actually register all the > nodes in advance, as we might have multiple interconnect providers > probing in different order. Each provider may have nodes linking to > nodes belonging to other providers (not probed yet). That's why we > create the nodes on the first reference and then, when the actual > provider driver is probed, the rest of the node data is filled. > Ah ok, the extra explanation helped a lot. This makes sense to me. Thanks. -Evan
Re: [PATCH v3 1/2] power: supply: sbs-battery: don't assume MANUFACTURER_DATA formats
Hi Phil, On Wed, Jun 06, 2018 at 10:03:59AM +0800, Phil Reid wrote: > On 2/06/2018 09:28, Brian Norris wrote: > > drivers/power/supply/sbs-battery.c | 54 +- > > 1 file changed, 46 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/power/supply/sbs-battery.c > > b/drivers/power/supply/sbs-battery.c > > index 83d7b4115857..a9691ea42f44 100644 > > --- a/drivers/power/supply/sbs-battery.c > > +++ b/drivers/power/supply/sbs-battery.c ... > > @@ -806,6 +837,7 @@ static int sbs_probe(struct i2c_client *client, > > if (!chip) > > return -ENOMEM; > > + chip->flags = (u32)(uintptr_t)of_device_get_match_data(>dev); > > chip->client = client; > > chip->enable_detection = false; > > psy_cfg.of_node = client->dev.of_node; > > @@ -915,12 +947,15 @@ static int sbs_suspend(struct device *dev) > > if (chip->poll_time > 0) > > cancel_delayed_work_sync(>work); > > - /* > > -* Write to manufacturer access with sleep command. > > -* Support is manufacturer dependend, so ignore errors. > > -*/ > > - sbs_write_word_data(client, sbs_data[REG_MANUFACTURER_DATA].addr, > > - MANUFACTURER_ACCESS_SLEEP); > > + if (chip->flags & SBS_FLAGS_TI_BQ20Z75) { > > + /* > > +* Write to manufacturer access with sleep command. > > +* Support is manufacturer dependent, so ignore errors. > > +*/ > > + sbs_write_word_data(client, > > + sbs_data[REG_MANUFACTURER_DATA].addr, > > + MANUFACTURER_ACCESS_SLEEP); > > + } > > Now that this is only being done for only TI devices that support this I > would think the comment > about ignoring errors needs to go, and errors checks added. > Ditto in the new sbs_get_ti_battery_presence_and_health() Seems reasonable. I'll look at doing that and respinning. > > return 0; > > } Thanks, Brian
Re: [PATCH v3 1/2] power: supply: sbs-battery: don't assume MANUFACTURER_DATA formats
Hi Phil, On Wed, Jun 06, 2018 at 10:03:59AM +0800, Phil Reid wrote: > On 2/06/2018 09:28, Brian Norris wrote: > > drivers/power/supply/sbs-battery.c | 54 +- > > 1 file changed, 46 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/power/supply/sbs-battery.c > > b/drivers/power/supply/sbs-battery.c > > index 83d7b4115857..a9691ea42f44 100644 > > --- a/drivers/power/supply/sbs-battery.c > > +++ b/drivers/power/supply/sbs-battery.c ... > > @@ -806,6 +837,7 @@ static int sbs_probe(struct i2c_client *client, > > if (!chip) > > return -ENOMEM; > > + chip->flags = (u32)(uintptr_t)of_device_get_match_data(>dev); > > chip->client = client; > > chip->enable_detection = false; > > psy_cfg.of_node = client->dev.of_node; > > @@ -915,12 +947,15 @@ static int sbs_suspend(struct device *dev) > > if (chip->poll_time > 0) > > cancel_delayed_work_sync(>work); > > - /* > > -* Write to manufacturer access with sleep command. > > -* Support is manufacturer dependend, so ignore errors. > > -*/ > > - sbs_write_word_data(client, sbs_data[REG_MANUFACTURER_DATA].addr, > > - MANUFACTURER_ACCESS_SLEEP); > > + if (chip->flags & SBS_FLAGS_TI_BQ20Z75) { > > + /* > > +* Write to manufacturer access with sleep command. > > +* Support is manufacturer dependent, so ignore errors. > > +*/ > > + sbs_write_word_data(client, > > + sbs_data[REG_MANUFACTURER_DATA].addr, > > + MANUFACTURER_ACCESS_SLEEP); > > + } > > Now that this is only being done for only TI devices that support this I > would think the comment > about ignoring errors needs to go, and errors checks added. > Ditto in the new sbs_get_ti_battery_presence_and_health() Seems reasonable. I'll look at doing that and respinning. > > return 0; > > } Thanks, Brian
Re: [PATCH v2 2/3] perf script python: Add more PMU fields
On 6/7/2018 2:36 AM, Arnaldo Carvalho de Melo wrote: Em Fri, Jun 01, 2018 at 05:01:02PM +0800, Jin Yao escreveu: +static int get_symoff(struct symbol *sym, struct addr_location *al, + bool print_off, char *bf, int size) +{ + unsigned long offset; + + if (!sym || !sym->name) + return scnprintf(bf, size, "%s", "[unknown]"); 5254.22 ubuntu:17.04 : FAIL gcc (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406 CC /tmp/build/perf/util/scripting-engines/trace-event-python.o util/scripting-engines/trace-event-python.c:534:20: error: address of array 'sym->name' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] if (!sym || !sym->name) ~~^~~~ 1 error generated. mv: cannot stat '/tmp/build/perf/util/scripting-engines/.trace-event-python.o.tmp': No such file or directory /git/linux/tools/build/Makefile.build:96: recipe for target '/tmp/build/perf/util/scripting-engines/trace-event-python.o' failed make[5]: *** [/tmp/build/perf/util/scripting-engines/trace-event-python.o] Error 1 Because: struct symbol { struct rb_node rb_node; u64 start; u64 end; u16 namelen; charname[0]; }; It sym->name is not a pointer, in symbol's constructor we have: struct symbol *symbol__new(u64 start, u64 len, u8 binding, u8 type, const char *name) { size_t namelen = strlen(name) + 1; struct symbol *sym = calloc(1, (symbol_conf.priv_size + sizeof(*sym) + namelen)); if (sym == NULL) return NULL; sym->namelen = namelen - 1; memcpy(sym->name, name, namelen); return sym; } So it is at least 1 char long, the test above should be: if (!sym || !sym->name[0]) return scnprintf(bf, size, "%s", "[unknown]"); I'm fixing this up here. - Arnaldo Oh, yes, my mistake, very sorry about that. Thanks for helping me to fix this. Thanks Jin Yao
Re: [PATCH v2 2/3] perf script python: Add more PMU fields
On 6/7/2018 2:36 AM, Arnaldo Carvalho de Melo wrote: Em Fri, Jun 01, 2018 at 05:01:02PM +0800, Jin Yao escreveu: +static int get_symoff(struct symbol *sym, struct addr_location *al, + bool print_off, char *bf, int size) +{ + unsigned long offset; + + if (!sym || !sym->name) + return scnprintf(bf, size, "%s", "[unknown]"); 5254.22 ubuntu:17.04 : FAIL gcc (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406 CC /tmp/build/perf/util/scripting-engines/trace-event-python.o util/scripting-engines/trace-event-python.c:534:20: error: address of array 'sym->name' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] if (!sym || !sym->name) ~~^~~~ 1 error generated. mv: cannot stat '/tmp/build/perf/util/scripting-engines/.trace-event-python.o.tmp': No such file or directory /git/linux/tools/build/Makefile.build:96: recipe for target '/tmp/build/perf/util/scripting-engines/trace-event-python.o' failed make[5]: *** [/tmp/build/perf/util/scripting-engines/trace-event-python.o] Error 1 Because: struct symbol { struct rb_node rb_node; u64 start; u64 end; u16 namelen; charname[0]; }; It sym->name is not a pointer, in symbol's constructor we have: struct symbol *symbol__new(u64 start, u64 len, u8 binding, u8 type, const char *name) { size_t namelen = strlen(name) + 1; struct symbol *sym = calloc(1, (symbol_conf.priv_size + sizeof(*sym) + namelen)); if (sym == NULL) return NULL; sym->namelen = namelen - 1; memcpy(sym->name, name, namelen); return sym; } So it is at least 1 char long, the test above should be: if (!sym || !sym->name[0]) return scnprintf(bf, size, "%s", "[unknown]"); I'm fixing this up here. - Arnaldo Oh, yes, my mistake, very sorry about that. Thanks for helping me to fix this. Thanks Jin Yao
[PATCH] staging: rtl8723bs: drop test
The test selects between two identical values, so it doesn't look useful. It turns out that the tested expression can only be true anyway, so drop the test, the corresponding parameter, and the corresponding argument at the only call site. The semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression e,e1; @@ * e ? e1 : e1 // Signed-off-by: Julia Lawall --- drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c index 2ee25b2..53d3bdf 100644 --- a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c +++ b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c @@ -1352,7 +1352,6 @@ static void _PHY_ReloadMACRegisters8723B( static void _PHY_PathADDAOn8723B( struct adapter *padapter, u32 *ADDAReg, - bool isPathAOn, bool is2T ) { @@ -1363,7 +1362,7 @@ static void _PHY_PathADDAOn8723B( ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD, ("ADDA ON.\n")); - pathOn = isPathAOn ? 0x01c00014 : 0x01c00014; + pathOn = 0x01c00014; if (false == is2T) { pathOn = 0x01c00014; PHY_SetBBReg(pDM_Odm->Adapter, ADDAReg[0], bMaskDWord, 0x01c00014); @@ -1556,7 +1555,7 @@ static void phy_IQCalibrate_8723B( } ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD, ("IQ Calibration for %s for %d times\n", (is2T ? "2T2R" : "1T1R"), t)); - _PHY_PathADDAOn8723B(padapter, ADDA_REG, true, is2T); + _PHY_PathADDAOn8723B(padapter, ADDA_REG, is2T); /* no serial mode */
[PATCH] staging: rtl8723bs: drop test
The test selects between two identical values, so it doesn't look useful. It turns out that the tested expression can only be true anyway, so drop the test, the corresponding parameter, and the corresponding argument at the only call site. The semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression e,e1; @@ * e ? e1 : e1 // Signed-off-by: Julia Lawall --- drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c index 2ee25b2..53d3bdf 100644 --- a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c +++ b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c @@ -1352,7 +1352,6 @@ static void _PHY_ReloadMACRegisters8723B( static void _PHY_PathADDAOn8723B( struct adapter *padapter, u32 *ADDAReg, - bool isPathAOn, bool is2T ) { @@ -1363,7 +1362,7 @@ static void _PHY_PathADDAOn8723B( ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD, ("ADDA ON.\n")); - pathOn = isPathAOn ? 0x01c00014 : 0x01c00014; + pathOn = 0x01c00014; if (false == is2T) { pathOn = 0x01c00014; PHY_SetBBReg(pDM_Odm->Adapter, ADDAReg[0], bMaskDWord, 0x01c00014); @@ -1556,7 +1555,7 @@ static void phy_IQCalibrate_8723B( } ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD, ("IQ Calibration for %s for %d times\n", (is2T ? "2T2R" : "1T1R"), t)); - _PHY_PathADDAOn8723B(padapter, ADDA_REG, true, is2T); + _PHY_PathADDAOn8723B(padapter, ADDA_REG, is2T); /* no serial mode */
Re: [RFC 0/1] mconf: Emacs-like isearch
Randy Dunlap writes: > On 06/06/2018 03:32 PM, Dirk Gouders wrote: >> Randy Dunlap writes: >> >>> On 06/06/2018 02:56 PM, Dirk Gouders wrote: Hello, being an Emacs user, I frequently find myself pressing CTRL-s in mconf to search for some menu entry, especially in large menus. I decided to implement a basic isearch in mconf and would like to hear if others find this functionality useful, as well. The new functionality is started with pressing CTRL-s followed by characters that form the search string. To search for further occurences of an entered string, press CTRL-s instead of further characters. For example: to navigate to the USB device drivers, press CTRL-s de ENTER ENTER usb ENTER ENTER >>> >>> Not being an emacs user, what is the "de" for above? >> >> "de" (with my .config) causes a match for "Device Drivers" -- >> no other menu entry matching the string "de" is befor that entry. >> > > Device Drivers is the first match for me also. > > To get to the USB drivers, I have to enter: > CTRL-s de ENTER ENTER CTRL-s usb ENTER ENTER Yes, I left out the second CTRL-s, thank you! Oh well, I shouldn't have sent this late at night, then I would probably have explained the needed input as: 1) CTRL-s // start isearch 2) de // substring that matches "Device Drivers" 3) ENTER // quit isearch 4) ENTER // enter Device Drivers menu 5) CTRL-s // start isearch 6) usb // navigate to USB support 7) ENTER // quit isearch 8) ENTER // enter USB support menu Again, I'm really sorry for the confusion. Dirk
Re: [RFC 0/1] mconf: Emacs-like isearch
Randy Dunlap writes: > On 06/06/2018 03:32 PM, Dirk Gouders wrote: >> Randy Dunlap writes: >> >>> On 06/06/2018 02:56 PM, Dirk Gouders wrote: Hello, being an Emacs user, I frequently find myself pressing CTRL-s in mconf to search for some menu entry, especially in large menus. I decided to implement a basic isearch in mconf and would like to hear if others find this functionality useful, as well. The new functionality is started with pressing CTRL-s followed by characters that form the search string. To search for further occurences of an entered string, press CTRL-s instead of further characters. For example: to navigate to the USB device drivers, press CTRL-s de ENTER ENTER usb ENTER ENTER >>> >>> Not being an emacs user, what is the "de" for above? >> >> "de" (with my .config) causes a match for "Device Drivers" -- >> no other menu entry matching the string "de" is befor that entry. >> > > Device Drivers is the first match for me also. > > To get to the USB drivers, I have to enter: > CTRL-s de ENTER ENTER CTRL-s usb ENTER ENTER Yes, I left out the second CTRL-s, thank you! Oh well, I shouldn't have sent this late at night, then I would probably have explained the needed input as: 1) CTRL-s // start isearch 2) de // substring that matches "Device Drivers" 3) ENTER // quit isearch 4) ENTER // enter Device Drivers menu 5) CTRL-s // start isearch 6) usb // navigate to USB support 7) ENTER // quit isearch 8) ENTER // enter USB support menu Again, I'm really sorry for the confusion. Dirk
Re: [PATCH] slab: Clean up the code comment in slab kmem_cache struct
On Wed, 6 Jun 2018, Baoquan He wrote: > I am back porting Thomas's sl[a|u]b freelist randomization feature to > our distros, need go through slab code for better understanding. From > git log history, they were 'obj_offset' and 'obj_size'. Later on > 'obj_size' was renamed to 'object_size' in commit 3b0efdfa1e("mm, sl[aou]b: > Extract common fields from struct kmem_cache") which is from your patch. > With my understanding, I guess you changed that on purpose because > object_size is size of each object, obj_offset is for the whole cache, > representing the offset the real object starts to be stored. And putting > them separately is for better desribing them in code comment and > distinction, e.g 'object_size' is in "4) cache creation/removal", > while 'obj_offset' is put alone to indicate it's for the whole. obj_offset only applies when CONFIG_SLAB_DEBUG is set. Ok so that screwy name also indicates that something special goes on.
Re: [PATCH] slab: Clean up the code comment in slab kmem_cache struct
On Wed, 6 Jun 2018, Baoquan He wrote: > I am back porting Thomas's sl[a|u]b freelist randomization feature to > our distros, need go through slab code for better understanding. From > git log history, they were 'obj_offset' and 'obj_size'. Later on > 'obj_size' was renamed to 'object_size' in commit 3b0efdfa1e("mm, sl[aou]b: > Extract common fields from struct kmem_cache") which is from your patch. > With my understanding, I guess you changed that on purpose because > object_size is size of each object, obj_offset is for the whole cache, > representing the offset the real object starts to be stored. And putting > them separately is for better desribing them in code comment and > distinction, e.g 'object_size' is in "4) cache creation/removal", > while 'obj_offset' is put alone to indicate it's for the whole. obj_offset only applies when CONFIG_SLAB_DEBUG is set. Ok so that screwy name also indicates that something special goes on.
Re: [PATCH v4 0/6] Enhance support for the SP805 WDT
Hi Florian, On 6/6/2018 12:29 PM, Florian Fainelli wrote: On 05/28/2018 11:01 AM, Ray Jui wrote: This patch series enhances the support for the SP805 watchdog timer. First of all, 'timeout-sec' devicetree property is added. In addition, support is also added to allow the driver to reset the watchdog if it has been detected that watchdot has been started in the bootloader. In this case, the driver will initiate the ping service from the kernel watchdog subsystem, before a user mode daemon takes over. This series also enables SP805 in the default ARM64 defconfig This patch series is based off v4.17-rc5 and is available on GIHUB: repo: https://github.com/Broadcom/arm64-linux.git branch: sp805-wdt-v4 Changes since v3: - Improve description of 'timeout-sec' in the binding document, per recommendation from Guenter Roeck Changes since v2: - Fix indent and format to make them consistent within arm,sp805.txt Changes since v1: - Consolidate two duplicated SP805 binding documents into one - Slight change of the wdt_is_running implementation per discussion Ray Jui (6): Documentation: DT: Consolidate SP805 binding docs Documentation: DT: Add optional 'timeout-sec' property for sp805 watchdog: sp805: add 'timeout-sec' DT property support watchdog: sp805: set WDOG_HW_RUNNING when appropriate arm64: dt: set initial SR watchdog timeout to 60 seconds arm64: defconfig: add CONFIG_ARM_SP805_WATCHDOG I can take the last two patches and Guenter would take the first 4 or would you want to proceed differently? It looks like we still need to figure out how to proceed based on discussions with Rob and Guenter on 1/6? Thanks, Ray
Re: [PATCH v4 0/6] Enhance support for the SP805 WDT
Hi Florian, On 6/6/2018 12:29 PM, Florian Fainelli wrote: On 05/28/2018 11:01 AM, Ray Jui wrote: This patch series enhances the support for the SP805 watchdog timer. First of all, 'timeout-sec' devicetree property is added. In addition, support is also added to allow the driver to reset the watchdog if it has been detected that watchdot has been started in the bootloader. In this case, the driver will initiate the ping service from the kernel watchdog subsystem, before a user mode daemon takes over. This series also enables SP805 in the default ARM64 defconfig This patch series is based off v4.17-rc5 and is available on GIHUB: repo: https://github.com/Broadcom/arm64-linux.git branch: sp805-wdt-v4 Changes since v3: - Improve description of 'timeout-sec' in the binding document, per recommendation from Guenter Roeck Changes since v2: - Fix indent and format to make them consistent within arm,sp805.txt Changes since v1: - Consolidate two duplicated SP805 binding documents into one - Slight change of the wdt_is_running implementation per discussion Ray Jui (6): Documentation: DT: Consolidate SP805 binding docs Documentation: DT: Add optional 'timeout-sec' property for sp805 watchdog: sp805: add 'timeout-sec' DT property support watchdog: sp805: set WDOG_HW_RUNNING when appropriate arm64: dt: set initial SR watchdog timeout to 60 seconds arm64: defconfig: add CONFIG_ARM_SP805_WATCHDOG I can take the last two patches and Guenter would take the first 4 or would you want to proceed differently? It looks like we still need to figure out how to proceed based on discussions with Rob and Guenter on 1/6? Thanks, Ray
Re: [PATCH v4 1/6] Documentation: DT: Consolidate SP805 binding docs
On 6/6/2018 9:33 AM, Rob Herring wrote: On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck wrote: On 06/05/2018 12:41 PM, Rob Herring wrote: On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote: Consolidate two SP805 binding documents "arm,sp805.txt" and "sp805-wdt.txt" into "arm,sp805.txt" that matches the naming of the desired compatible string to be used Signed-off-by: Ray Jui --- .../devicetree/bindings/watchdog/arm,sp805.txt | 27 ++- .../devicetree/bindings/watchdog/sp805-wdt.txt | 31 -- 2 files changed, 20 insertions(+), 38 deletions(-) delete mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt Would be good to get a ACK from FSL/NXP person on this. It looks to me like the driver fetches the wrong clock as it gets the first one and the driver really wants 'wdog_clk'. In any case, their dts files should be updated. This is really confusing, since he deleted file lists apb_pclk first. Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear to me. arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at least it says so. Note that that clock source is 32KHz. That is obviously a mistake because no one clocks their bus/register interface at 32KHz. Someone just filled in something that happened to work. The fsl dts files all have apb_pclk first. It's all kind of a mess, but fortunately one we should be able to clean-up. It is indeed a mess. Note the SP805 driver only derive one clock from DT, and that's not done based on name. As a result, the first clock defined in DT will be fetched and the rate calculation will be carried out based on that clock rate. I assumed the clock entries and their names defined in the binding document are just placeholders, at least for the 2nd clock. Based on how the current driver is, the first clock needs to be the WDOGCLK for things to work properly. According to the SP805 TRM, APB clock is the PCLK, that drives the bus for register access. The relationship between WDOGCLK and PCLK is defined as: - the rising edges of WDOGCLK must be synchronous and balanced with a rising edge of PCLK - the WDOGCLK frequency cannot be greater than the PCLK frequency The compatible string changes too, but AMBA bus devices don't actually use the compatible string as they use the ID registers to match. I suppose some other OS could do things differently. Worth the risk to clean-up IMO. Either case, why are two clocks asked for in the first place ? Are there situations where the second clock is actually used/useful ? For clocks, the bus needs "apb_pclk" and the driver just gets the first clock. The driver is obviously going to want the functional clock that determines the counter rate. That should Primecell peripherals are about the only ones that have clear specs WRT clock inputs. Yet we've still managed to screw them up. There are 2 clocks in the spec, so the DT has (or should have) 2 clocks. Rob Let me know how you guys want to proceed with this? Thanks, Ray
Re: [PATCH v4 1/6] Documentation: DT: Consolidate SP805 binding docs
On 6/6/2018 9:33 AM, Rob Herring wrote: On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck wrote: On 06/05/2018 12:41 PM, Rob Herring wrote: On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote: Consolidate two SP805 binding documents "arm,sp805.txt" and "sp805-wdt.txt" into "arm,sp805.txt" that matches the naming of the desired compatible string to be used Signed-off-by: Ray Jui --- .../devicetree/bindings/watchdog/arm,sp805.txt | 27 ++- .../devicetree/bindings/watchdog/sp805-wdt.txt | 31 -- 2 files changed, 20 insertions(+), 38 deletions(-) delete mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt Would be good to get a ACK from FSL/NXP person on this. It looks to me like the driver fetches the wrong clock as it gets the first one and the driver really wants 'wdog_clk'. In any case, their dts files should be updated. This is really confusing, since he deleted file lists apb_pclk first. Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear to me. arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at least it says so. Note that that clock source is 32KHz. That is obviously a mistake because no one clocks their bus/register interface at 32KHz. Someone just filled in something that happened to work. The fsl dts files all have apb_pclk first. It's all kind of a mess, but fortunately one we should be able to clean-up. It is indeed a mess. Note the SP805 driver only derive one clock from DT, and that's not done based on name. As a result, the first clock defined in DT will be fetched and the rate calculation will be carried out based on that clock rate. I assumed the clock entries and their names defined in the binding document are just placeholders, at least for the 2nd clock. Based on how the current driver is, the first clock needs to be the WDOGCLK for things to work properly. According to the SP805 TRM, APB clock is the PCLK, that drives the bus for register access. The relationship between WDOGCLK and PCLK is defined as: - the rising edges of WDOGCLK must be synchronous and balanced with a rising edge of PCLK - the WDOGCLK frequency cannot be greater than the PCLK frequency The compatible string changes too, but AMBA bus devices don't actually use the compatible string as they use the ID registers to match. I suppose some other OS could do things differently. Worth the risk to clean-up IMO. Either case, why are two clocks asked for in the first place ? Are there situations where the second clock is actually used/useful ? For clocks, the bus needs "apb_pclk" and the driver just gets the first clock. The driver is obviously going to want the functional clock that determines the counter rate. That should Primecell peripherals are about the only ones that have clear specs WRT clock inputs. Yet we've still managed to screw them up. There are 2 clocks in the spec, so the DT has (or should have) 2 clocks. Rob Let me know how you guys want to proceed with this? Thanks, Ray
Re: [RFC 0/1] mconf: Emacs-like isearch
On 06/06/2018 03:32 PM, Dirk Gouders wrote: > Randy Dunlap writes: > >> On 06/06/2018 02:56 PM, Dirk Gouders wrote: >>> Hello, >>> >>> being an Emacs user, I frequently find myself pressing CTRL-s in mconf >>> to search for some menu entry, especially in large menus. >>> >>> I decided to implement a basic isearch in mconf and would like to hear >>> if others find this functionality useful, as well. >>> >>> The new functionality is started with pressing CTRL-s followed by >>> characters that form the search string. To search for further >>> occurences of an entered string, press CTRL-s instead of further >>> characters. >>> >>> For example: to navigate to the USB device drivers, press CTRL-s de ENTER >>> ENTER usb ENTER ENTER >> >> Not being an emacs user, what is the "de" for above? > > "de" (with my .config) causes a match for "Device Drivers" -- > no other menu entry matching the string "de" is befor that entry. > Device Drivers is the first match for me also. To get to the USB drivers, I have to enter: CTRL-s de ENTER ENTER CTRL-s usb ENTER ENTER >> >>> Pressing just CTRL-s subsequently results in line-by-line navigation >>> through the menu (search for empty strings). >>> >>> The isearch is terminated by pressing either ESC ESC or ENTER. >>> >>> Because I expect that errors are found in the code and changes are >>> requested, I >>> completely left out the documentation part and will add it to V2 >>> should anyone find this functionality useful. >> >> Hm, it seems to take 2 entries of Ctrl-s to begin the search?? >> No, it takes 2 entries of Ctrl-s to display the "isearch:" prompt, >> but entering one Ctrl-s + a string will display it also. >> >> Anyway, I am having trouble getting the USB drivers example to work. > > Yes, I should have stated that this example explains what _I_ have to do to > navigate to the USB device drivers. Probably, "de" in your case matches > some other menu entry and the navigation requires more or other input. > > I'm sorry for causing confusion. No problem. Thanks. > Dirk > >>> Thanks, >>> >>> Dirk >>> >>> Dirk Gouders (1): >>> Emacs-like isearch for mconf. >>> >>> scripts/kconfig/lxdialog/dialog.h | 5 ++ >>> scripts/kconfig/lxdialog/menubox.c | 140 >>> - >>> scripts/kconfig/lxdialog/util.c| 1 + >>> 3 files changed, 145 insertions(+), 1 deletion(-) >>> >> >> thanks, -- ~Randy
Re: [RFC 0/1] mconf: Emacs-like isearch
On 06/06/2018 03:32 PM, Dirk Gouders wrote: > Randy Dunlap writes: > >> On 06/06/2018 02:56 PM, Dirk Gouders wrote: >>> Hello, >>> >>> being an Emacs user, I frequently find myself pressing CTRL-s in mconf >>> to search for some menu entry, especially in large menus. >>> >>> I decided to implement a basic isearch in mconf and would like to hear >>> if others find this functionality useful, as well. >>> >>> The new functionality is started with pressing CTRL-s followed by >>> characters that form the search string. To search for further >>> occurences of an entered string, press CTRL-s instead of further >>> characters. >>> >>> For example: to navigate to the USB device drivers, press CTRL-s de ENTER >>> ENTER usb ENTER ENTER >> >> Not being an emacs user, what is the "de" for above? > > "de" (with my .config) causes a match for "Device Drivers" -- > no other menu entry matching the string "de" is befor that entry. > Device Drivers is the first match for me also. To get to the USB drivers, I have to enter: CTRL-s de ENTER ENTER CTRL-s usb ENTER ENTER >> >>> Pressing just CTRL-s subsequently results in line-by-line navigation >>> through the menu (search for empty strings). >>> >>> The isearch is terminated by pressing either ESC ESC or ENTER. >>> >>> Because I expect that errors are found in the code and changes are >>> requested, I >>> completely left out the documentation part and will add it to V2 >>> should anyone find this functionality useful. >> >> Hm, it seems to take 2 entries of Ctrl-s to begin the search?? >> No, it takes 2 entries of Ctrl-s to display the "isearch:" prompt, >> but entering one Ctrl-s + a string will display it also. >> >> Anyway, I am having trouble getting the USB drivers example to work. > > Yes, I should have stated that this example explains what _I_ have to do to > navigate to the USB device drivers. Probably, "de" in your case matches > some other menu entry and the navigation requires more or other input. > > I'm sorry for causing confusion. No problem. Thanks. > Dirk > >>> Thanks, >>> >>> Dirk >>> >>> Dirk Gouders (1): >>> Emacs-like isearch for mconf. >>> >>> scripts/kconfig/lxdialog/dialog.h | 5 ++ >>> scripts/kconfig/lxdialog/menubox.c | 140 >>> - >>> scripts/kconfig/lxdialog/util.c| 1 + >>> 3 files changed, 145 insertions(+), 1 deletion(-) >>> >> >> thanks, -- ~Randy
Re: [PATCH v6 0/4] enable early printing of hashed pointers
On Mon, May 28, 2018 at 09:59:15AM -0400, Theodore Y. Ts'o wrote: > On Mon, May 28, 2018 at 11:46:38AM +1000, Tobin C. Harding wrote: > > > > During the versions of this set I have been totally confused about which > > patches go through which tree. This version again puts all 4 patches > > together in the hope they will go through Andrew's tree. > > I'm also happy to take the whole patch series through the random tree > if everyone else is OK with it. > > Thoughts? Looks like every one is happy (or silent) now Ted, can you please take this version through your tree. thanks, Tobin.
Re: [PATCH v6 0/4] enable early printing of hashed pointers
On Mon, May 28, 2018 at 09:59:15AM -0400, Theodore Y. Ts'o wrote: > On Mon, May 28, 2018 at 11:46:38AM +1000, Tobin C. Harding wrote: > > > > During the versions of this set I have been totally confused about which > > patches go through which tree. This version again puts all 4 patches > > together in the hope they will go through Andrew's tree. > > I'm also happy to take the whole patch series through the random tree > if everyone else is OK with it. > > Thoughts? Looks like every one is happy (or silent) now Ted, can you please take this version through your tree. thanks, Tobin.
Re: [PATCH v6 0/4] enable early printing of hashed pointers
On Wed, Jun 06, 2018 at 03:02:20PM +0200, Thomas Gleixner wrote: > On Mon, 28 May 2018, Tobin C. Harding wrote: > > > Currently printing pointers early in the boot sequence can result in a > > dummy string '(ptrval)' being printed. While resolving this > > issue it was noticed that we can use the hw RNG if available for hashing > > pointers. > > > > Patch one and two do the ground work to be able to use hw RNG removing > > from get_random_bytes_arch() the call to get_random_bytes() and > > returning the number of bytes of random material successfully returned. > > > > Patch three uses the hw RNG to get keying material if it is available. > > > > Patch four further assists debugging early in the boot sequence for > > machines that do not have a hw RNG by adding a command line option > > 'debug_boot_weak_hash'. If enabled, non-cryptographically secure hashing > > is used instead of siphash so we can hash at any time. > > > > During the versions of this set I have been totally confused about which > > patches go through which tree. This version again puts all 4 patches > > together in the hope they will go through Andrew's tree. > > > > > > Steve, > > > > Could you please take a quick squiz at the final 2 patches if you get a > > chance. I assumed we are in preemptible context during early_init based > > on your code (and code comment) and called static_branch_disable() > > directly if hw RNG returned keying material. It's a pretty simple > > change but I'd love to get someone else to check I've not noob'ed it. > > early_initcalls() are not that early :) They run in thread context fully > preemtible so calling static_branch_disable() is perfectly fine. Thanks for the explanation. Tobin.
Re: [PATCH v6 0/4] enable early printing of hashed pointers
On Wed, Jun 06, 2018 at 03:02:20PM +0200, Thomas Gleixner wrote: > On Mon, 28 May 2018, Tobin C. Harding wrote: > > > Currently printing pointers early in the boot sequence can result in a > > dummy string '(ptrval)' being printed. While resolving this > > issue it was noticed that we can use the hw RNG if available for hashing > > pointers. > > > > Patch one and two do the ground work to be able to use hw RNG removing > > from get_random_bytes_arch() the call to get_random_bytes() and > > returning the number of bytes of random material successfully returned. > > > > Patch three uses the hw RNG to get keying material if it is available. > > > > Patch four further assists debugging early in the boot sequence for > > machines that do not have a hw RNG by adding a command line option > > 'debug_boot_weak_hash'. If enabled, non-cryptographically secure hashing > > is used instead of siphash so we can hash at any time. > > > > During the versions of this set I have been totally confused about which > > patches go through which tree. This version again puts all 4 patches > > together in the hope they will go through Andrew's tree. > > > > > > Steve, > > > > Could you please take a quick squiz at the final 2 patches if you get a > > chance. I assumed we are in preemptible context during early_init based > > on your code (and code comment) and called static_branch_disable() > > directly if hw RNG returned keying material. It's a pretty simple > > change but I'd love to get someone else to check I've not noob'ed it. > > early_initcalls() are not that early :) They run in thread context fully > preemtible so calling static_branch_disable() is perfectly fine. Thanks for the explanation. Tobin.
Re: [PATCH v6 0/4] enable early printing of hashed pointers
On Wed, Jun 06, 2018 at 03:08:25PM +0200, Anna-Maria Gleixner wrote: > On Tue, 5 Jun 2018, Anna-Maria Gleixner wrote: > > > On Thu, 31 May 2018, Steven Rostedt wrote: > > > > > On Mon, 28 May 2018 11:46:38 +1000 > > > "Tobin C. Harding" wrote: > > > > > > > Steve, > > > > > > Hi Tobin, > > > > > > Sorry for the late reply, I'm currently at a conference and have had > > > little time to read email. > > > > > > > > > > > Could you please take a quick squiz at the final 2 patches if you get a > > > > chance. I assumed we are in preemptible context during early_init based > > > > on your code (and code comment) and called static_branch_disable() > > > > directly if hw RNG returned keying material. It's a pretty simple > > > > change but I'd love to get someone else to check I've not noob'ed it. > > > > > > I can take a look, and perhaps do some tests. But it was Anna-Maria > > > that originally triggered the issue. She's on Cc, perhaps she can try > > > this and see if it works. > > > > I'll test it today - sorry for the delay. > > > > I tested it with command line option enabled. This works early enough > because it works right after early_trace_init(). Awesome, thanks Anna-Maria! Tobin
Re: [PATCH v6 0/4] enable early printing of hashed pointers
On Wed, Jun 06, 2018 at 03:08:25PM +0200, Anna-Maria Gleixner wrote: > On Tue, 5 Jun 2018, Anna-Maria Gleixner wrote: > > > On Thu, 31 May 2018, Steven Rostedt wrote: > > > > > On Mon, 28 May 2018 11:46:38 +1000 > > > "Tobin C. Harding" wrote: > > > > > > > Steve, > > > > > > Hi Tobin, > > > > > > Sorry for the late reply, I'm currently at a conference and have had > > > little time to read email. > > > > > > > > > > > Could you please take a quick squiz at the final 2 patches if you get a > > > > chance. I assumed we are in preemptible context during early_init based > > > > on your code (and code comment) and called static_branch_disable() > > > > directly if hw RNG returned keying material. It's a pretty simple > > > > change but I'd love to get someone else to check I've not noob'ed it. > > > > > > I can take a look, and perhaps do some tests. But it was Anna-Maria > > > that originally triggered the issue. She's on Cc, perhaps she can try > > > this and see if it works. > > > > I'll test it today - sorry for the delay. > > > > I tested it with command line option enabled. This works early enough > because it works right after early_trace_init(). Awesome, thanks Anna-Maria! Tobin
Re: [PATCH] ACPI LID: increment wakeup count only when notified.
On Thu, Jun 7, 2018 at 1:11 AM, Benson Leung wrote: > Hi Rafael, > > On Wed, Jun 06, 2018 at 09:00:43AM +0200, Rafael J. Wysocki wrote: >> > @@ -417,6 +414,7 @@ static void acpi_button_notify(struct acpi_device >> > *device, u32 event) >> > /* fall through */ >> > case ACPI_BUTTON_NOTIFY_STATUS: >> > input = button->input; >> > + acpi_pm_wakeup_event(>dev); >> >> Not really. >> >> There already is an acpi_pm_wakeup_event() call in the else branch below. >> > > Ravi removes that other call below. OK, I missed that, not sure why, sorry. > The intent for this is to call > acpi_pm_wakeup_event() regardless if the button->type is ACPI_BUTTON_TYPE_LID, > in case that event is ACPI_BUTTON_NOTIFY_STATUS. Well, the patch really drops the acpi_pm_wakeup_event() call from acpi_lid_notify_state() and so it has to ensure that this function will be called here for ACPI_BUTTON_NOTIFY_STATUS regardless of the button->type value. That's fine, but still the changelog doesn't make it clear why the acpi_pm_wakeup_event() call in acpi_lid_notify_state() is not necessary in general. Thanks, Rafael
Re: [PATCH] ACPI LID: increment wakeup count only when notified.
On Thu, Jun 7, 2018 at 1:11 AM, Benson Leung wrote: > Hi Rafael, > > On Wed, Jun 06, 2018 at 09:00:43AM +0200, Rafael J. Wysocki wrote: >> > @@ -417,6 +414,7 @@ static void acpi_button_notify(struct acpi_device >> > *device, u32 event) >> > /* fall through */ >> > case ACPI_BUTTON_NOTIFY_STATUS: >> > input = button->input; >> > + acpi_pm_wakeup_event(>dev); >> >> Not really. >> >> There already is an acpi_pm_wakeup_event() call in the else branch below. >> > > Ravi removes that other call below. OK, I missed that, not sure why, sorry. > The intent for this is to call > acpi_pm_wakeup_event() regardless if the button->type is ACPI_BUTTON_TYPE_LID, > in case that event is ACPI_BUTTON_NOTIFY_STATUS. Well, the patch really drops the acpi_pm_wakeup_event() call from acpi_lid_notify_state() and so it has to ensure that this function will be called here for ACPI_BUTTON_NOTIFY_STATUS regardless of the button->type value. That's fine, but still the changelog doesn't make it clear why the acpi_pm_wakeup_event() call in acpi_lid_notify_state() is not necessary in general. Thanks, Rafael
Re: [RFC 00/10] perf: Add cputime events/metrics
> I had some issues with IDLE counter being miscounted due to stopping > of the idle tick. I tried to solve it in this patch (it's part of the > patchset): > perf/cputime: Don't stop idle tick if there's live cputime event > > but I'm pretty sure it's wrong and there's better solution. At least on intel we already have hardware counters for different idle states. You just would need to add them and convert to the same unit. But of course it's still useful when this is not available. > My current plan is now to read those counters in perf top/record/report > to show (at least) the idle percentage for the current profile. It's useful. Thanks for working on it. I was thinking about doing something similar for some time. -Andi
Re: [PATCH 01/10] perf tools: Uniquify the event name if there's no other matched event
On Thu, Jun 07, 2018 at 12:15:04AM +0200, Jiri Olsa wrote: > Currently by default we try to match the user specified PMU > name to all PMU units available and use them to aggregate > all matched PMUs event counts into one 'pattern' event. > > While this is useful for uncore events, it screws up names > for other events, where this is not desirable, like: > > Before: > # perf stat -e cp/cpu-cycles/ kill I assume you mean cpU/cpu-cycles/ > >Performance counter stats for 'kill': > >1,573,757 cp/cpu-cycles/ > > Keeping the pattern matching logic, but making the name unique > in case there's no other match found. That fixes the example > above and hopefully does not screw up anything else. > > After: > # perf stat -e cp/cpu-cycles/ kill > >Performance counter stats for 'kill': > >1,573,757 cpu/cpu-cycles/ The output is 100% identical? -Andi
Re: [PATCH 01/10] perf tools: Uniquify the event name if there's no other matched event
On Thu, Jun 07, 2018 at 12:15:04AM +0200, Jiri Olsa wrote: > Currently by default we try to match the user specified PMU > name to all PMU units available and use them to aggregate > all matched PMUs event counts into one 'pattern' event. > > While this is useful for uncore events, it screws up names > for other events, where this is not desirable, like: > > Before: > # perf stat -e cp/cpu-cycles/ kill I assume you mean cpU/cpu-cycles/ > >Performance counter stats for 'kill': > >1,573,757 cp/cpu-cycles/ > > Keeping the pattern matching logic, but making the name unique > in case there's no other match found. That fixes the example > above and hopefully does not screw up anything else. > > After: > # perf stat -e cp/cpu-cycles/ kill > >Performance counter stats for 'kill': > >1,573,757 cpu/cpu-cycles/ The output is 100% identical? -Andi
Re: [RFC 00/10] perf: Add cputime events/metrics
> I had some issues with IDLE counter being miscounted due to stopping > of the idle tick. I tried to solve it in this patch (it's part of the > patchset): > perf/cputime: Don't stop idle tick if there's live cputime event > > but I'm pretty sure it's wrong and there's better solution. At least on intel we already have hardware counters for different idle states. You just would need to add them and convert to the same unit. But of course it's still useful when this is not available. > My current plan is now to read those counters in perf top/record/report > to show (at least) the idle percentage for the current profile. It's useful. Thanks for working on it. I was thinking about doing something similar for some time. -Andi