* Re: [PATCH] docs: kernel-doc: fix parsing of arrays
From: Jonathan Corbet @ 2018-03-29 21:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, johannes
In-Reply-To: <d5892e146872e40717117da6cc01e57525a063d2.1522335536.git.mchehab@s-opensource.com>
On Thu, 29 Mar 2018 10:58:59 -0400
Mauro Carvalho Chehab <mchehab@s-opensource.com> wrote:
> The logic with parses array has a bug that prevents it to
> parse arrays like:
> struct {
> ...
> struct {
> u64 msdu[IEEE80211_NUM_TIDS + 1];
> ...
> ...
>
> Fix the parser to accept it.
Applied, thanks.
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 00/32] docs/vm: convert to ReST format
From: Jonathan Corbet @ 2018-03-29 21:46 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrey Ryabinin, Richard Henderson, Ivan Kokshaysky, Matt Turner,
Tony Luck, Fenghua Yu, Ralf Baechle, James Hogan,
Michael Ellerman, Alexander Viro, linux-kernel, linux-doc,
kasan-dev, linux-alpha, linux-ia64, linux-mips, linuxppc-dev,
linux-fsdevel, linux-mm
In-Reply-To: <1521660168-14372-1-git-send-email-rppt@linux.vnet.ibm.com>
On Wed, 21 Mar 2018 21:22:16 +0200
Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> These patches convert files in Documentation/vm to ReST format, add an
> initial index and link it to the top level documentation.
>
> There are no contents changes in the documentation, except few spelling
> fixes. The relatively large diffstat stems from the indentation and
> paragraph wrapping changes.
>
> I've tried to keep the formatting as consistent as possible, but I could
> miss some places that needed markup and add some markup where it was not
> necessary.
So I've been pondering on these for a bit. It looks like a reasonable and
straightforward RST conversion, no real complaints there. But I do have a
couple of concerns...
One is that, as we move documentation into RST, I'm really trying to
organize it a bit so that it is better tuned to the various audiences we
have. For example, ksm.txt is going to be of interest to sysadmin types,
who might want to tune it. mmu_notifier.txt is of interest to ...
somebody, but probably nobody who is thinking in user space. And so on.
So I would really like to see this material split up and put into the
appropriate places in the RST hierarchy - admin-guide for administrative
stuff, core-api for kernel development topics, etc. That, of course,
could be done separately from the RST conversion, but I suspect I know
what will (or will not) happen if we agree to defer that for now :)
The other is the inevitable merge conflicts that changing that many doc
files will create. Sending the patches through Andrew could minimize
that, I guess, or at least make it his problem. Alternatively, we could
try to do it as an end-of-merge-window sort of thing. I can try to manage
that, but an ack or two from the mm crowd would be nice to have.
Thanks,
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 1/9] sysctl: Add flags to support min/max range clamping
From: Luis R. Rodriguez @ 2018-03-29 18:56 UTC (permalink / raw)
To: Waiman Long
Cc: Luis R. Rodriguez, Kees Cook, linux-kernel, linux-fsdevel,
linux-doc, Jonathan Corbet, Andrew Morton, Al Viro,
Matthew Wilcox, Eric W. Biederman
In-Reply-To: <3e2b8e6a-f6d1-f80a-ed43-e937d8ca8383@redhat.com>
On Thu, Mar 29, 2018 at 02:47:18PM -0400, Waiman Long wrote:
> On 03/29/2018 02:15 PM, Luis R. Rodriguez wrote:
> > On Mon, Mar 19, 2018 at 11:39:19AM -0400, Waiman Long wrote:
> >> On 03/16/2018 09:10 PM, Luis R. Rodriguez wrote:
> >>> On Fri, Mar 16, 2018 at 02:13:42PM -0400, Waiman Long wrote:
> >>>> When the CTL_FLAGS_CLAMP_RANGE flag is set in the ctl_table
> >>>> entry, any update from the userspace will be clamped to the given
> >>>> range without error if either the proc_dointvec_minmax() or the
> >>>> proc_douintvec_minmax() handlers is used.
> >>> I don't get it. Why define a generic range flag when we can be mores specific and
> >>> you do that in your next patch. What's the point of this flag then?
> >>>
> >>> Luis
> >> I was thinking about using the signed/unsigned bits as just annotations
> >> for ranges for future extension. For the purpose of this patchset alone,
> >> I can merge the three bits into just two.
> > Only introduce flags which you will actually use in the same patch series.
> >
> > Luis
>
> Yes, will do. Since the merge window is coming, should I wait until it
> is over to send out the new patch?
Probably best. May be too tight for review now if Linus spins out a release
this weekend.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 0/9] ipc: Clamp *mni to the real IPCMNI limit & increase that limit
From: Luis R. Rodriguez @ 2018-03-29 18:53 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Andrew Morton, Waiman Long, Kees Cook, linux-kernel,
linux-fsdevel, linux-doc, Jonathan Corbet, Al Viro,
Matthew Wilcox, Eric W. Biederman
In-Reply-To: <20180329181935.GN30543@wotan.suse.de>
On Thu, Mar 29, 2018 at 06:19:35PM +0000, Luis R. Rodriguez wrote:
> Andrew,
>
> please drop these patches until further notice. I would recommend we avoid merging
> these patches until we get proper Acked-by for the entire set. Waiman has a bit more
> work to do and even after the 5th iteration this is not right yet.
Sorry or the noise, I see they are not merged.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 1/9] sysctl: Add flags to support min/max range clamping
From: Waiman Long @ 2018-03-29 18:47 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Kees Cook, linux-kernel, linux-fsdevel, linux-doc,
Jonathan Corbet, Andrew Morton, Al Viro, Matthew Wilcox,
Eric W. Biederman
In-Reply-To: <20180329181515.GL30543@wotan.suse.de>
On 03/29/2018 02:15 PM, Luis R. Rodriguez wrote:
> On Mon, Mar 19, 2018 at 11:39:19AM -0400, Waiman Long wrote:
>> On 03/16/2018 09:10 PM, Luis R. Rodriguez wrote:
>>> On Fri, Mar 16, 2018 at 02:13:42PM -0400, Waiman Long wrote:
>>>> When the CTL_FLAGS_CLAMP_RANGE flag is set in the ctl_table
>>>> entry, any update from the userspace will be clamped to the given
>>>> range without error if either the proc_dointvec_minmax() or the
>>>> proc_douintvec_minmax() handlers is used.
>>> I don't get it. Why define a generic range flag when we can be mores specific and
>>> you do that in your next patch. What's the point of this flag then?
>>>
>>> Luis
>> I was thinking about using the signed/unsigned bits as just annotations
>> for ranges for future extension. For the purpose of this patchset alone,
>> I can merge the three bits into just two.
> Only introduce flags which you will actually use in the same patch series.
>
> Luis
Yes, will do. Since the merge window is coming, should I wait until it
is over to send out the new patch?
Cheers,
Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 0/9] ipc: Clamp *mni to the real IPCMNI limit & increase that limit
From: Luis R. Rodriguez @ 2018-03-29 18:19 UTC (permalink / raw)
To: Andrew Morton
Cc: Luis R. Rodriguez, Waiman Long, Kees Cook, linux-kernel,
linux-fsdevel, linux-doc, Jonathan Corbet, Al Viro,
Matthew Wilcox, Eric W. Biederman
In-Reply-To: <1521224030-2185-1-git-send-email-longman@redhat.com>
Andrew,
please drop these patches until further notice. I would recommend we avoid merging
these patches until we get proper Acked-by for the entire set. Waiman has a bit more
work to do and even after the 5th iteration this is not right yet.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 2/9] proc/sysctl: Provide additional ctl_table.flags checks
From: Luis R. Rodriguez @ 2018-03-29 18:16 UTC (permalink / raw)
To: Waiman Long
Cc: Luis R. Rodriguez, Kees Cook, linux-kernel, linux-fsdevel,
linux-doc, Jonathan Corbet, Andrew Morton, Al Viro,
Matthew Wilcox, Eric W. Biederman
In-Reply-To: <d616e759-368f-6287-3492-a970d2e8f77a@redhat.com>
On Mon, Mar 19, 2018 at 11:35:19AM -0400, Waiman Long wrote:
> On 03/16/2018 08:54 PM, Luis R. Rodriguez wrote:
> > On Fri, Mar 16, 2018 at 02:13:43PM -0400, Waiman Long wrote:
> >> Checking code is added to provide the following additional
> >> ctl_table.flags checks:
> >>
> >> 1) No unknown flag is allowed.
> >> 2) Minimum of a range cannot be larger than the maximum value.
> >> 3) The signed and unsigned flags are mutually exclusive.
> >> 4) The proc_handler should be consistent with the signed or unsigned
> >> flags.
> >>
> >> Two new flags are added to indicate if the min/max values are signed
> >> or unsigned - CTL_FLAGS_SIGNED_RANGE & CTL_FLAGS_UNSIGNED_RANGE.
> >> These 2 flags can be optionally enabled for range checking purpose.
> >> But either one of them must be set with CTL_FLAGS_CLAMP_RANGE.
> >>
> >> Signed-off-by: Waiman Long <longman@redhat.com>
> >> ---
> >> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> >> index e446e1f..088f032 100644
> >> --- a/include/linux/sysctl.h
> >> +++ b/include/linux/sysctl.h
> >> @@ -134,14 +134,26 @@ struct ctl_table
> >> * the input value. No lower bound or upper bound checking will be
> >> * done if the corresponding minimum or maximum value isn't provided.
> >> *
> >> + * @CTL_FLAGS_SIGNED_RANGE: Set to indicate that the extra1 and extra2
> >> + * fields are pointers to minimum and maximum signed values of
> >> + * an allowable range.
> >> + *
> >> + * @CTL_FLAGS_UNSIGNED_RANGE: Set to indicate that the extra1 and extra2
> >> + * fields are pointers to minimum and maximum unsigned values of
> >> + * an allowable range.
> >> + *
> >> * At most 16 different flags are allowed.
> >> */
> >> enum ctl_table_flags {
> >> CTL_FLAGS_CLAMP_RANGE = BIT(0),
> >> - __CTL_FLAGS_MAX = BIT(1),
> >> + CTL_FLAGS_SIGNED_RANGE = BIT(1),
> >> + CTL_FLAGS_UNSIGNED_RANGE = BIT(2),
> >> + __CTL_FLAGS_MAX = BIT(3),
> >> };
> > You are adding new flags which the user can set, and yet these are used
> > internally.
> >
> > It would be best if internal flags are just that, not flags that a user can set.
> >
> > This patch should be folded with the first one.
> >
> > I'm starting to loose hope on these patch sets.
> >
> > Luis
>
> In order to do the correct min > max check, I need to know if the
> quantity is signed or not. Just looking at the proc_handler alone is not
> a reliable indicator if it is signed or unsigned.
>
> Yes, I can put the signed bit into the previous patch.
Do that and also remove the unused flags. It is confusing as a reviewer
why a flag was added and then you use another flag later. Seriously, please
take a bit more time to review your own patches prior to submission. Each
change should make sense and have use in the patch series.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 1/9] sysctl: Add flags to support min/max range clamping
From: Luis R. Rodriguez @ 2018-03-29 18:15 UTC (permalink / raw)
To: Waiman Long
Cc: Luis R. Rodriguez, Kees Cook, linux-kernel, linux-fsdevel,
linux-doc, Jonathan Corbet, Andrew Morton, Al Viro,
Matthew Wilcox, Eric W. Biederman
In-Reply-To: <35f54e8f-b80e-aa3b-b008-79ba7ca3bff2@redhat.com>
On Mon, Mar 19, 2018 at 11:39:19AM -0400, Waiman Long wrote:
> On 03/16/2018 09:10 PM, Luis R. Rodriguez wrote:
> > On Fri, Mar 16, 2018 at 02:13:42PM -0400, Waiman Long wrote:
> >> When the CTL_FLAGS_CLAMP_RANGE flag is set in the ctl_table
> >> entry, any update from the userspace will be clamped to the given
> >> range without error if either the proc_dointvec_minmax() or the
> >> proc_douintvec_minmax() handlers is used.
> > I don't get it. Why define a generic range flag when we can be mores specific and
> > you do that in your next patch. What's the point of this flag then?
> >
> > Luis
>
> I was thinking about using the signed/unsigned bits as just annotations
> for ranges for future extension. For the purpose of this patchset alone,
> I can merge the three bits into just two.
Only introduce flags which you will actually use in the same patch series.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 4/8] dt-bindings: Add doc for the Ingenic TCU drivers
From: Paul Cercueil @ 2018-03-29 15:59 UTC (permalink / raw)
To: Rob Herring
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Lee Jones,
Daniel Lezcano, Ralf Baechle, Jonathan Corbet, Mark Rutland,
James Hogan, Maarten ter Huurne, linux-clk, devicetree,
linux-kernel@vger.kernel.org, Linux-MIPS, linux-doc
In-Reply-To: <CAL_JsqLA8WbYZ49_oVb-uVgwUixVqDw2wtLy2tkyQwBM3ow5ew@mail.gmail.com>
Hi Rob,
Le mer. 28 mars 2018 à 18:28, Rob Herring <robh@kernel.org> a écrit :
> On Wed, Mar 28, 2018 at 10:33 AM, Paul Cercueil
> <paul@crapouillou.net> wrote:
>> Le 2018-03-27 16:46, Rob Herring a écrit :
>>>
>>> On Sun, Mar 18, 2018 at 12:28:57AM +0100, Paul Cercueil wrote:
>>>>
>>>> Add documentation about how to properly use the Ingenic TCU
>>>> (Timer/Counter Unit) drivers from devicetree.
>>>>
>>>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>>> ---
>>>> .../bindings/clock/ingenic,tcu-clocks.txt | 42
>>>> ++++++++++++++++
>>>> .../bindings/interrupt-controller/ingenic,tcu.txt | 39
>>>> +++++++++++++++
>>>> .../devicetree/bindings/mfd/ingenic,tcu.txt | 56
>>>> ++++++++++++++++++++++
>>>> .../devicetree/bindings/timer/ingenic,tcu.txt | 41
>>>> ++++++++++++++++
>>>> 4 files changed, 178 insertions(+)
>>>> create mode 100644
>>>> Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt
>>>> create mode 100644
>>>>
>>>> Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt
>>>> create mode 100644
>>>> Documentation/devicetree/bindings/mfd/ingenic,tcu.txt
>>>> create mode 100644
>>>> Documentation/devicetree/bindings/timer/ingenic,tcu.txt
>>>>
>>>> v4: New patch in this series. Corresponds to V2 patches 3-4-5
>>>> with
>>>> added content.
>>>> +/ {
>>>> + tcu: mfd@10002000 {
>>>> + compatible = "ingenic,tcu", "simple-mfd",
>>>> "syscon";
>>>> + reg = <0x10002000 0x1000>;
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>> + ranges = <0x0 0x10002000 0x1000>;
>>>> +
>>>> + tcu_timer: timer@10 {
>>>> + compatible = "ingenic,jz4740-tcu";
>>>> + reg = <0x10 0xff0>;
>>>> +
>>>> + clocks = <&tcu_clk 0>, <&tcu_clk 1>,
>>>> <&tcu_clk
>>>> 2>, <&tcu_clk 3>,
>>>> + <&tcu_clk 4>, <&tcu_clk
>>>> 5>,
>>>> <&tcu_clk 6>, <&tcu_clk 7>;
>>>> + clock-names = "timer0", "timer1",
>>>> "timer2",
>>>> "timer3",
>>>> + "timer4",
>>>> "timer5",
>>>> "timer6", "timer7";
>>>> +
>>>> + interrupt-parent = <&tcu_irq>;
>>>> + interrupts = <0 1 2 3 4 5 6 7>;
>>>
>>>
>>> Thinking about this some more... You simply have 8 timers (and no
>>> other
>>> functions?) with some internal clock and irq controls for each
>>> timer. I
>>> don't think it really makes sense to create separate clock and irq
>>> drivers in that case. That would be like creating clock drivers for
>>> every clock divider in timers, pwms, uarts, etc. Unless the clocks
>>> get
>>> exposed to other parts of the system, then there is no point.
>>
>>
>> I have 8 timers with some internal clock and IRQ controls, that can
>> be used
>> as PWM.
>
> Please include how you plan to do the PWM support too. I need a
> complete picture of the h/w, not piecemeal, evolving bindings.
Alright.
>> But the TCU also controls the IRQ of the OS Timer (which is
>> separate),
>> as well as masking of the clocks for the OS timer and the watchdog.
>
> The OS timer and watchdog are different blocks outside the TCU? This
> doesn't seem to be the case based on the register definitions.
Their register areas are mostly separate, although contiguous. On the
other
hand, the watchdog and OST can be started/stopped from a bit within a
TCU
register, so they're probably part of the same h/w block.
>> I thought having clean drivers for different frameworks working on
>> the same
>> regmap was cleaner than having one big-ass driver handling
>> everything.
>
> DT is not the only way to instantiate drivers and how one OS splits
> drivers should not define your DT binding. An MFD driver can create
> child devices and a single DT node can be a provider of multiple
> things. It is appropriate for an MFD to have child nodes primarily
> when the sub devices need their own resources defined as properties in
> DT or when the sub device is an IP block reused in multiple devices.
> Just to have a node per driver/provider is not what should drive the
> decision.
The idea is not to have necesarily one node per driver. I just wanted
to keep
it simple.
Regards,
-Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] docs: kernel-doc: fix parsing of arrays
From: Mauro Carvalho Chehab @ 2018-03-29 14:58 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, corbet, johannes
The logic with parses array has a bug that prevents it to
parse arrays like:
struct {
...
struct {
u64 msdu[IEEE80211_NUM_TIDS + 1];
...
...
Fix the parser to accept it.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
scripts/kernel-doc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index fee8952037b1..4356afda5afb 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1062,7 +1062,7 @@ sub dump_struct($$) {
# Handle bitmaps
$arg =~ s/:\s*\d+\s*//g;
# Handle arrays
- $arg =~ s/\[\S+\]//g;
+ $arg =~ s/\[.*\]//g;
# The type may have multiple words,
# and multiple IDs can be defined, like:
# const struct foo, *bar, foobar
--
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v4 7/8] clocksource: Add a new timer-ingenic driver
From: Paul Cercueil @ 2018-03-29 14:52 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Lee Jones,
Ralf Baechle, Rob Herring, Jonathan Corbet, Mark Rutland,
James Hogan, Maarten ter Huurne, linux-clk, devicetree,
linux-kernel, linux-mips, linux-doc
In-Reply-To: <af33e522-7f87-d62a-0a35-d56a403387b7@linaro.org>
Le mer. 28 mars 2018 à 18:25, Daniel Lezcano
<daniel.lezcano@linaro.org> a écrit :
> On 28/03/2018 17:15, Paul Cercueil wrote:
>> Le 2018-03-24 07:26, Daniel Lezcano a écrit :
>>> On 18/03/2018 00:29, Paul Cercueil wrote:
>>>> This driver will use the TCU (Timer Counter Unit) present on the
>>>> Ingenic
>>>> JZ47xx SoCs to provide the kernel with a clocksource and timers.
>>>
>>> Please provide a more detailed description about the timer.
>>
>> There's a doc file for that :)
>
> Usually, when there is a new driver I ask for a description in the
> changelog for reference.
>
>>> Where is the clocksource ?
>>
>> Right, there is no clocksource, just timers.
>>
>>> I don't see the point of using channel idx and pwm checking here.
>>>
>>> There is one clockevent, why create multiple channels ? Can't you
>>> stick
>>> to the usual init routine for a timer.
>>
>> So the idea is that we use all the TCU channels that won't be used
>> for PWM
>> as timers. Hence the PWM checking. Why is this bad?
>
> It is not bad but arguable. By checking the channels used by the pwm
> in
> the code, you introduce an adherence between two subsystems even if it
> is just related to the DT parsing part.
>
> As it is not needed to have more than one timer in the time framework
> (at least with the same characteristics), the pwm channels check is
> pointless. We can assume the author of the DT file is smart enough to
> prevent conflicts and define a pwm and a timer properly instead of
> adding more code complexity.
>
> In addition, simplifying the code will allow you to use the timer-of
> code and reduce very significantly the init function.
That's what I had in my V1 and V2, my DT node for the timer-ingenic
driver
had a "timers" property (e.g. "timers = <4 5>;") to select the channels
that
should be used as timers. Then Rob told me I shouldn't do that, and
instead
detect the channels that will be used for PWM.
>>>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>>> ---
>>>> drivers/clocksource/Kconfig | 8 ++
>>>> drivers/clocksource/Makefile | 1 +
>>>> drivers/clocksource/timer-ingenic.c | 278
>>>> ++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 287 insertions(+)
>>>> create mode 100644 drivers/clocksource/timer-ingenic.c
>>>>
>>>> v2: Use SPDX identifier for the license
>>>> v3: - Move documentation to its own patch
>>>> - Search the devicetree for PWM clients, and use all the TCU
>>>> channels that won't be used for PWM
>>>> v4: - Add documentation about why we search for PWM clients
>>>> - Verify that the PWM clients are for the TCU PWM driver
>>>>
>>>> diff --git a/drivers/clocksource/Kconfig
>>>> b/drivers/clocksource/Kconfig
>>>> index d2e5382821a4..481422145fb4 100644
>>>> --- a/drivers/clocksource/Kconfig
>>>> +++ b/drivers/clocksource/Kconfig
>>>> @@ -592,4 +592,12 @@ config CLKSRC_ST_LPC
>>>> Enable this option to use the Low Power controller timer
>>>> as clocksource.
>>>>
>>>> +config INGENIC_TIMER
>>>> + bool "Clocksource/timer using the TCU in Ingenic JZ SoCs"
>>>> + depends on MACH_INGENIC || COMPILE_TEST
>>>
>>> bool "Clocksource/timer using the TCU in Ingenic JZ SoCs" if
>>> COMPILE_TEST
>>>
>>> Remove the depends MACH_INGENIC.
>>
>> This driver is not useful on anything else than Ingenic SoCs, why
>> should I
>> remove MACH_INGENIC then?
>
> For COMPILE_TEST on x86.
Well that's a logical OR right here, so it will work...
>>>> + select CLKSRC_OF
>>>> + default y
>>>
>>> No default, Kconfig platform selects the timer.
>>
>> Alright.
> [ ... ]
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM
> SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: nested structs parsing
From: Mauro Carvalho Chehab @ 2018-03-29 14:48 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-doc, linux-wireless
In-Reply-To: <1522333593.24162.12.camel@sipsolutions.net>
Em Thu, 29 Mar 2018 16:26:33 +0200
Johannes Berg <johannes@sipsolutions.net> escreveu:
> Hi,
>
> > The original patchset for nested structs was supporting it only
> > when not inlined. This should be fixed on this patchset:
> >
> > https://lkml.org/lkml/2018/2/19/387
> >
> > Do you have those patches on your tree?
>
> No, looks like I don't have those yet. I'll wait for those then.
>
> > With regards to duplicated warnings, that use to happen if the same header
> > is included several times (with is a common pratice at the net subsystem).
>
> Yeah, doesn't really matter anyway. I think I have to, in a sense,
> because I'm getting lots of functions separately from the headers.
>
> > Could you please merge from docs-next and see if those problems
> > get solved?
>
> No, that doesn't seem to address it fully:
>
> net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
> net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
> net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
> net/mac80211/sta_info.h:586: warning: Function parameter or member 'msdu' not described in 'sta_info'
>
> You can reproduce this in
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
>
> (merging with docs-next) and running
>
> make SPHINXDIRS=driver-api/80211 htmldocs
No need to run it for checking the errors... you can run just:
./scripts/kernel-doc -none net/mac80211/sta_info.h
Applying the enclosed patch seems to work:
<patch>
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index f64eb86ca64b..d81cb6155e8d 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -477,6 +477,10 @@ struct ieee80211_sta_rx_stats {
* @tdls_chandef: a TDLS peer can have a wider chandef that is compatible to
* the BSS one.
* @tx_stats: TX statistics
+ * @tx_stats.packets: foo
+ * @tx_stats.last_rate: bar
+ * @tx_stats.bytes: foobar
+ * @tx_stats.msdu: foo
* @rx_stats: RX statistics
* @pcpu_rx_stats: per-CPU RX statistics, assigned only if the driver needs
* this (by advertising the USES_RSS hw flag)
</patch>
What's weird is that tx_stats.msdu field seems to be parsed wrong.
I'll take a look on it.
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: nested structs parsing
From: Johannes Berg @ 2018-03-29 14:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-doc, linux-wireless
In-Reply-To: <20180329112209.5c0e0320@vento.lan>
Hi,
> The original patchset for nested structs was supporting it only
> when not inlined. This should be fixed on this patchset:
>
> https://lkml.org/lkml/2018/2/19/387
>
> Do you have those patches on your tree?
No, looks like I don't have those yet. I'll wait for those then.
> With regards to duplicated warnings, that use to happen if the same header
> is included several times (with is a common pratice at the net subsystem).
Yeah, doesn't really matter anyway. I think I have to, in a sense,
because I'm getting lots of functions separately from the headers.
> Could you please merge from docs-next and see if those problems
> get solved?
No, that doesn't seem to address it fully:
net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
net/mac80211/sta_info.h:586: warning: Function parameter or member 'msdu' not described in 'sta_info'
You can reproduce this in
git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
(merging with docs-next) and running
make SPHINXDIRS=driver-api/80211 htmldocs
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: nested structs parsing
From: Mauro Carvalho Chehab @ 2018-03-29 14:22 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-doc, linux-wireless
In-Reply-To: <1522316827.5932.11.camel@sipsolutions.net>
Em Thu, 29 Mar 2018 11:47:07 +0200
Johannes Berg <johannes@sipsolutions.net> escreveu:
> On Thu, 2018-03-29 at 11:46 +0200, Johannes Berg wrote:
> > Hi,
> >
> > For a while I haven't looked at my documentation for 802.11, and now I
> > noticed I'm getting warnings due to the nested parsing.
> >
> > However, something seems to be wrong? I have, for example, this (in
> > net/mac80211/sta_info.h)
> >
> > struct sta_info {
> > ...
> > struct {
> > u64 packets[IEEE80211_NUM_ACS];
> > u64 bytes[IEEE80211_NUM_ACS];
> > struct ieee80211_tx_rate last_rate;
> > u64 msdu[IEEE80211_NUM_TIDS + 1];
> > } tx_stats;
> > };
> >
> > but I'm getting the following warnings now, with only "@tx_stats" being
> > described in the documentation:
> >
> > net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
> > net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
> > net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
> > net/mac80211/sta_info.h:590: warning: Function parameter or member 'msdu' not described in 'sta_info'
> >
> > I can understand the first three of those, but not the last one? Why is
> > the last one not qualified?
> >
> > If I change it to this:
> >
> > struct {
> > u64 packets[IEEE80211_NUM_ACS];
> > u64 bytes[IEEE80211_NUM_ACS];
> > /**
> > * @last_rate: last TX rate
> > */
> > struct ieee80211_tx_rate last_rate;
> > /**
> > * @msdu: # of MSDUs per TID
> > */
> > u64 msdu[IEEE80211_NUM_TIDS + 1];
> > } tx_stats;
> >
> > I still get a warning on "tx_stats.last_rate", but not on "msdu", which
> > is sort of obvious from the warning text, but also rather unexpected.
> >
> > Normally I'd say that the "msdu" warning is originally wrong
> >
> > However, I'd also argue that if I'm using inline declarations, I
> > shouldn't have to write it like this:
> >
> > struct {
> > u64 packets[IEEE80211_NUM_ACS];
> > u64 bytes[IEEE80211_NUM_ACS];
> > /**
> > * @tx_stats.last_rate: last TX rate
> > */
> > struct ieee80211_tx_rate last_rate;
> > ...
> > } tx_stats;
>
> Whoops, sent a fraction of a second too early - this actually causes a
> warning too (no idea why four times):
>
> net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format: * @tx_stats.last_rate: last TX rate
> net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format: * @tx_stats.last_rate: last TX rate
> net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format: * @tx_stats.last_rate: last TX rate
> net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format: * @tx_stats.last_rate: last TX rate
The original patchset for nested structs was supporting it only
when not inlined. This should be fixed on this patchset:
https://lkml.org/lkml/2018/2/19/387
Do you have those patches on your tree?
With regards to duplicated warnings, that use to happen if the same header
is included several times (with is a common pratice at the net subsystem).
I also submitted some patches to linux-next addressing it.
Could you please merge from docs-next and see if those problems
get solved?
It is located at:
git://git.lwn.net/linux.git docs-next
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Documentation/usb: Fix dead links and convert others to https
From: Johan Hovold @ 2018-03-29 10:02 UTC (permalink / raw)
To: Sanjeev Gupta; +Cc: corbet, gregkh, linux-doc, linux-kernel
In-Reply-To: <20180328160752.17840-1-ghane0@gmail.com>
On Thu, Mar 29, 2018 at 12:07:52AM +0800, Sanjeev Gupta wrote:
> All links checked, for those dead, I have replaced with copies on
> archive.org. For some, https is not supported, http has been
> kept.
>
> The git log says this was last done by Justin P. Mattock in 2010.
> Hi, Justin!
Not sure this belongs in the commit message (and did you forget to CC
Justin?).
> Signed-off-by: Sanjeev Gupta <ghane0@gmail.com>
> ---
> Documentation/usb/usb-serial.txt | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
> For any questions or problems with this driver, please contact Hugh
> Blemings at hugh@misc.nu
> @@ -436,7 +437,8 @@ Winchiphead CH341 Driver
> also implements an IEEE 1284 parallel port, I2C and SPI, but that is not
> supported by the driver. The protocol was analyzed from the behaviour
> of the Windows driver, no datasheet is available at present.
> - The manufacturer's website: http://www.winchiphead.com/.
> + The manufacturer's website:
> + https://web.archive.org/web/20170911133726/http://winchiphead.com/ .
AFAICT this link is still working.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: nested structs parsing
From: Johannes Berg @ 2018-03-29 9:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-doc, linux-wireless
In-Reply-To: <1522316775.5932.10.camel@sipsolutions.net>
On Thu, 2018-03-29 at 11:46 +0200, Johannes Berg wrote:
> Hi,
>
> For a while I haven't looked at my documentation for 802.11, and now I
> noticed I'm getting warnings due to the nested parsing.
>
> However, something seems to be wrong? I have, for example, this (in
> net/mac80211/sta_info.h)
>
> struct sta_info {
> ...
> struct {
> u64 packets[IEEE80211_NUM_ACS];
> u64 bytes[IEEE80211_NUM_ACS];
> struct ieee80211_tx_rate last_rate;
> u64 msdu[IEEE80211_NUM_TIDS + 1];
> } tx_stats;
> };
>
> but I'm getting the following warnings now, with only "@tx_stats" being
> described in the documentation:
>
> net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
> net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
> net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
> net/mac80211/sta_info.h:590: warning: Function parameter or member 'msdu' not described in 'sta_info'
>
> I can understand the first three of those, but not the last one? Why is
> the last one not qualified?
>
> If I change it to this:
>
> struct {
> u64 packets[IEEE80211_NUM_ACS];
> u64 bytes[IEEE80211_NUM_ACS];
> /**
> * @last_rate: last TX rate
> */
> struct ieee80211_tx_rate last_rate;
> /**
> * @msdu: # of MSDUs per TID
> */
> u64 msdu[IEEE80211_NUM_TIDS + 1];
> } tx_stats;
>
> I still get a warning on "tx_stats.last_rate", but not on "msdu", which
> is sort of obvious from the warning text, but also rather unexpected.
>
> Normally I'd say that the "msdu" warning is originally wrong
>
> However, I'd also argue that if I'm using inline declarations, I
> shouldn't have to write it like this:
>
> struct {
> u64 packets[IEEE80211_NUM_ACS];
> u64 bytes[IEEE80211_NUM_ACS];
> /**
> * @tx_stats.last_rate: last TX rate
> */
> struct ieee80211_tx_rate last_rate;
> ...
> } tx_stats;
Whoops, sent a fraction of a second too early - this actually causes a
warning too (no idea why four times):
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format: * @tx_stats.last_rate: last TX rate
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format: * @tx_stats.last_rate: last TX rate
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format: * @tx_stats.last_rate: last TX rate
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format: * @tx_stats.last_rate: last TX rate
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* nested structs parsing
From: Johannes Berg @ 2018-03-29 9:46 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-doc, linux-wireless
Hi,
For a while I haven't looked at my documentation for 802.11, and now I
noticed I'm getting warnings due to the nested parsing.
However, something seems to be wrong? I have, for example, this (in
net/mac80211/sta_info.h)
struct sta_info {
...
struct {
u64 packets[IEEE80211_NUM_ACS];
u64 bytes[IEEE80211_NUM_ACS];
struct ieee80211_tx_rate last_rate;
u64 msdu[IEEE80211_NUM_TIDS + 1];
} tx_stats;
};
but I'm getting the following warnings now, with only "@tx_stats" being
described in the documentation:
net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
net/mac80211/sta_info.h:590: warning: Function parameter or member 'msdu' not described in 'sta_info'
I can understand the first three of those, but not the last one? Why is
the last one not qualified?
If I change it to this:
struct {
u64 packets[IEEE80211_NUM_ACS];
u64 bytes[IEEE80211_NUM_ACS];
/**
* @last_rate: last TX rate
*/
struct ieee80211_tx_rate last_rate;
/**
* @msdu: # of MSDUs per TID
*/
u64 msdu[IEEE80211_NUM_TIDS + 1];
} tx_stats;
I still get a warning on "tx_stats.last_rate", but not on "msdu", which
is sort of obvious from the warning text, but also rather unexpected.
Normally I'd say that the "msdu" warning is originally wrong
However, I'd also argue that if I'm using inline declarations, I
shouldn't have to write it like this:
struct {
u64 packets[IEEE80211_NUM_ACS];
u64 bytes[IEEE80211_NUM_ACS];
/**
* @tx_stats.last_rate: last TX rate
*/
struct ieee80211_tx_rate last_rate;
...
} tx_stats;
since the comment is contained in the scope of tx_stats already, but
that seems to be what I'd have to do today?
At least fixing one of these to make it consistent would be good :-)
Thanks,
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] perf: riscv: preliminary RISC-V support
From: Alan Kao @ 2018-03-29 2:30 UTC (permalink / raw)
To: Alex Solomatnikov
Cc: Palmer Dabbelt, Albert Ou, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Jonathan Corbet, linux-riscv, linux-doc,
linux-kernel, Nick Hu, Greentime Hu
In-Reply-To: <CAJ2AOiNQuMJoPZPxy2CDFD0vnxk8E+N-8xiir2nPYKjWJKxshQ@mail.gmail.com>
Hi Alex,
I'm appreciated for your reply and tests.
On Wed, Mar 28, 2018 at 03:58:41PM -0700, Alex Solomatnikov wrote:
> Did you test this code?
I did test this patch on QEMU's virt model with multi-hart, which is the only
RISC-V machine I have for now. But as I mentioned in
https://github.com/riscv/riscv-qemu/pull/115 , the hardware counter support
in QEMU is not fully conformed to the 1.10 Priv-Spec, so I had to slightly
tweak the code to make reading work.
Specifically, the read to cycle and instret in QEMU looks like this:
...
case CSR_INSTRET:
case CSR_CYCLE:
// if (ctr_ok) {
return cpu_get_host_ticks();
// }
break;
...
and the two lines of comment was the tweak.
On such environment, I did not get anything unexpected. No matter which of them
is requested, QEMU returns the host's tick.
>
> I got funny numbers when I tried to run it on HiFive Unleashed:
>
> perf stat mem-latency
> ...
>
> Performance counter stats for 'mem-latency':
>
> 157.907000 task-clock (msec) # 0.940 CPUs utilized
>
> 1 context-switches # 0.006 K/sec
>
> 1 cpu-migrations # 0.006 K/sec
>
> 4102 page-faults # 0.026 M/sec
>
> 157923752 cycles # 1.000 GHz
>
> 9223372034948899840 instructions # 58403957087.78 insn
> per cycle
> <not supported> branches
>
> <not supported> branch-misses
>
>
> 0.168046000 seconds time elapsed
>
>
> Tracing read_counter(), I see this:
>
> Jan 1 00:41:50 buildroot user.info kernel: [ 2510.058809] CPU 3:
> read_counter idx=0 val=2528358954912
> Jan 1 00:41:50 buildroot user.info kernel: [ 2510.063339] CPU 3:
> read_counter idx=1 val=53892244920
> Jan 1 00:41:50 buildroot user.info kernel: [ 2510.118160] CPU 3:
> read_counter idx=0 val=2528418303035
> Jan 1 00:41:50 buildroot user.info kernel: [ 2510.122694] CPU 3:
> read_counter idx=1 val=53906699665
> Jan 1 00:41:50 buildroot user.info kernel: [ 2510.216736] CPU 1:
> read_counter idx=0 val=2528516878664
> Jan 1 00:41:50 buildroot user.info kernel: [ 2510.221270] CPU 1:
> read_counter idx=1 val=51986369142
>
> It looks like the counter values from different cores are subtracted and
> wraparound occurs.
>
Thanks for the hint. It makes sense. 9223372034948899840 is 7fffffff8e66a400,
which should be a wraparound with the mask I set (63-bit) in the code.
I will try this direction. Ideally, we can solve it by explicitly syncing the
hwc->prev_count when a cpu migration event happens.
>
> Also, core IDs and socket IDs are wrong in perf report:
>
As Palmer has replied to this, I have no comment here.
> perf report --header -I
> Error:
> The perf.data file has no samples!
> # ========
> # captured on: Thu Jan 1 02:52:07 1970
> # hostname : buildroot
> # os release : 4.15.0-00045-g0d7c030-dirty
> # perf version : 4.15.0
> # arch : riscv64
> # nrcpus online : 4
> # nrcpus avail : 5
> # total memory : 8188340 kB
> # cmdline : /usr/bin/perf record -F 1000 lat_mem_rd -P 1 -W 1 -N 1 -t 10
> # event : name = cycles:ppp, , size = 112, { sample_period, sample_freq } =
> 1000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap =
> 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, precise_ip = 3,
> sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1
> # sibling cores : 1
> # sibling cores : 2
> # sibling cores : 3
> # sibling cores : 4
> # sibling threads : 1
> # sibling threads : 2
> # sibling threads : 3
> # sibling threads : 4
> # CPU 0: Core ID -1, Socket ID -1
> # CPU 1: Core ID 0, Socket ID -1
> # CPU 2: Core ID 0, Socket ID -1
> # CPU 3: Core ID 0, Socket ID -1
> # CPU 4: Core ID 0, Socket ID -1
> # pmu mappings: cpu = 4, software = 1
> # CPU cache info:
> # L1 Instruction 32K [1]
> # L1 Data 32K [1]
> # L1 Instruction 32K [2]
> # L1 Data 32K [2]
> # L1 Instruction 32K [3]
> # L1 Data 32K [3]
> # missing features: TRACING_DATA BUILD_ID CPUDESC CPUID NUMA_TOPOLOGY
> BRANCH_STACK GROUP_DESC AUXTRACE STAT
> # ========
>
>
> Alex
>
Many thanks,
Alan
> On Mon, Mar 26, 2018 at 12:57 AM, Alan Kao <alankao@andestech.com> wrote:
>
> > This patch provide a basic PMU, riscv_base_pmu, which supports two
> > general hardware event, instructions and cycles. Furthermore, this
> > PMU serves as a reference implementation to ease the portings in
> > the future.
> >
> > riscv_base_pmu should be able to run on any RISC-V machine that
> > conforms to the Priv-Spec. Note that the latest qemu model hasn't
> > fully support a proper behavior of Priv-Spec 1.10 yet, but work
> > around should be easy with very small fixes. Please check
> > https://github.com/riscv/riscv-qemu/pull/115 for future updates.
> >
> > Cc: Nick Hu <nickhu@andestech.com>
> > Cc: Greentime Hu <greentime@andestech.com>
> > Signed-off-by: Alan Kao <alankao@andestech.com>
> > ---
> > arch/riscv/Kconfig | 12 +
> > arch/riscv/include/asm/perf_event.h | 76 +++++-
> > arch/riscv/kernel/Makefile | 1 +
> > arch/riscv/kernel/perf_event.c | 469 ++++++++++++++++++++++++++++++
> > ++++++
> > 4 files changed, 554 insertions(+), 4 deletions(-)
> > create mode 100644 arch/riscv/kernel/perf_event.c
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 310b9a5d6737..dd4aecfb5265 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -195,6 +195,18 @@ config RISCV_ISA_C
> > config RISCV_ISA_A
> > def_bool y
> >
> > +menu "PMU type"
> > + depends on PERF_EVENTS
> > +
> > +config RISCV_BASE_PMU
> > + bool "Base Performance Monitoring Unit"
> > + def_bool y
> > + help
> > + A base PMU that serves as a reference implementation and has
> > limited
> > + feature of perf.
> > +
> > +endmenu
> > +
> > endmenu
> >
> > menu "Kernel type"
> > diff --git a/arch/riscv/include/asm/perf_event.h
> > b/arch/riscv/include/asm/perf_event.h
> > index e13d2ff29e83..98e2efb02d25 100644
> > --- a/arch/riscv/include/asm/perf_event.h
> > +++ b/arch/riscv/include/asm/perf_event.h
> > @@ -1,13 +1,81 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > /*
> > * Copyright (C) 2018 SiFive
> > + * Copyright (C) 2018 Andes Technology Corporation
> > *
> > - * This program is free software; you can redistribute it and/or
> > - * modify it under the terms of the GNU General Public Licence
> > - * as published by the Free Software Foundation; either version
> > - * 2 of the Licence, or (at your option) any later version.
> > */
> >
> > #ifndef _ASM_RISCV_PERF_EVENT_H
> > #define _ASM_RISCV_PERF_EVENT_H
> >
> > +#include <linux/perf_event.h>
> > +#include <linux/ptrace.h>
> > +
> > +#define RISCV_BASE_COUNTERS 2
> > +
> > +/*
> > + * The RISCV_MAX_COUNTERS parameter should be specified.
> > + */
> > +
> > +#ifdef CONFIG_RISCV_BASE_PMU
> > +#define RISCV_MAX_COUNTERS 2
> > +#endif
> > +
> > +#ifndef RISCV_MAX_COUNTERS
> > +#error "Please provide a valid RISCV_MAX_COUNTERS for the PMU."
> > +#endif
> > +
> > +/*
> > + * These are the indexes of bits in counteren register *minus* 1,
> > + * except for cycle. It would be coherent if it can directly mapped
> > + * to counteren bit definition, but there is a *time* register at
> > + * counteren[1]. Per-cpu structure is scarce resource here.
> > + *
> > + * According to the spec, an implementation can support counter up to
> > + * mhpmcounter31, but many high-end processors has at most 6 general
> > + * PMCs, we give the definition to MHPMCOUNTER8 here.
> > + */
> > +#define RISCV_PMU_CYCLE 0
> > +#define RISCV_PMU_INSTRET 1
> > +#define RISCV_PMU_MHPMCOUNTER3 2
> > +#define RISCV_PMU_MHPMCOUNTER4 3
> > +#define RISCV_PMU_MHPMCOUNTER5 4
> > +#define RISCV_PMU_MHPMCOUNTER6 5
> > +#define RISCV_PMU_MHPMCOUNTER7 6
> > +#define RISCV_PMU_MHPMCOUNTER8 7
> > +
> > +#define RISCV_OP_UNSUPP (-EOPNOTSUPP)
> > +
> > +struct cpu_hw_events {
> > + /* # currently enabled events*/
> > + int n_events;
> > + /* currently enabled events */
> > + struct perf_event *events[RISCV_MAX_COUNTERS];
> > + /* vendor-defined PMU data */
> > + void *platform;
> > +};
> > +
> > +struct riscv_pmu {
> > + struct pmu *pmu;
> > +
> > + /* generic hw/cache events table */
> > + const int *hw_events;
> > + const int (*cache_events)[PERF_COUNT_HW_CACHE_MAX]
> > + [PERF_COUNT_HW_CACHE_OP_MAX]
> > + [PERF_COUNT_HW_CACHE_RESULT_MAX];
> > + /* method used to map hw/cache events */
> > + int (*map_hw_event)(u64 config);
> > + int (*map_cache_event)(u64 config);
> > +
> > + /* max generic hw events in map */
> > + int max_events;
> > + /* number total counters, 2(base) + x(general) */
> > + int num_counters;
> > + /* the width of the counter */
> > + int counter_width;
> > +
> > + /* vendor-defined PMU features */
> > + void *platform;
> > +};
> > +
> > #endif /* _ASM_RISCV_PERF_EVENT_H */
> > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> > index 196f62ffc428..849c38d9105f 100644
> > --- a/arch/riscv/kernel/Makefile
> > +++ b/arch/riscv/kernel/Makefile
> > @@ -36,5 +36,6 @@ obj-$(CONFIG_SMP) += smp.o
> > obj-$(CONFIG_MODULES) += module.o
> > obj-$(CONFIG_FUNCTION_TRACER) += mcount.o
> > obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
> > +obj-$(CONFIG_PERF_EVENTS) += perf_event.o
> >
> > clean:
> > diff --git a/arch/riscv/kernel/perf_event.c b/arch/riscv/kernel/perf_
> > event.c
> > new file mode 100644
> > index 000000000000..b78cb486683b
> > --- /dev/null
> > +++ b/arch/riscv/kernel/perf_event.c
> > @@ -0,0 +1,469 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2008 Thomas Gleixner <tglx@linutronix.de>
> > + * Copyright (C) 2008-2009 Red Hat, Inc., Ingo Molnar
> > + * Copyright (C) 2009 Jaswinder Singh Rajput
> > + * Copyright (C) 2009 Advanced Micro Devices, Inc., Robert Richter
> > + * Copyright (C) 2008-2009 Red Hat, Inc., Peter Zijlstra
> > + * Copyright (C) 2009 Intel Corporation, <markus.t.metzger@intel.com>
> > + * Copyright (C) 2009 Google, Inc., Stephane Eranian
> > + * Copyright 2014 Tilera Corporation. All Rights Reserved.
> > + * Copyright (C) 2018 Andes Technology Corporation
> > + *
> > + * Perf_events support for RISC-V platforms.
> > + *
> > + * Since the spec. (as of now, Priv-Spec 1.10) does not provide enough
> > + * functionality for perf event to fully work, this file provides
> > + * the very basic framework only.
> > + *
> > + * For platform portings, please check Documentations/riscv/pmu.txt.
> > + *
> > + * The Copyright line includes x86 and tile ones.
> > + */
> > +
> > +#include <linux/kprobes.h>
> > +#include <linux/kernel.h>
> > +#include <linux/kdebug.h>
> > +#include <linux/mutex.h>
> > +#include <linux/bitmap.h>
> > +#include <linux/irq.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/perf_event.h>
> > +#include <linux/atomic.h>
> > +#include <asm/perf_event.h>
> > +
> > +static const struct riscv_pmu *riscv_pmu __read_mostly;
> > +static DEFINE_PER_CPU(struct cpu_hw_events, cpu_hw_events);
> > +
> > +/*
> > + * Hardware & cache maps and their methods
> > + */
> > +
> > +static const int riscv_hw_event_map[] = {
> > + [PERF_COUNT_HW_CPU_CYCLES] = RISCV_PMU_CYCLE,
> > + [PERF_COUNT_HW_INSTRUCTIONS] = RISCV_PMU_INSTRET,
> > + [PERF_COUNT_HW_CACHE_REFERENCES] = RISCV_OP_UNSUPP,
> > + [PERF_COUNT_HW_CACHE_MISSES] = RISCV_OP_UNSUPP,
> > + [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = RISCV_OP_UNSUPP,
> > + [PERF_COUNT_HW_BRANCH_MISSES] = RISCV_OP_UNSUPP,
> > + [PERF_COUNT_HW_BUS_CYCLES] = RISCV_OP_UNSUPP,
> > +};
> > +
> > +#define C(x) PERF_COUNT_HW_CACHE_##x
> > +static const int riscv_cache_event_map[PERF_COUNT_HW_CACHE_MAX]
> > +[PERF_COUNT_HW_CACHE_OP_MAX]
> > +[PERF_COUNT_HW_CACHE_RESULT_MAX] = {
> > + [C(L1D)] = {
> > + [C(OP_READ)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_WRITE)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_PREFETCH)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + },
> > + [C(L1I)] = {
> > + [C(OP_READ)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_WRITE)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_PREFETCH)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + },
> > + [C(LL)] = {
> > + [C(OP_READ)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_WRITE)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_PREFETCH)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + },
> > + [C(DTLB)] = {
> > + [C(OP_READ)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_WRITE)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_PREFETCH)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + },
> > + [C(ITLB)] = {
> > + [C(OP_READ)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_WRITE)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_PREFETCH)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + },
> > + [C(BPU)] = {
> > + [C(OP_READ)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_WRITE)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + [C(OP_PREFETCH)] = {
> > + [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
> > + [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
> > + },
> > + },
> > +};
> > +
> > +static int riscv_map_hw_event(u64 config)
> > +{
> > + if (config >= riscv_pmu->max_events)
> > + return -EINVAL;
> > +
> > + return riscv_pmu->hw_events[config];
> > +}
> > +
> > +int riscv_map_cache_decode(u64 config, unsigned int *type,
> > + unsigned int *op, unsigned int *result)
> > +{
> > + return -ENOENT;
> > +}
> > +
> > +static int riscv_map_cache_event(u64 config)
> > +{
> > + unsigned int type, op, result;
> > + int err = -ENOENT;
> > + int code;
> > +
> > + err = riscv_map_cache_decode(config, &type, &op, &result);
> > + if (!riscv_pmu->cache_events || err)
> > + return err;
> > +
> > + if (type >= PERF_COUNT_HW_CACHE_MAX ||
> > + op >= PERF_COUNT_HW_CACHE_OP_MAX ||
> > + result >= PERF_COUNT_HW_CACHE_RESULT_MAX)
> > + return -EINVAL;
> > +
> > + code = (*riscv_pmu->cache_events)[type][op][result];
> > + if (code == RISCV_OP_UNSUPP)
> > + return -EINVAL;
> > +
> > + return code;
> > +}
> > +
> > +/*
> > + * Low-level functions: reading/writing counters
> > + */
> > +
> > +static inline u64 read_counter(int idx)
> > +{
> > + u64 val = 0;
> > +
> > + switch (idx) {
> > + case RISCV_PMU_CYCLE:
> > + val = csr_read(cycle);
> > + break;
> > + case RISCV_PMU_INSTRET:
> > + val = csr_read(instret);
> > + break;
> > + default:
> > + WARN_ON_ONCE(idx < 0 || idx > RISCV_MAX_COUNTERS);
> > + return -EINVAL;
> > + }
> > +
> > + return val;
> > +}
> > +
> > +static inline void write_counter(int idx, u64 value)
> > +{
> > + /* currently not supported */
> > +}
> > +
> > +/*
> > + * pmu->read: read and update the counter
> > + *
> > + * Other architectures' implementation often have a xxx_perf_event_update
> > + * routine, which can return counter values when called in the IRQ, but
> > + * return void when being called by the pmu->read method.
> > + */
> > +static void riscv_pmu_read(struct perf_event *event)
> > +{
> > + struct hw_perf_event *hwc = &event->hw;
> > + u64 prev_raw_count, new_raw_count;
> > + u64 oldval;
> > + int idx = hwc->idx;
> > + u64 delta;
> > +
> > + do {
> > + prev_raw_count = local64_read(&hwc->prev_count);
> > + new_raw_count = read_counter(idx);
> > +
> > + oldval = local64_cmpxchg(&hwc->prev_count, prev_raw_count,
> > + new_raw_count);
> > + } while (oldval != prev_raw_count);
> > +
> > + /*
> > + * delta is the value to update the counter we maintain in the
> > kernel.
> > + */
> > + delta = (new_raw_count - prev_raw_count) &
> > + ((1ULL << riscv_pmu->counter_width) - 1);
> > + local64_add(delta, &event->count);
> > + /*
> > + * Something like local64_sub(delta, &hwc->period_left) here is
> > + * needed if there is an interrupt for perf.
> > + */
> > +}
> > +
> > +/*
> > + * State transition functions:
> > + *
> > + * stop()/start() & add()/del()
> > + */
> > +
> > +/*
> > + * pmu->stop: stop the counter
> > + */
> > +static void riscv_pmu_stop(struct perf_event *event, int flags)
> > +{
> > + struct hw_perf_event *hwc = &event->hw;
> > +
> > + WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> > + hwc->state |= PERF_HES_STOPPED;
> > +
> > + if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE))
> > {
> > + riscv_pmu_read(event);
> > + hwc->state |= PERF_HES_UPTODATE;
> > + }
> > +}
> > +
> > +/*
> > + * pmu->start: start the event.
> > + */
> > +static void riscv_pmu_start(struct perf_event *event, int flags)
> > +{
> > + struct hw_perf_event *hwc = &event->hw;
> > +
> > + if (WARN_ON_ONCE(!(event->hw.state & PERF_HES_STOPPED)))
> > + return;
> > +
> > + if (flags & PERF_EF_RELOAD) {
> > + WARN_ON_ONCE(!(event->hw.state & PERF_HES_UPTODATE));
> > +
> > + /*
> > + * Set the counter to the period to the next interrupt
> > here,
> > + * if you have any.
> > + */
> > + }
> > +
> > + hwc->state = 0;
> > + perf_event_update_userpage(event);
> > +
> > + /*
> > + * Since we cannot write to counters, this serves as an
> > initialization
> > + * to the delta-mechanism in pmu->read(); otherwise, the delta
> > would be
> > + * wrong when pmu->read is called for the first time.
> > + */
> > + if (local64_read(&hwc->prev_count) == 0)
> > + local64_set(&hwc->prev_count, read_counter(hwc->idx));
> > +}
> > +
> > +/*
> > + * pmu->add: add the event to PMU.
> > + */
> > +static int riscv_pmu_add(struct perf_event *event, int flags)
> > +{
> > + struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> > + struct hw_perf_event *hwc = &event->hw;
> > +
> > + if (cpuc->n_events == riscv_pmu->num_counters)
> > + return -ENOSPC;
> > +
> > + /*
> > + * We don't have general conunters, so no binding-event-to-counter
> > + * process here.
> > + *
> > + * Indexing using hwc->config generally not works, since config may
> > + * contain extra information, but here the only info we have in
> > + * hwc->config is the event index.
> > + */
> > + hwc->idx = hwc->config;
> > + cpuc->events[hwc->idx] = event;
> > + cpuc->n_events++;
> > +
> > + hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
> > +
> > + if (flags & PERF_EF_START)
> > + riscv_pmu_start(event, PERF_EF_RELOAD);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * pmu->del: delete the event from PMU.
> > + */
> > +static void riscv_pmu_del(struct perf_event *event, int flags)
> > +{
> > + struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> > + struct hw_perf_event *hwc = &event->hw;
> > +
> > + cpuc->events[hwc->idx] = NULL;
> > + cpuc->n_events--;
> > + riscv_pmu_stop(event, PERF_EF_UPDATE);
> > + perf_event_update_userpage(event);
> > +}
> > +
> > +/*
> > + * Interrupt
> > + */
> > +
> > +static DEFINE_MUTEX(pmc_reserve_mutex);
> > +typedef void (*perf_irq_t)(void *riscv_perf_irq);
> > +perf_irq_t perf_irq;
> > +
> > +void riscv_pmu_handle_irq(void *riscv_perf_irq)
> > +{
> > +}
> > +
> > +static perf_irq_t reserve_pmc_hardware(void)
> > +{
> > + perf_irq_t old;
> > +
> > + mutex_lock(&pmc_reserve_mutex);
> > + old = perf_irq;
> > + perf_irq = &riscv_pmu_handle_irq;
> > + mutex_unlock(&pmc_reserve_mutex);
> > +
> > + return old;
> > +}
> > +
> > +void release_pmc_hardware(void)
> > +{
> > + mutex_lock(&pmc_reserve_mutex);
> > + perf_irq = NULL;
> > + mutex_unlock(&pmc_reserve_mutex);
> > +}
> > +
> > +/*
> > + * Event Initialization
> > + */
> > +
> > +static atomic_t riscv_active_events;
> > +
> > +static void riscv_event_destroy(struct perf_event *event)
> > +{
> > + if (atomic_dec_return(&riscv_active_events) == 0)
> > + release_pmc_hardware();
> > +}
> > +
> > +static int riscv_event_init(struct perf_event *event)
> > +{
> > + struct perf_event_attr *attr = &event->attr;
> > + struct hw_perf_event *hwc = &event->hw;
> > + perf_irq_t old_irq_handler = NULL;
> > + int code;
> > +
> > + if (atomic_inc_return(&riscv_active_events) == 1)
> > + old_irq_handler = reserve_pmc_hardware();
> > +
> > + if (old_irq_handler) {
> > + pr_warn("PMC hardware busy (reserved by oprofile)\n");
> > + atomic_dec(&riscv_active_events);
> > + return -EBUSY;
> > + }
> > +
> > + switch (event->attr.type) {
> > + case PERF_TYPE_HARDWARE:
> > + code = riscv_pmu->map_hw_event(attr->config);
> > + break;
> > + case PERF_TYPE_HW_CACHE:
> > + code = riscv_pmu->map_cache_event(attr->config);
> > + break;
> > + case PERF_TYPE_RAW:
> > + return -EOPNOTSUPP;
> > + default:
> > + return -ENOENT;
> > + }
> > +
> > + event->destroy = riscv_event_destroy;
> > + if (code < 0) {
> > + event->destroy(event);
> > + return code;
> > + }
> > +
> > + /*
> > + * idx is set to -1 because the index of a general event should
> > not be
> > + * decided until binding to some counter in pmu->add().
> > + *
> > + * But since we don't have such support, later in pmu->add(), we
> > just
> > + * use hwc->config as the index instead.
> > + */
> > + hwc->config = code;
> > + hwc->idx = -1;
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Initialization
> > + */
> > +
> > +static struct pmu min_pmu = {
> > + .name = "riscv-base",
> > + .event_init = riscv_event_init,
> > + .add = riscv_pmu_add,
> > + .del = riscv_pmu_del,
> > + .start = riscv_pmu_start,
> > + .stop = riscv_pmu_stop,
> > + .read = riscv_pmu_read,
> > +};
> > +
> > +static const struct riscv_pmu riscv_base_pmu = {
> > + .pmu = &min_pmu,
> > + .max_events = ARRAY_SIZE(riscv_hw_event_map),
> > + .map_hw_event = riscv_map_hw_event,
> > + .hw_events = riscv_hw_event_map,
> > + .map_cache_event = riscv_map_cache_event,
> > + .cache_events = &riscv_cache_event_map,
> > + .counter_width = 63,
> > + .num_counters = RISCV_BASE_COUNTERS + 0,
> > +};
> > +
> > +struct pmu * __weak __init riscv_init_platform_pmu(void)
> > +{
> > + riscv_pmu = &riscv_base_pmu;
> > + return riscv_pmu->pmu;
> > +}
> > +
> > +int __init init_hw_perf_events(void)
> > +{
> > + struct pmu *pmu = riscv_init_platform_pmu();
> > +
> > + perf_irq = NULL;
> > + perf_pmu_register(pmu, "cpu", PERF_TYPE_RAW);
> > + return 0;
> > +}
> > +arch_initcall(init_hw_perf_events);
> > --
> > 2.16.2
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] perf: riscv: preliminary RISC-V support
From: Palmer Dabbelt @ 2018-03-29 0:41 UTC (permalink / raw)
To: sols
Cc: alankao, albert, peterz, mingo, acme, alexander.shishkin, jolsa,
namhyung, corbet, linux-riscv, linux-doc, linux-kernel, nickhu,
greentime
In-Reply-To: <CAJ2AOiNQiTpJ9cBCb_fXVCyVNj9U-Xk3tUG=9toAC7HpQTRQvA@mail.gmail.com>
On Wed, 28 Mar 2018 15:31:30 PDT (-0700), sols@sifive.com wrote:
> Also, core IDs and socket IDs are wrong in perf report:
It looks like for this we need the stuff in topology.h. I've cobbled something
together quickly for Alex to use for now to see if that fixes other problems,
I'll submit a sane version when we can.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v12 07/22] selftests/vm: fixed bugs in pkey_disable_clear()
From: Thiago Jung Bauermann @ 2018-03-28 23:51 UTC (permalink / raw)
To: Dave Hansen
Cc: Ram Pai, shuahkh, linux-kselftest, mpe, linuxppc-dev, linux-mm,
x86, linux-arch, linux-doc, linux-kernel, mingo, akpm, benh,
paulus, khandual, aneesh.kumar, bsingharora, hbabu, mhocko,
ebiederm, arnd
In-Reply-To: <34fd1ae9-9697-ac6c-d6bc-7c25b4515a25@intel.com>
Dave Hansen <dave.hansen@intel.com> writes:
> On 03/28/2018 01:47 PM, Thiago Jung Bauermann wrote:
>>>> if (flags)
>>>> - assert(rdpkey_reg() > orig_pkey_reg);
>>>> + assert(rdpkey_reg() < orig_pkey_reg);
>>>> }
>>>>
>>>> void pkey_write_allow(int pkey)
>>> This seems so horribly wrong that I wonder how it worked in the first
>>> place. Any idea?
>> The code simply wasn't used. pkey_disable_clear() is called by
>> pkey_write_allow() and pkey_access_allow(), but before this patch series
>> nothing called either of these functions.
>>
>
> Ahh, that explains it. Can that get stuck in the changelog, please?
Yes, will be in the next version.
--
Thiago Jung Bauermann
IBM Linux Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v12 07/22] selftests/vm: fixed bugs in pkey_disable_clear()
From: Dave Hansen @ 2018-03-28 20:55 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Ram Pai, shuahkh, linux-kselftest, mpe, linuxppc-dev, linux-mm,
x86, linux-arch, linux-doc, linux-kernel, mingo, akpm, benh,
paulus, khandual, aneesh.kumar, bsingharora, hbabu, mhocko,
ebiederm, arnd
In-Reply-To: <87muys3p2v.fsf@morokweng.localdomain>
On 03/28/2018 01:47 PM, Thiago Jung Bauermann wrote:
>>> if (flags)
>>> - assert(rdpkey_reg() > orig_pkey_reg);
>>> + assert(rdpkey_reg() < orig_pkey_reg);
>>> }
>>>
>>> void pkey_write_allow(int pkey)
>> This seems so horribly wrong that I wonder how it worked in the first
>> place. Any idea?
> The code simply wasn't used. pkey_disable_clear() is called by
> pkey_write_allow() and pkey_access_allow(), but before this patch series
> nothing called either of these functions.
>
Ahh, that explains it. Can that get stuck in the changelog, please?
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v12 07/22] selftests/vm: fixed bugs in pkey_disable_clear()
From: Thiago Jung Bauermann @ 2018-03-28 20:47 UTC (permalink / raw)
To: Dave Hansen
Cc: Ram Pai, shuahkh, linux-kselftest, mpe, linuxppc-dev, linux-mm,
x86, linux-arch, linux-doc, linux-kernel, mingo, akpm, benh,
paulus, khandual, aneesh.kumar, bsingharora, hbabu, mhocko,
ebiederm, arnd
In-Reply-To: <dc5ee0c8-afe3-78aa-001d-7b49b398337b@intel.com>
Dave Hansen <dave.hansen@intel.com> writes:
> On 02/21/2018 05:55 PM, Ram Pai wrote:
>> --- a/tools/testing/selftests/vm/protection_keys.c
>> +++ b/tools/testing/selftests/vm/protection_keys.c
>> @@ -461,7 +461,7 @@ void pkey_disable_clear(int pkey, int flags)
>> pkey, pkey, pkey_rights);
>> pkey_assert(pkey_rights >= 0);
>>
>> - pkey_rights |= flags;
>> + pkey_rights &= ~flags;
>>
>> ret = pkey_set(pkey, pkey_rights, 0);
>> /* pkey_reg and flags have the same format */
>> @@ -475,7 +475,7 @@ void pkey_disable_clear(int pkey, int flags)
>> dprintf1("%s(%d) pkey_reg: 0x%016lx\n", __func__,
>> pkey, rdpkey_reg());
>> if (flags)
>> - assert(rdpkey_reg() > orig_pkey_reg);
>> + assert(rdpkey_reg() < orig_pkey_reg);
>> }
>>
>> void pkey_write_allow(int pkey)
>
> This seems so horribly wrong that I wonder how it worked in the first
> place. Any idea?
The code simply wasn't used. pkey_disable_clear() is called by
pkey_write_allow() and pkey_access_allow(), but before this patch series
nothing called either of these functions.
--
Thiago Jung Bauermann
IBM Linux Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 2/8] dt-bindings: ingenic: Add DT bindings for TCU clocks
From: Mathieu Malaterre @ 2018-03-28 18:35 UTC (permalink / raw)
To: Paul Cercueil
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Lee Jones,
Daniel Lezcano, Ralf Baechle, Rob Herring, Jonathan Corbet,
Mark Rutland, James Hogan, Maarten ter Huurne, linux-clk,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
Linux-MIPS, linux-doc
In-Reply-To: <c02d2ae665890e7214bcfa6c42a49c69@crapouillou.net>
On Wed, Mar 28, 2018 at 5:04 PM, Paul Cercueil <paul@crapouillou.net> wrote:
> Le 2018-03-20 08:15, Mathieu Malaterre a écrit :
>>
>> Hi Paul,
>>
>> Two things:
>>
>> On Sun, Mar 18, 2018 at 12:28 AM, Paul Cercueil <paul@crapouillou.net>
>> wrote:
>>>
>>> This header provides clock numbers for the ingenic,tcu
>>> DT binding.
>>
>>
>> I have tested the whole series on my Creator CI20 with success, using:
>>
>> + tcu@10002000 {
>> + compatible = "ingenic,jz4780-tcu";
>> + reg = <0x10002000 0x140>;
>> +
>> + interrupt-parent = <&intc>;
>> + interrupts = <27 26 25>;
>> + };
>>
>>
>> So:
>>
>> Tested-by: Mathieu Malaterre <malat@debian.org>
>
>
> Great! Is that for the whole patchset or just this one patch?
The sentence just above was "whole series" :) so yes that was for the
whole series. Technically I only tested it on JZ4780, I hope this is
acceptable for the tag.
>
>>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>> ---
>>> include/dt-bindings/clock/ingenic,tcu.h | 23 +++++++++++++++++++++++
>>> 1 file changed, 23 insertions(+)
>>> create mode 100644 include/dt-bindings/clock/ingenic,tcu.h
>>>
>>> v2: Use SPDX identifier for the license
>>> v3: No change
>>> v4: No change
>>>
>>> diff --git a/include/dt-bindings/clock/ingenic,tcu.h
>>> b/include/dt-bindings/clock/ingenic,tcu.h
>>> new file mode 100644
>>> index 000000000000..179815d7b3bb
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/ingenic,tcu.h
>>> @@ -0,0 +1,23 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * This header provides clock numbers for the ingenic,tcu DT binding.
>>> + */
>>> +
>>> +#ifndef __DT_BINDINGS_CLOCK_INGENIC_TCU_H__
>>> +#define __DT_BINDINGS_CLOCK_INGENIC_TCU_H__
>>> +
>>> +#define JZ4740_CLK_TIMER0 0
>>> +#define JZ4740_CLK_TIMER1 1
>>> +#define JZ4740_CLK_TIMER2 2
>>> +#define JZ4740_CLK_TIMER3 3
>>> +#define JZ4740_CLK_TIMER4 4
>>> +#define JZ4740_CLK_TIMER5 5
>>> +#define JZ4740_CLK_TIMER6 6
>>> +#define JZ4740_CLK_TIMER7 7
>>> +#define JZ4740_CLK_WDT 8
>>> +#define JZ4740_CLK_LAST JZ4740_CLK_WDT
>>> +
>>> +#define JZ4770_CLK_OST 9
>>> +#define JZ4770_CLK_LAST JZ4770_CLK_OST
>>> +
>>
>>
>> When working on JZ4780 support, I always struggle to read those kind
>> of #define. Since this is a new patch would you mind changing
>> s/JZ4740/JZ47XX/ in your header ?
>
>
> Well the idea was that these defines are for JZ4740 and newer.
> But that means all Ingenic SoCs, so I guess I can change it.
>
>> Thanks for your work on JZ !
>
>
> Sure, thank you for testing!
>
>
>>> +#endif /* __DT_BINDINGS_CLOCK_INGENIC_TCU_H__ */
>>> --
>>> 2.11.0
>>>
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] docs/memory-barriers.txt: Fix broken DMA vs MMIO ordering example
From: Peter Zijlstra @ 2018-03-28 18:03 UTC (permalink / raw)
To: Tony Luck
Cc: Sinan Kaya, Paul McKenney, Will Deacon,
linux-kernel@vger.kernel.org, linux-doc, Benjamin Herrenschmidt,
Arnd Bergmann, Jason Gunthorpe, Ingo Molnar, Jonathan Corbet,
linux-ia64@vger.kernel.org
In-Reply-To: <CA+8MBb+61AhMkbo8xtd3Y0fY8xRUPD+kUsEvWgOz94KGEuovOw@mail.gmail.com>
On Wed, Mar 28, 2018 at 10:57:11AM -0700, Tony Luck wrote:
> On Wed, Mar 28, 2018 at 6:02 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> > +linux-ia64
> > Does IA64 follow this requirement? If not, is implementation planned?
> >
> > "no wmb() before writel()"
> >
> > Linus asked us to get rid of wmb() in front of writel() for UC memory.
> > Just checking that we are not breaking anything for IA64.
>
> We should be OK on ia64, writel() uses a cast to:
>
> *(volatile unsigned int __force *)
>
> which the compiler takes as a request to use a "st4.rel" instruction
> (meaning "store with release semantics"). So the value stored will
> be visible to anything that follows.
Just to nitpick, regular release semantics don't guarantee anything like
that, but ia64 never actually got around to implementing proper release
and it's a full barrier and thus what you say is true.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] docs/memory-barriers.txt: Fix broken DMA vs MMIO ordering example
From: Tony Luck @ 2018-03-28 17:57 UTC (permalink / raw)
To: Sinan Kaya
Cc: Paul McKenney, Will Deacon, linux-kernel@vger.kernel.org,
linux-doc, Benjamin Herrenschmidt, Arnd Bergmann, Jason Gunthorpe,
Peter Zijlstra, Ingo Molnar, Jonathan Corbet,
linux-ia64@vger.kernel.org
In-Reply-To: <c93ed5db-8cfd-1258-3396-62ca2076c44d@codeaurora.org>
On Wed, Mar 28, 2018 at 6:02 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> +linux-ia64
> Does IA64 follow this requirement? If not, is implementation planned?
>
> "no wmb() before writel()"
>
> Linus asked us to get rid of wmb() in front of writel() for UC memory.
> Just checking that we are not breaking anything for IA64.
We should be OK on ia64, writel() uses a cast to:
*(volatile unsigned int __force *)
which the compiler takes as a request to use a "st4.rel" instruction
(meaning "store with release semantics"). So the value stored will
be visible to anything that follows.
-Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox