* Re: [PATCH v3 6/6] interconnect: Add OPP table support for interconnects
From: Vincent Guittot @ 2019-07-15 8:16 UTC (permalink / raw)
To: Saravana Kannan
Cc: Georgi Djakov, Rob Herring, Mark Rutland, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, Sweeney, Sean,
daidavid1, Rajendra Nayak, sibis, Bjorn Andersson, Evan Green,
Android Kernel Team, open list:THERMAL,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel
In-Reply-To: <CAGETcx90WC2o+ZmkyhOPp1xJbfSk1wpAv2RA-4VgnhJfcsmJiA@mail.gmail.com>
On Tue, 9 Jul 2019 at 21:03, Saravana Kannan <saravanak@google.com> wrote:
>
> On Tue, Jul 9, 2019 at 12:25 AM Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
> >
> > On Sun, 7 Jul 2019 at 23:48, Saravana Kannan <saravanak@google.com> wrote:
> > >
> > > On Thu, Jul 4, 2019 at 12:12 AM Vincent Guittot
> > > <vincent.guittot@linaro.org> wrote:
> > > >
> > > > On Wed, 3 Jul 2019 at 23:33, Saravana Kannan <saravanak@google.com> wrote:
> > > > >
> > > > > On Tue, Jul 2, 2019 at 11:45 PM Vincent Guittot
> > > > > <vincent.guittot@linaro.org> wrote:
> > > > > >
> > > > > > On Wed, 3 Jul 2019 at 03:10, Saravana Kannan <saravanak@google.com> wrote:
> > > > > > >
> > > > > > > Interconnect paths can have different performance points. Now that OPP
> > > > > > > framework supports bandwidth OPP tables, add OPP table support for
> > > > > > > interconnects.
> > > > > > >
> > > > > > > Devices can use the interconnect-opp-table DT property to specify OPP
> > > > > > > tables for interconnect paths. And the driver can obtain the OPP table for
> > > > > > > an interconnect path by calling icc_get_opp_table().
> > > > > >
> > > > > > The opp table of a path must come from the aggregation of OPP tables
> > > > > > of the interconnect providers.
> > > > >
> > > > > The aggregation of OPP tables of the providers is certainly the
> > > > > superset of what a path can achieve, but to say that OPPs for
> > > > > interconnect path should match that superset is an oversimplification
> > > > > of the reality in hardware.
> > > > >
> > > > > There are lots of reasons an interconnect path might not want to use
> > > > > all the available bandwidth options across all the interconnects in
> > > > > the route.
> > > > >
> > > > > 1. That particular path might not have been validated or verified
> > > > > during the HW design process for some of the frequencies/bandwidth
> > > > > combinations of the providers.
> > > >
> > > > All these constraint are provider's constraints and not consumer's one
> > > >
> > > > The consumer asks for a bandwidth according to its needs and then the
> > > > providers select the optimal bandwidth of each interconnect after
> > > > aggregating all the request and according to what OPP have been
> > > > validated
> > >
> > > Not really. The screening can be a consumer specific issue. The
> > > consumer IP itself might have some issue with using too low of a
> > > bandwidth or bandwidth that's not within some range. It should not be
> >
> > How can an IP ask for not enough bandwidth ?
> > It asks the needed bandwidth based on its requirements
>
> The "enough bandwidth" is not always obvious. It's only for very
> simple cases that you can calculate the required bandwidth. Even for
> cases that you think might be "obvious/easy" aren't always easy.
>
> For example, you'd think a display IP would have a fixed bandwidth
> requirement for a fixed resolution screen. But that's far from the
> truth. It can also change as the number of layers change per frame.
> For video decoder/encoder, it depends on how well the frames compress
> with a specific compression scheme.
> So the "required" bandwidth is often a heuristic based on the IP
> frequency or traffic measurement.
>
> But that's not even the point I was making in this specific "bullet".
>
> A hardware IP might be screen/verified with only certain bandwidth
> levels. Or it might have hardware bugs that prevent it from using
> lower bandwidths even though it's technically sufficient. We need a
> way to capture that per path. This is not even a fictional case. This
> has been true multiple times over widely used IPs.
here you are mixing HW constraint on the soc and OPP screening with
bandwidth request from consumer
ICC framework is about getting bandwidth request not trying to fix
some HW/voltage dependency of the SoC
>
> > > the provider's job to take into account all the IP that might be
> > > connected to the interconnects. If the interconnect HW itself didn't
> >
> > That's not what I'm saying. The provider knows which bandwidth the
> > interconnect can provide as it is the ones which configures it. So if
> > the interconnect has a finite number of bandwidth point based probably
> > on the possible clock frequency and others config of the interconnect,
> > it selects the best final config after aggregating the request of the
> > consumer.
>
> I completely agree with this. What you are stating above is how it
> should work and that's the whole point of the interconnect framework.
>
> But this is orthogonal to the point I'm making.
It's not orthogonal because you want to add a OPP table pointer in the
ICC path structure to fix your platform HW constraint whereas it's not
the purpose of the framework IMO
>
> > > change, the provider driver shouldn't need to change. By your
> > > definition, a provider driver will have to account for all the
> > > possible bus masters that might be connected to it across all SoCs.
> >
> > you didn't catch my point
>
> Same. I think we are talking over each other. Let me try again.
>
> You are trying to describe how and interconnect provider and framework
> should work. There's no disagreement there.
>
> My point is that consumers might not want to or can not always use all
> the available bandwidth levels offered by the providers. There can be
> many reasons for that (which is what I listed in my earlier emails)
> and we need a good and generic way to capture that so that everyone
> isn't trying to invent their own property.
And my point is that you want to describe some platform or even UCs
specific constraint in the ICC framework which is not the place to do.
If the consumers might not want to use all available bandwidth because
this is not power efficient as an example, this should be describe
somewhere else to express that there is a shared power domain
between some devices and we shoudl ensure that all devices in this
power domain should use the Optimal Operating Point (optimal freq for
a voltage)
ICC framework describes the bandwidth request that are expressed by
the consumers for the current running state of their IP but it doesn't
reflect the fact that on platform A, the consumer should use bandwidth
X because it will select a voltage level of a shared power domain that
is optimized for the other devices B, C ... . It's up to the provider
to know HW details of the bus that it drives and to make such
decision; the consumer should always request the same
>
> > > That's not good design nor is it scalable.
> > >
> > > > >
> > > > > 2. Similarly during parts screening in the factory, some of the
> > > > > combinations might not have been screened and can't be guaranteed
> > > > > to work.
> > > >
> > > > As above, it's the provider's job to select the final bandwidth
> > > > according to its constraint
> > >
> > > Same reply as above.
> > >
> > > > >
> > > > > 3. Only a certain set of bandwidth levels might make sense to use from
> > > > > a power/performance balance given the device using it. For example:
> > > > > - The big CPU might not want to use some of the lower bandwidths
> > > > > but the little CPU might want to.
> > > > > - The big CPU might not want to use some intermediate bandwidth
> > > > > points if they don't save a lot of power compared to a higher
> > > > > bandwidth levels, but the little CPU might want to.
> > > > > - The little CPU might never want to use the higher set of
> > > > > bandwidth levels since they won't be power efficient for the use
> > > > > cases that might run on it.
> > > >
> > > > These example are quite vague about the reasons why little might never
> > > > want to use higher bandwidth.
> > >
> > > How is it vague? I just said because of power/performance balance.
> > >
> > > > But then, if little doesn't ask high bandwidth it will not use them.
> > >
> > > If you are running a heuristics based algorithm to pick bandwidth,
> > > this is how it'll know NOT to use some of the bandwidth levels.
> >
> > so you want to set a bandwidth according to the cpu frequency which is
> > what has been proposed in other thread
>
> Nope, that's just one heuristic. Often times it's based on hardware
> monitors measuring interconnect activity. If you go look at the SDM845
> in a Pixel 3, almost nothing is directly tied to the CPU frequency.
>
> Even if you are scaling bandwidth based on other hardware
> measurements, you might want to avoid some bandwidth level provided by
> the interconnect providers because it's suboptimal.
>
> For example, when making bandwidth votes to accommodate the big CPUs,
> you might never want to use some of the lower bandwidth levels because
> they are not power efficient for any CPU frequency or any bandwidth
> level. Because at those levels the memory/interconnect is so slow that
> it has a non-trivial utilization increase (because the CPU is
> stalling) of the big CPUs.
>
> Again, this is completely different from what the providers/icc
> framework does. Which is, once the request is made, they aggregate and
> set the actual interconnect frequencies correctly.
>
> > >
> > > > >
> > > > > 4. It might not make sense from a system level power perspective.
> > > > > Let's take an example of a path S (source) -> A -> B -> C -> D
> > > > > (destination).
> > > > > - A supports only 2, 5, 7 and 10 GB/s. B supports 1, 2 ... 10 GB/s.
> > > > > C supports 5 and 10 GB/s
> > > > > - If you combine and list the superset of bandwidth levels
> > > > > supported in that path, that'd be 1, 2, 3, ... 10 GB/s.
> > > > > - Which set of bandwidth levels make sense will depend on the
> > > > > hardware characteristics of the interconnects.
> > > > > - If B is the biggest power sink, then you might want to use all 10
> > > > > levels.
> > > > > - If A is the biggest power sink, then you might want to use all 2,
> > > > > 5 and 10 GB/s of the levels.
> > > > > - If C is the biggest power sink then you might only want to use 5
> > > > > and 10 GB/s
> > > > > - The more hops and paths you get the more convoluted this gets.
> > > > >
> > > > > 5. The design of the interconnects themselves might have an impact on
> > > > > which bandwidth levels are used.
> > > > > - For example, the FIFO depth between two specific interconnects
> > > > > might affect the valid bandwidth levels for a specific path.
> > > > > - Say S1 -> A -> B -> D1, S2 -> C -> B -> D1 and S2 -> C -> D2 are
> > > > > three paths.
> > > > > - If C <-> B FIFO depth is small, then there might be a requirement
> > > > > that C and B be closely performance matched to avoid system level
> > > > > congestion due to back pressure.
> > > > > - So S2 -> D1 path can't use all the bandwidth levels supported by
> > > > > C-B combination.
> > > > > - But S2 -> D2 can use all the bandwidth levels supported by C.
> > > > > - And S1 -> D1 can use all the levels supported by A-B combination.
> > > > >
> > > >
> > > > All the examples above makes sense but have to be handle by the
> > > > provider not the consumer. The consumer asks for a bandwidth according
> > > > to its constraints. Then the provider which is the driver that manages
> > > > the interconnect IP, should manage all this hardware and platform
> > > > specific stuff related to the interconnect IP in order to set the
> > > > optimal bandwidth that fit both consumer constraint and platform
> > > > specific configuration.
> > >
> > > Sure, but the provider itself can have interconnect properties to
> > > indicate which other interconnects it's tied to. And the provider will
> > > still need the interconnect-opp-table to denote which bandwidth levels
> > > are sensible to use with each of its connections.
>
> You seem to have missed this comment.
>
> Thanks,
> Saravana
>
> > > So in some instances the interconnect-opp-table covers the needs of
> > > purely consumers and in some instances purely providers. But in either
> > > case, it's still needed to describe the hardware properly.
> > >
> > > -Saravana
> > >
> > > > > These are just some of the reasons I could recollect in a few minutes.
> > > > > These are all real world cases I had to deal with in the past several
> > > > > years of dealing with scaling interconnects. I'm sure vendors and SoCs
> > > > > I'm not familiar with have other good reasons I'm not aware of.
> > > > >
> > > > > Trying to figure this all out by aggregating OPP tables of
> > > > > interconnect providers just isn't feasible nor is it efficient. The
> > > > > OPP tables for an interconnect path is describing the valid BW levels
> > > > > supported by that path and verified in hardware and makes a lot of
> > > > > sense to capture it clearly in DT.
> > > > >
> > > > > > So such kind of OPP table should be at
> > > > > > provider level but not at path level.
> > > > >
> > > > > They can also use it if they want to, but they'll probably want to use
> > > > > a frequency OPP table.
> > > > >
> > > > >
> > > > > -Saravana
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > > > > > ---
> > > > > > > drivers/interconnect/core.c | 27 ++++++++++++++++++++++++++-
> > > > > > > include/linux/interconnect.h | 7 +++++++
> > > > > > > 2 files changed, 33 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> > > > > > > index 871eb4bc4efc..881bac80bc1e 100644
> > > > > > > --- a/drivers/interconnect/core.c
> > > > > > > +++ b/drivers/interconnect/core.c
> > > > > > > @@ -47,6 +47,7 @@ struct icc_req {
> > > > > > > */
> > > > > > > struct icc_path {
> > > > > > > size_t num_nodes;
> > > > > > > + struct opp_table *opp_table;
> > > > > > > struct icc_req reqs[];
> > > > > > > };
> > > > > > >
> > > > > > > @@ -313,7 +314,7 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
> > > > > > > {
> > > > > > > struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
> > > > > > > struct icc_node *src_node, *dst_node;
> > > > > > > - struct device_node *np = NULL;
> > > > > > > + struct device_node *np = NULL, *opp_node;
> > > > > > > struct of_phandle_args src_args, dst_args;
> > > > > > > int idx = 0;
> > > > > > > int ret;
> > > > > > > @@ -381,10 +382,34 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
> > > > > > > dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
> > > > > > > mutex_unlock(&icc_lock);
> > > > > > >
> > > > > > > + opp_node = of_parse_phandle(np, "interconnect-opp-table", idx);
> > > > > > > + if (opp_node) {
> > > > > > > + path->opp_table = dev_pm_opp_of_find_table_from_node(opp_node);
> > > > > > > + of_node_put(opp_node);
> > > > > > > + }
> > > > > > > +
> > > > > > > +
> > > > > > > return path;
> > > > > > > }
> > > > > > > EXPORT_SYMBOL_GPL(of_icc_get);
> > > > > > >
> > > > > > > +/**
> > > > > > > + * icc_get_opp_table() - Get the OPP table that corresponds to a path
> > > > > > > + * @path: reference to the path returned by icc_get()
> > > > > > > + *
> > > > > > > + * This function will return the OPP table that corresponds to a path handle.
> > > > > > > + * If the interconnect API is disabled, NULL is returned and the consumer
> > > > > > > + * drivers will still build. Drivers are free to handle this specifically, but
> > > > > > > + * they don't have to.
> > > > > > > + *
> > > > > > > + * Return: opp_table pointer on success. NULL is returned when the API is
> > > > > > > + * disabled or the OPP table is missing.
> > > > > > > + */
> > > > > > > +struct opp_table *icc_get_opp_table(struct icc_path *path)
> > > > > > > +{
> > > > > > > + return path->opp_table;
> > > > > > > +}
> > > > > > > +
> > > > > > > /**
> > > > > > > * icc_set_bw() - set bandwidth constraints on an interconnect path
> > > > > > > * @path: reference to the path returned by icc_get()
> > > > > > > diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> > > > > > > index dc25864755ba..0c0bc55f0e89 100644
> > > > > > > --- a/include/linux/interconnect.h
> > > > > > > +++ b/include/linux/interconnect.h
> > > > > > > @@ -9,6 +9,7 @@
> > > > > > >
> > > > > > > #include <linux/mutex.h>
> > > > > > > #include <linux/types.h>
> > > > > > > +#include <linux/pm_opp.h>
> > > > > > >
> > > > > > > /* macros for converting to icc units */
> > > > > > > #define Bps_to_icc(x) ((x) / 1000)
> > > > > > > @@ -28,6 +29,7 @@ struct device;
> > > > > > > struct icc_path *icc_get(struct device *dev, const int src_id,
> > > > > > > const int dst_id);
> > > > > > > struct icc_path *of_icc_get(struct device *dev, const char *name);
> > > > > > > +struct opp_table *icc_get_opp_table(struct icc_path *path);
> > > > > > > void icc_put(struct icc_path *path);
> > > > > > > int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw);
> > > > > > >
> > > > > > > @@ -49,6 +51,11 @@ static inline void icc_put(struct icc_path *path)
> > > > > > > {
> > > > > > > }
> > > > > > >
> > > > > > > +static inline struct opp_table *icc_get_opp_table(struct icc_path *path)
> > > > > > > +{
> > > > > > > + return NULL;
> > > > > > > +}
> > > > > > > +
> > > > > > > static inline int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> > > > > > > {
> > > > > > > return 0;
> > > > > > > --
> > > > > > > 2.22.0.410.gd8fdbe21b5-goog
> > > > > > >
> > > >
> > > > --
> > > > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
> > > >
^ permalink raw reply
* Re: [PATCH v2 0/2] char: tpm: add new driver for tpm i2c ptp
From: Tomer Maimon @ 2019-07-15 8:08 UTC (permalink / raw)
To: Oshri Alkobi, Alexander Steffen, Jarkko Sakkinen
Cc: Rob Herring, Mark Rutland, peterhuewe, jgg, Arnd Bergmann,
Greg KH, IS20 Oshri Alkoby, devicetree,
AP MS30 Linux Kernel community, linux-integrity, gcwilson,
kgoldman, nayna, IS30 Dan Morav, eyal.cohen
In-Reply-To: <6e7ff1b958d84f6e8e585fd3273ef295@NTILML02.nuvoton.com>
[-- Attachment #1: Type: text/plain, Size: 3418 bytes --]
Hi Jarkko and All,
Thanks for your feedback and sorry for the late response.
Due to the amount of work required to handle this technical feedback and
project constraints we need to put this task on hold for the near future.
In the meantime, anyone from the community is welcome to take over this
code and handle the re-design for the benefit of the entire TPM community.
Thanks again,
Tomer
On Mon, 15 Jul 2019 at 10:54, <tomer.maimon@nuvoton.com> wrote:
>
>
> -----Original Message-----
> From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Sent: Friday, 5 July 2019 14:28
> To: Oshri Alkobi <oshrialkoby85@gmail.com>; Alexander Steffen <
> Alexander.Steffen@infineon.com>
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; peterhuewe@gmx.de;
> jgg@ziepe.ca; arnd@arndb.de; gregkh@linuxfoundation.org; IS20 Oshri
> Alkoby <oshri.alkoby@nuvoton.com>; devicetree@vger.kernel.org; AP MS30
> Linux Kernel community <linux-kernel@vger.kernel.org>;
> linux-integrity@vger.kernel.org; gcwilson@us.ibm.com; kgoldman@us.ibm.com;
> nayna@linux.vnet.ibm.com; IS30 Dan Morav <Dan.Morav@nuvoton.com>; IS20
> Tomer Maimon <tomer.maimon@nuvoton.com>
> Subject: Re: [PATCH v2 0/2] char: tpm: add new driver for tpm i2c ptp
>
> On Thu, 2019-07-04 at 12:48 -0500, Oshri Alkobi wrote:
> > Alex, Jarkko, thank you very much for your feedbacks!
>
> Please configure your email client to use plain text.
>
> > I totally agree, there are some duplications that can be common,
> > indeed it will require some work in tpm_tis_core.
> > Since I believe it is not going to happen soon, I would suggest to
> > examine what duplications can currently be dropped from the new
> > driver, so the kernel will support the PTP I2C interface in the meantime.
> > I will appreciate getting ideas about any tpm_tis_core logic that
> > currently can be used as is by the new drive.
>
> I rather wait for a solution that integrates with our mature stack for TIS
> (or these days FIFO) than integrate something half-baked. If you want
> something in, please do right things right.
>
> What you are proposing would mean maintaining duplicate stacks forever.
>
> > Since the TIS is an old specification that mostly defines FIFO for
> > TPM1.2 I would say the name tpm_tis_i2c does not completely reflect
> > its goal. However we really don't have any problem with any name that
> the group will agree on.
> > Does tpm_ptp_i2c sound better than the current name?
>
> Absolutely not going to use that name. The naming convention is what it is
> for other drivers that are adapt tpm_tis_core to different HW interfaces.
>
> /Jarkko
>
>
>
>
> ===========================================================================================
> The privileged confidential information contained in this email is
> intended for use only by the addressees as indicated by the original sender
> of this email. If you are not the addressee indicated in this email or are
> not responsible for delivery of the email to such a person, please kindly
> reply to the sender indicating this fact and delete all copies of it from
> your computer and network server immediately. Your cooperation is highly
> appreciated. It is advised that any unauthorized use of confidential
> information of Nuvoton is strictly prohibited; and any information in this
> email irrelevant to the official business of Nuvoton shall be deemed as
> neither given nor endorsed by Nuvoton.
>
[-- Attachment #2: Type: text/html, Size: 5821 bytes --]
^ permalink raw reply
* Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ulf Hansson @ 2019-07-15 8:01 UTC (permalink / raw)
To: Ludovic BARRE
Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin,
Alexandre TORGUE, Linux ARM, Linux Kernel Mailing List, DTML,
linux-mmc@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com
In-Reply-To: <1563176363071.36427@st.com>
On Mon, 15 Jul 2019 at 09:39, Ludovic BARRE <ludovic.barre@st.com> wrote:
>
> Hi Ulf
>
> like scheduled, I send you a "gentleman ping" about this series.
Thanks, I am just looking at it, again.
Kind regards
Uffe
>
> Regards,
> Ludo
> ________________________________________
> De : Ulf Hansson <ulf.hansson@linaro.org>
> Envoyé : jeudi 20 juin 2019 15:50
> À : Ludovic BARRE
> Cc : Rob Herring; Srinivas Kandagatla; Maxime Coquelin; Alexandre TORGUE; Linux ARM; Linux Kernel Mailing List; DTML; linux-mmc@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com
> Objet : Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
>
> Hi Ludovic,
>
> On Thu, 13 Jun 2019 at 15:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Thu, 13 Jun 2019 at 15:02, Ludovic BARRE <ludovic.barre@st.com> wrote:
> > >
> > > hi Ulf
> > >
> > > Just a "gentleman ping" about this series.
> > > I know you are busy, it's just to be sure you do not forget me :-)
> >
> > Thanks! I started briefly to review, but got distracted again. I will
> > come to it, but it just seems to take more time than it should, my
> > apologies.
>
> Alright, so I planned to review this this week - but failed. I have
> been overwhelmed with work lately (as usual when vacation is getting
> closer).
>
> I need to gently request to come back to this as of week 28, when I
> will give this the highest prio. Again apologize for the delays!
>
> Kind regards
> Uffe
>
> >
> > Br
> > Uffe
> >
> > >
> > > Regards
> > > Ludo
> > >
> > > On 6/3/19 5:55 PM, Ludovic Barre wrote:
> > > > From: Ludovic Barre <ludovic.barre@st.com>
> > > >
> > > > This patch series adds busy detect for stm32 sdmmc variant.
> > > > Some adaptations are required:
> > > > -Clear busy status bit if busy_detect_flag and busy_detect_mask are
> > > > different.
> > > > -Add hardware busy timeout with MMCIDATATIMER register.
> > > >
> > > > V3:
> > > > -rebase on latest mmc next
> > > > -replace re-read by status parameter.
> > > >
> > > > V2:
> > > > -mmci_cmd_irq cleanup in separate patch.
> > > > -simplify the busy_detect_flag exclude
> > > > -replace sdmmc specific comment in
> > > > "mmc: mmci: avoid fake busy polling in mmci_irq"
> > > > to focus on common behavior
> > > >
> > > > Ludovic Barre (3):
> > > > mmc: mmci: fix read status for busy detect
> > > > mmc: mmci: add hardware busy timeout feature
> > > > mmc: mmci: add busy detect for stm32 sdmmc variant
> > > >
> > > > drivers/mmc/host/mmci.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
> > > > drivers/mmc/host/mmci.h | 3 +++
> > > > 2 files changed, 44 insertions(+), 8 deletions(-)
> > > >
^ permalink raw reply
* RE: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic BARRE @ 2019-07-15 7:39 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin,
Alexandre TORGUE, Linux ARM, Linux Kernel Mailing List, DTML,
linux-mmc@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com
In-Reply-To: <CAPDyKFr_KNpNY-xgGdKXdAnmmD5OD1=wxgs2LmBAUJOn0mZwqg@mail.gmail.com>
Hi Ulf
like scheduled, I send you a "gentleman ping" about this series.
Regards,
Ludo
________________________________________
De : Ulf Hansson <ulf.hansson@linaro.org>
Envoyé : jeudi 20 juin 2019 15:50
À : Ludovic BARRE
Cc : Rob Herring; Srinivas Kandagatla; Maxime Coquelin; Alexandre TORGUE; Linux ARM; Linux Kernel Mailing List; DTML; linux-mmc@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com
Objet : Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
Hi Ludovic,
On Thu, 13 Jun 2019 at 15:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 13 Jun 2019 at 15:02, Ludovic BARRE <ludovic.barre@st.com> wrote:
> >
> > hi Ulf
> >
> > Just a "gentleman ping" about this series.
> > I know you are busy, it's just to be sure you do not forget me :-)
>
> Thanks! I started briefly to review, but got distracted again. I will
> come to it, but it just seems to take more time than it should, my
> apologies.
Alright, so I planned to review this this week - but failed. I have
been overwhelmed with work lately (as usual when vacation is getting
closer).
I need to gently request to come back to this as of week 28, when I
will give this the highest prio. Again apologize for the delays!
Kind regards
Uffe
>
> Br
> Uffe
>
> >
> > Regards
> > Ludo
> >
> > On 6/3/19 5:55 PM, Ludovic Barre wrote:
> > > From: Ludovic Barre <ludovic.barre@st.com>
> > >
> > > This patch series adds busy detect for stm32 sdmmc variant.
> > > Some adaptations are required:
> > > -Clear busy status bit if busy_detect_flag and busy_detect_mask are
> > > different.
> > > -Add hardware busy timeout with MMCIDATATIMER register.
> > >
> > > V3:
> > > -rebase on latest mmc next
> > > -replace re-read by status parameter.
> > >
> > > V2:
> > > -mmci_cmd_irq cleanup in separate patch.
> > > -simplify the busy_detect_flag exclude
> > > -replace sdmmc specific comment in
> > > "mmc: mmci: avoid fake busy polling in mmci_irq"
> > > to focus on common behavior
> > >
> > > Ludovic Barre (3):
> > > mmc: mmci: fix read status for busy detect
> > > mmc: mmci: add hardware busy timeout feature
> > > mmc: mmci: add busy detect for stm32 sdmmc variant
> > >
> > > drivers/mmc/host/mmci.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
> > > drivers/mmc/host/mmci.h | 3 +++
> > > 2 files changed, 44 insertions(+), 8 deletions(-)
> > >
^ permalink raw reply
* RE: [PATCH v2 00/11] Arasan SDHCI enhancements and ZynqMP Tap Delays Handling
From: Manish Narani @ 2019-07-15 7:15 UTC (permalink / raw)
To: Manish Narani, ulf.hansson@linaro.org, robh+dt@kernel.org,
mark.rutland@arm.com, heiko@sntech.de, Michal Simek,
adrian.hunter@intel.com, christoph.muellner@theobroma-systems.com,
philipp.tomsich@theobroma-systems.com, viresh.kumar@linaro.org,
scott.branden@broadcom.com, ayaka@soulik.info, kernel@esmil.dk,
tony.xie@rock-chips.com, mdf@kernel.org, olof
Cc: linux-mmc@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
In-Reply-To: <1561958991-21935-1-git-send-email-manish.narani@xilinx.com>
Ping!
> -----Original Message-----
> From: Manish Narani <manish.narani@xilinx.com>
> Sent: Monday, July 1, 2019 11:00 AM
> To: ulf.hansson@linaro.org; robh+dt@kernel.org; mark.rutland@arm.com;
> heiko@sntech.de; Michal Simek <michals@xilinx.com>;
> adrian.hunter@intel.com; christoph.muellner@theobroma-systems.com;
> philipp.tomsich@theobroma-systems.com; viresh.kumar@linaro.org;
> scott.branden@broadcom.com; ayaka@soulik.info; kernel@esmil.dk;
> tony.xie@rock-chips.com; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah
> <JOLLYS@xilinx.com>; Nava kishore Manne <navam@xilinx.com>;
> mdf@kernel.org; Manish Narani <MNARANI@xilinx.com>; olof@lixom.net
> Cc: linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> rockchip@lists.infradead.org
> Subject: [PATCH v2 00/11] Arasan SDHCI enhancements and ZynqMP Tap
> Delays Handling
>
> This patch series does the following:
> - Reorganize the Clock Handling in Arasan SD driver
> - Adds new sampling clock in Arasan SD driver
> - Adds support to set Clock Delays in SD Arasan Driver
> - Add SDIO Tap Delay handling in ZynqMP firmware driver
> - Add support for ZynqMP Tap Delays setting in Arasan SD driver
>
> Changes in v2:
> - Replaced the deprecated calls to clock framework APIs
> - Added support for dev_clk_get() call to work for SD card clock
> - Separated the clock data struct
> - Fragmented the patch series in smaller patches to make it more
> readable
>
> This patch series contains a DT patch, which I think should be there to
> maintain the order of commits.
>
> Manish Narani (11):
> dt-bindings: mmc: arasan: Update documentation for SD Card Clock
> arm64: dts: rockchip: Add optional clock property indicating sdcard
> clock
> mmc: sdhci-of-arasan: Replace deprecated clk API calls
> mmc: sdhci-of-arasan: Separate out clk related data to another
> structure
> dt-bindings: mmc: arasan: Update Documentation for the input clock
> mmc: sdhci-of-arasan: Add sampling clock for a phy to use
> dt-bindings: mmc: arasan: Add optional properties for Arasan SDHCI
> mmc: sdhci-of-arasan: Add support to set clock phase delays for SD
> firmware: xilinx: Add SDIO Tap Delay APIs
> dt-bindings: mmc: arasan: Document 'xlnx,zynqmp-8.9a' controller
> mmc: sdhci-of-arasan: Add support for ZynqMP Platform Tap Delays Setup
>
> .../devicetree/bindings/mmc/arasan,sdhci.txt | 49 ++-
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 4 +-
> drivers/firmware/xilinx/zynqmp.c | 48 +++
> drivers/mmc/host/sdhci-of-arasan.c | 453 ++++++++++++++++++++-
> include/linux/firmware/xlnx-zynqmp.h | 15 +-
> 5 files changed, 540 insertions(+), 29 deletions(-)
>
> --
> 2.1.1
^ permalink raw reply
* Re: [PATCH v2 3/3] ARM: dts: stm32: add syscfg to ADC on stm32mp157c
From: Fabrice Gasnier @ 2019-07-15 7:01 UTC (permalink / raw)
To: Jonathan Cameron
Cc: mark.rutland, devicetree, lars, alexandre.torgue, linux-iio,
pmeerw, linux-kernel, robh+dt, mcoquelin.stm32, knaack.h,
linux-stm32, linux-arm-kernel
In-Reply-To: <20190714171310.1816afe3@archlinux>
On 7/14/19 6:13 PM, Jonathan Cameron wrote:
> On Wed, 3 Jul 2019 12:08:16 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>
>> On stm32mp157c, the ADC inputs are multiplexed with analog switches which
>> have reduced performances when their supply is below 2.7V (vdda by
>> default).
>> Add syscfg registers that can be used on stm32mp157c, to get full ADC
>> analog performances.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> I've applied the patches using this on the assumption this will go via
> the usual SoC route.
>
> Thanks,
Hi Jonathan,
Many thanks,
Fabrice
>
> Jonathan
>
>> ---
>> arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
>> index 2dd5162..b9a5b58 100644
>> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
>> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
>> @@ -862,6 +862,7 @@
>> clocks = <&rcc ADC12>, <&rcc ADC12_K>;
>> clock-names = "bus", "adc";
>> interrupt-controller;
>> + st,syscfg = <&syscfg>;
>> #interrupt-cells = <1>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>
^ permalink raw reply
* [RFC PATCH v2 15/15] tracing: of: Add function-graph tracer option properties
From: Masami Hiramatsu @ 2019-07-15 5:13 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Add following function-graph tracer related options
- fgraph-filters : string array of filter
- fgraph-notraces : string array of notrace-filter
- fgraph-max-depth : u32 value of max depth
Note that these properties are available on ftrace root node
only, because these filters are globally applied.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/ftrace.c | 85 ++++++++++++++++++++++++++++++-----------------
kernel/trace/trace_of.c | 68 ++++++++++++++++++++++++++++++++++++++
2 files changed, 123 insertions(+), 30 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5c3eadb143ed..7bc60ac4583e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1241,7 +1241,7 @@ static void clear_ftrace_mod_list(struct list_head *head)
mutex_unlock(&ftrace_lock);
}
-static void free_ftrace_hash(struct ftrace_hash *hash)
+void free_ftrace_hash(struct ftrace_hash *hash)
{
if (!hash || hash == EMPTY_HASH)
return;
@@ -4895,7 +4895,7 @@ __setup("ftrace_filter=", set_ftrace_filter);
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
static char ftrace_graph_buf[FTRACE_FILTER_SIZE] __initdata;
static char ftrace_graph_notrace_buf[FTRACE_FILTER_SIZE] __initdata;
-static int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
+int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
static int __init set_graph_function(char *str)
{
@@ -5224,6 +5224,26 @@ __ftrace_graph_open(struct inode *inode, struct file *file,
return ret;
}
+struct ftrace_hash *ftrace_graph_copy_hash(bool enable)
+{
+ struct ftrace_hash *hash;
+
+ mutex_lock(&graph_lock);
+
+ if (enable)
+ hash = rcu_dereference_protected(ftrace_graph_hash,
+ lockdep_is_held(&graph_lock));
+ else
+ hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
+ lockdep_is_held(&graph_lock));
+
+ hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, hash);
+
+ mutex_unlock(&graph_lock);
+
+ return hash;
+}
+
static int
ftrace_graph_open(struct inode *inode, struct file *file)
{
@@ -5280,11 +5300,40 @@ ftrace_graph_notrace_open(struct inode *inode, struct file *file)
return ret;
}
+int ftrace_graph_apply_hash(struct ftrace_hash *hash, bool enable)
+{
+ struct ftrace_hash *old_hash, *new_hash;
+
+ new_hash = __ftrace_hash_move(hash);
+ if (!new_hash)
+ return -ENOMEM;
+
+ mutex_lock(&graph_lock);
+
+ if (enable) {
+ old_hash = rcu_dereference_protected(ftrace_graph_hash,
+ lockdep_is_held(&graph_lock));
+ rcu_assign_pointer(ftrace_graph_hash, new_hash);
+ } else {
+ old_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
+ lockdep_is_held(&graph_lock));
+ rcu_assign_pointer(ftrace_graph_notrace_hash, new_hash);
+ }
+
+ mutex_unlock(&graph_lock);
+
+ /* Wait till all users are no longer using the old hash */
+ synchronize_rcu();
+
+ free_ftrace_hash(old_hash);
+
+ return 0;
+}
+
static int
ftrace_graph_release(struct inode *inode, struct file *file)
{
struct ftrace_graph_data *fgd;
- struct ftrace_hash *old_hash, *new_hash;
struct trace_parser *parser;
int ret = 0;
@@ -5309,41 +5358,17 @@ ftrace_graph_release(struct inode *inode, struct file *file)
trace_parser_put(parser);
- new_hash = __ftrace_hash_move(fgd->new_hash);
- if (!new_hash) {
- ret = -ENOMEM;
- goto out;
- }
-
- mutex_lock(&graph_lock);
-
- if (fgd->type == GRAPH_FILTER_FUNCTION) {
- old_hash = rcu_dereference_protected(ftrace_graph_hash,
- lockdep_is_held(&graph_lock));
- rcu_assign_pointer(ftrace_graph_hash, new_hash);
- } else {
- old_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
- lockdep_is_held(&graph_lock));
- rcu_assign_pointer(ftrace_graph_notrace_hash, new_hash);
- }
-
- mutex_unlock(&graph_lock);
-
- /* Wait till all users are no longer using the old hash */
- synchronize_rcu();
-
- free_ftrace_hash(old_hash);
+ ret = ftrace_graph_apply_hash(fgd->new_hash,
+ fgd->type == GRAPH_FILTER_FUNCTION);
}
- out:
free_ftrace_hash(fgd->new_hash);
kfree(fgd);
return ret;
}
-static int
-ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer)
+int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer)
{
struct ftrace_glob func_g;
struct dyn_ftrace *rec;
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 1ee6fab918f5..d78a082b1752 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -75,6 +75,71 @@ trace_of_set_instance_options(struct trace_array *tr, struct device_node *node)
}
}
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+extern unsigned int fgraph_max_depth;
+extern struct ftrace_hash *ftrace_graph_copy_hash(bool enable);
+extern int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
+extern int ftrace_graph_apply_hash(struct ftrace_hash *hash, bool enable);
+extern void free_ftrace_hash(struct ftrace_hash *hash);
+
+static void __init
+trace_of_set_fgraph_filter(struct device_node *node, const char *property,
+ bool enable)
+{
+ struct ftrace_hash *hash;
+ struct property *prop;
+ const char *p;
+ char buf[MAX_BUF_LEN];
+ int err;
+
+ if (!of_find_property(node, property, NULL))
+ return;
+
+ /* Get current filter hash */
+ hash = ftrace_graph_copy_hash(enable);
+ if (!hash) {
+ pr_err("Failed to copy hash\n");
+ return;
+ }
+
+ of_property_for_each_string(node, property, prop, p) {
+ if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+ pr_err("filter string is too long: %s\n", p);
+ goto free_hash;
+ }
+ err = ftrace_graph_set_hash(hash, buf);
+ if (err) {
+ pr_err("Failed to add %s: %s\n", property, buf);
+ goto free_hash;
+ }
+ }
+
+ if (ftrace_graph_apply_hash(hash, enable) < 0) {
+ pr_err("Failed to apply new hash\n");
+ goto free_hash;
+ }
+
+ return;
+
+free_hash:
+ free_ftrace_hash(hash);
+}
+
+static void __init
+trace_of_set_fgraph_options(struct device_node *node)
+{
+ u32 v = 0;
+
+ trace_of_set_fgraph_filter(node, "fgraph-filters", true);
+ trace_of_set_fgraph_filter(node, "fgraph-notraces", false);
+
+ if (!of_property_read_u32_index(node, "fgraph-max-depth", 0, &v))
+ fgraph_max_depth = (unsigned int)v;
+}
+#else
+#define trace_of_set_fgraph_options(node) do {} while (0)
+#endif
+
static void __init
trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
{
@@ -100,6 +165,9 @@ trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
if (tracing_alloc_snapshot() < 0)
pr_err("Failed to allocate snapshot buffer\n");
+ /* function graph filters are global settings. */
+ trace_of_set_fgraph_options(node);
+
trace_of_set_instance_options(tr, node);
}
^ permalink raw reply related
* [RFC PATCH v2 14/15] tracing: of: Add function tracer filter properties
From: Masami Hiramatsu @ 2019-07-15 5:13 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Add function tracer filter properties which includes
- ftrace-filters : string array of filter rules
- ftrace-notraces : string array of notrace rules
These properties are available on ftrace root node and
instance node.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_of.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 14d6c9a8fd81..1ee6fab918f5 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -322,11 +322,47 @@ trace_of_init_events(struct trace_array *tr, struct device_node *node)
#define trace_of_init_events(tr, node) do {} while (0)
#endif
+#ifdef CONFIG_FUNCTION_TRACER
+extern bool ftrace_filter_param __initdata;
+extern int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
+ int len, int reset);
+extern int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
+ int len, int reset);
+
+static void __init
+trace_of_set_ftrace_filter(struct ftrace_ops *ops, const char *property,
+ struct device_node *node)
+{
+ struct property *prop;
+ const char *p;
+ char buf[MAX_BUF_LEN];
+ int err;
+
+ of_property_for_each_string(node, property, prop, p) {
+ if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+ pr_err("filter string is too long: %s\n", p);
+ return;
+ }
+ err = ftrace_set_filter(ops, buf, strlen(buf), 0);
+ if (err) {
+ pr_err("Failed to add %s: %s\n", property, buf);
+ return;
+ }
+ ftrace_filter_param = true;
+ }
+}
+#else
+#define trace_of_set_ftrace_filter(ops, prop, node) do {} while (0)
+#endif
+
static void __init
trace_of_enable_tracer(struct trace_array *tr, struct device_node *node)
{
const char *p;
+ trace_of_set_ftrace_filter(tr->ops, "ftrace-filters", node);
+ trace_of_set_ftrace_filter(tr->ops, "ftrace-notraces", node);
+
if (!of_property_read_string(node, "tracer", &p)) {
if (tracing_set_tracer(tr, p) < 0)
pr_err("Failed to set given tracer: %s\n", p);
^ permalink raw reply related
* [RFC PATCH v2 13/15] tracing: of: Add cpumask property support
From: Masami Hiramatsu @ 2019-07-15 5:13 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Add cpumask property for tracing. Note that this property accepts
a string cpumask, not a digit number.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace.c | 41 ++++++++++++++++++++++++++++-------------
kernel/trace/trace_of.c | 14 ++++++++++++++
2 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6f50c58a3ad4..251d9de14b12 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4437,20 +4437,12 @@ tracing_cpumask_read(struct file *filp, char __user *ubuf,
return count;
}
-static ssize_t
-tracing_cpumask_write(struct file *filp, const char __user *ubuf,
- size_t count, loff_t *ppos)
+int tracing_set_cpumask(struct trace_array *tr, cpumask_var_t tracing_cpumask_new)
{
- struct trace_array *tr = file_inode(filp)->i_private;
- cpumask_var_t tracing_cpumask_new;
- int err, cpu;
-
- if (!alloc_cpumask_var(&tracing_cpumask_new, GFP_KERNEL))
- return -ENOMEM;
+ int cpu;
- err = cpumask_parse_user(ubuf, count, tracing_cpumask_new);
- if (err)
- goto err_unlock;
+ if (!tr)
+ return -EINVAL;
local_irq_disable();
arch_spin_lock(&tr->max_lock);
@@ -4474,11 +4466,34 @@ tracing_cpumask_write(struct file *filp, const char __user *ubuf,
local_irq_enable();
cpumask_copy(tr->tracing_cpumask, tracing_cpumask_new);
+
+ return 0;
+}
+
+static ssize_t
+tracing_cpumask_write(struct file *filp, const char __user *ubuf,
+ size_t count, loff_t *ppos)
+{
+ struct trace_array *tr = file_inode(filp)->i_private;
+ cpumask_var_t tracing_cpumask_new;
+ int err;
+
+ if (!alloc_cpumask_var(&tracing_cpumask_new, GFP_KERNEL))
+ return -ENOMEM;
+
+ err = cpumask_parse_user(ubuf, count, tracing_cpumask_new);
+ if (err)
+ goto err_free;
+
+ err = tracing_set_cpumask(tr, tracing_cpumask_new);
+ if (err)
+ goto err_free;
+
free_cpumask_var(tracing_cpumask_new);
return count;
-err_unlock:
+err_free:
free_cpumask_var(tracing_cpumask_new);
return err;
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 7fe81c25dc59..14d6c9a8fd81 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -22,6 +22,8 @@ extern void __init trace_init_tracepoint_printk(void);
extern ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
unsigned long size, int cpu_id);
extern struct trace_array *trace_array_create(const char *name);
+extern int tracing_set_cpumask(struct trace_array *tr,
+ cpumask_var_t tracing_cpumask_new);
static void __init
trace_of_set_instance_options(struct trace_array *tr, struct device_node *node)
@@ -59,6 +61,18 @@ trace_of_set_instance_options(struct trace_array *tr, struct device_node *node)
pr_err("Failed to resize trace buffer to %d KB\n",
v >> 10);
}
+
+ err = of_property_read_string(node, "cpumask", &p);
+ if (!err) {
+ cpumask_var_t new_mask;
+
+ if (alloc_cpumask_var(&new_mask, GFP_KERNEL)) {
+ if (cpumask_parse(p, new_mask) < 0 ||
+ tracing_set_cpumask(tr, new_mask) < 0)
+ pr_err("Failed to set new CPU mask %s\n", p);
+ free_cpumask_var(new_mask);
+ }
+ }
}
static void __init
^ permalink raw reply related
* [RFC PATCH v2 12/15] tracing: of: Add instance node support
From: Masami Hiramatsu @ 2019-07-15 5:13 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Add instance node support to devicetree ftrace binding.
User can set some options and event nodes in instance node.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_of.c | 67 +++++++++++++++++++++++++++++++++++++++--------
1 file changed, 55 insertions(+), 12 deletions(-)
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index e9142c63ece1..7fe81c25dc59 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -21,9 +21,10 @@ extern int tracing_set_tracer(struct trace_array *tr, const char *buf);
extern void __init trace_init_tracepoint_printk(void);
extern ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
unsigned long size, int cpu_id);
+extern struct trace_array *trace_array_create(const char *name);
static void __init
-trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
+trace_of_set_instance_options(struct trace_array *tr, struct device_node *node)
{
struct property *prop;
const char *p;
@@ -48,6 +49,24 @@ trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
pr_err("Failed to set trace clock: %s\n", p);
}
+ /* This accepts per-cpu buffer size in KB */
+ err = of_property_read_u32_index(node, "buffer-size-kb", 0, &v);
+ if (!err) {
+ v <<= 10; /* KB to Byte */
+ if (v < PAGE_SIZE)
+ pr_err("Buffer size is too small: %d KB\n", v >> 10);
+ if (tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0)
+ pr_err("Failed to resize trace buffer to %d KB\n",
+ v >> 10);
+ }
+}
+
+static void __init
+trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
+{
+ u32 v = 0;
+ int err;
+
/* Command line boot options */
if (of_find_property(node, "dump-on-oops", NULL)) {
err = of_property_read_u32_index(node, "dump-on-oops", 0, &v);
@@ -63,20 +82,11 @@ trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
if (of_find_property(node, "tp-printk", NULL))
trace_init_tracepoint_printk();
- /* This accepts per-cpu buffer size in KB */
- err = of_property_read_u32_index(node, "buffer-size-kb", 0, &v);
- if (!err) {
- v <<= 10; /* KB to Byte */
- if (v < PAGE_SIZE)
- pr_err("Buffer size is too small: %d KB\n", v >> 10);
- if (tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0)
- pr_err("Failed to resize trace buffer to %d KB\n",
- v >> 10);
- }
-
if (of_find_property(node, "alloc-snapshot", NULL))
if (tracing_alloc_snapshot() < 0)
pr_err("Failed to allocate snapshot buffer\n");
+
+ trace_of_set_instance_options(tr, node);
}
#ifdef CONFIG_EVENT_TRACING
@@ -310,6 +320,38 @@ trace_of_enable_tracer(struct trace_array *tr, struct device_node *node)
}
}
+static void __init
+trace_of_init_instances(struct device_node *__node)
+{
+ struct device_node *node;
+ struct trace_array *tr;
+ const char *p;
+ int err;
+
+ for_each_child_of_node(__node, node) {
+ if (!of_node_name_prefix(node, "instance"))
+ continue;
+
+ err = of_property_read_string(node, "instance", &p);
+ if (err) {
+ pr_err("Failed to get instance name on %s\n",
+ of_node_full_name(node));
+ continue;
+ }
+
+ tr = trace_array_create(p);
+ if (IS_ERR(tr)) {
+ pr_err("Failed to create instance %s\n", p);
+ continue;
+ }
+
+ trace_of_set_instance_options(tr, node);
+ trace_of_init_events(tr, node);
+ trace_of_enable_events(tr, node);
+ trace_of_enable_tracer(tr, node);
+ }
+}
+
static struct device_node * __init trace_of_find_ftrace_node(void)
{
if (!of_chosen)
@@ -337,6 +379,7 @@ static int __init trace_of_init(void)
trace_of_init_events(tr, trace_node);
trace_of_enable_events(tr, trace_node);
trace_of_enable_tracer(tr, trace_node);
+ trace_of_init_instances(trace_node);
end:
of_node_put(trace_node);
^ permalink raw reply related
* [RFC PATCH v2 11/15] tracing: of: Add synthetic event support
From: Masami Hiramatsu @ 2019-07-15 5:13 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Add synthetic event node support. The synthetic event node must be
a child node of ftrace node, and the node must start with "synth@"
prefix. The synth node requires fields string (not string array),
which defines the fields as same as tracing/synth_events interface.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Changes in v2:
- Make synth property available only if CONFIG_HIST_TRIGGERS=y
---
kernel/trace/trace_events_hist.c | 5 +++
kernel/trace/trace_of.c | 64 ++++++++++++++++++++++++++++++++++++--
2 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index db973928e580..e7f5d0a353e2 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1343,6 +1343,11 @@ static int create_or_delete_synth_event(int argc, char **argv)
return ret == -ECANCELED ? -EINVAL : ret;
}
+int synth_event_run_command(const char *command)
+{
+ return trace_run_command(command, create_or_delete_synth_event);
+}
+
static int synth_event_create(int argc, const char **argv)
{
const char *name = argv[0];
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 56c5deb45f54..e9142c63ece1 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -146,6 +146,49 @@ trace_of_add_kprobe_event(struct device_node *node,
}
#endif
+#ifdef CONFIG_HIST_TRIGGERS
+extern int synth_event_run_command(const char *command);
+
+static int __init
+trace_of_add_synth_event(struct device_node *node, const char *event)
+{
+ struct property *prop;
+ char buf[MAX_BUF_LEN], *q;
+ const char *p;
+ int len, delta, ret;
+
+ len = ARRAY_SIZE(buf);
+ delta = snprintf(buf, len, "%s", event);
+ if (delta >= len) {
+ pr_err("Event name is too long: %s\n", event);
+ return -E2BIG;
+ }
+ len -= delta; q = buf + delta;
+
+ of_property_for_each_string(node, "fields", prop, p) {
+ delta = snprintf(q, len, " %s;", p);
+ if (delta >= len) {
+ pr_err("fields string is too long: %s\n", p);
+ return -E2BIG;
+ }
+ len -= delta; q += delta;
+ }
+
+ ret = synth_event_run_command(buf);
+ if (ret < 0)
+ pr_err("Failed to add synthetic event: %s\n", buf);
+
+ return ret;
+}
+#else
+static inline int __init
+trace_of_add_synth_event(struct device_node *node, const char *event)
+{
+ pr_err("Synthetic event is not supported.\n");
+ return -ENOTSUPP;
+}
+#endif
+
static void __init
trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
{
@@ -173,15 +216,30 @@ trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
event = buf;
group = strsep(&event, ":");
- /* For a kprobe event, we have to generates an event at first */
+
+ /* Generates kprobe/synth event at first */
if (of_find_property(node, "probes", NULL)) {
+ if (of_find_property(node, "fields", NULL)) {
+ pr_err("Error: %s node has both probes and fields\n",
+ of_node_full_name(node));
+ return;
+ }
if (!event) {
event = buf;
group = "kprobes";
}
- err = trace_of_add_kprobe_event(node, group, event);
- if (err < 0)
+ if (trace_of_add_kprobe_event(node, group, event) < 0)
+ return;
+ } else if (of_find_property(node, "fields", NULL)) {
+ if (!event)
+ event = buf;
+ else if (strcmp(group, "synthetic") != 0) {
+ pr_err("Synthetic event must be in synthetic group\n");
+ return;
+ }
+ if (trace_of_add_synth_event(node, event) < 0)
return;
+ group = "synthetic";
} else {
if (!event) {
pr_err("%s has no group name\n", buf);
^ permalink raw reply related
* [RFC PATCH v2 10/15] tracing: of: Add kprobe event support
From: Masami Hiramatsu @ 2019-07-15 5:12 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Add kprobe event support in event node. User can add probe definitions
by "probes" property as a string array.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Changes in v2:
- Make probes property avaiable if CONFIG_KPROBE_EVENTS=y.
---
kernel/trace/trace_kprobe.c | 5 +++
kernel/trace/trace_of.c | 76 +++++++++++++++++++++++++++++++++++++------
2 files changed, 71 insertions(+), 10 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5135c07b6557..03ce60928c18 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -728,6 +728,11 @@ static int create_or_delete_trace_kprobe(int argc, char **argv)
return ret == -ECANCELED ? -EINVAL : ret;
}
+int trace_kprobe_run_command(const char *command)
+{
+ return trace_run_command(command, create_or_delete_trace_kprobe);
+}
+
static int trace_kprobe_release(struct dyn_event *ev)
{
struct trace_kprobe *tk = to_trace_kprobe(ev);
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 3719f711144a..56c5deb45f54 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -101,14 +101,59 @@ trace_of_enable_events(struct trace_array *tr, struct device_node *node)
}
}
+#ifdef CONFIG_KPROBE_EVENTS
+extern int trace_kprobe_run_command(const char *command);
+
+static int __init
+trace_of_add_kprobe_event(struct device_node *node,
+ const char *group, const char *event)
+{
+ struct property *prop;
+ char buf[MAX_BUF_LEN];
+ const char *p;
+ char *q;
+ int len, ret;
+
+ len = snprintf(buf, ARRAY_SIZE(buf) - 1, "p:%s/%s ", group, event);
+ if (len >= ARRAY_SIZE(buf)) {
+ pr_err("Event name is too long: %s\n", event);
+ return -E2BIG;
+ }
+ q = buf + len;
+ len = ARRAY_SIZE(buf) - len;
+
+ of_property_for_each_string(node, "probes", prop, p) {
+ if (strlcpy(q, p, len) >= len) {
+ pr_err("Probe definition is too long: %s\n", p);
+ return -E2BIG;
+ }
+ ret = trace_kprobe_run_command(buf);
+ if (ret < 0) {
+ pr_err("Failed to add probe: %s\n", buf);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+#else
+static inline int __init
+trace_of_add_kprobe_event(struct device_node *node,
+ const char *group, const char *event)
+{
+ pr_err("Kprobe event is not supported.\n");
+ return -ENOTSUPP;
+}
+#endif
+
static void __init
trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
{
struct trace_event_file *file;
struct property *prop;
char buf[MAX_BUF_LEN];
- char *bufp;
- const char *p;
+ const char *p, *group;
+ char *event;
int err;
if (!of_node_name_prefix(node, "event"))
@@ -125,18 +170,29 @@ trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
pr_err("Event name is too long: %s\n", p);
return;
}
- bufp = buf;
-
- p = strsep(&bufp, ":");
- if (!bufp) {
- pr_err("%s has no group name\n", buf);
- return;
+ event = buf;
+
+ group = strsep(&event, ":");
+ /* For a kprobe event, we have to generates an event at first */
+ if (of_find_property(node, "probes", NULL)) {
+ if (!event) {
+ event = buf;
+ group = "kprobes";
+ }
+ err = trace_of_add_kprobe_event(node, group, event);
+ if (err < 0)
+ return;
+ } else {
+ if (!event) {
+ pr_err("%s has no group name\n", buf);
+ return;
+ }
}
mutex_lock(&event_mutex);
- file = find_event_file(tr, buf, bufp);
+ file = find_event_file(tr, group, event);
if (!file) {
- pr_err("Failed to find event: %s\n", buf);
+ pr_err("Failed to find event: %s:%s\n", group, event);
return;
}
^ permalink raw reply related
* [RFC PATCH v2 09/15] tracing: of: Add trace event settings
From: Masami Hiramatsu @ 2019-07-15 5:12 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Add per-event settings, which includes filter and actions.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Changes in v2:
- Make event node available only if CONFIG_EVENT_TRACING=y
---
kernel/trace/trace_events_trigger.c | 2 -
kernel/trace/trace_of.c | 82 +++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 2a2912cb4533..74a19c18219f 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -208,7 +208,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
return ret;
}
-static int trigger_process_regex(struct trace_event_file *file, char *buff)
+int trigger_process_regex(struct trace_event_file *file, char *buff)
{
char *command, *next = buff;
struct event_command *p;
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 5e108a1d5bb3..3719f711144a 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -81,6 +81,7 @@ trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
#ifdef CONFIG_EVENT_TRACING
extern int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
+extern int trigger_process_regex(struct trace_event_file *file, char *buff);
static void __init
trace_of_enable_events(struct trace_array *tr, struct device_node *node)
@@ -99,8 +100,88 @@ trace_of_enable_events(struct trace_array *tr, struct device_node *node)
pr_err("Failed to enable event: %s\n", p);
}
}
+
+static void __init
+trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
+{
+ struct trace_event_file *file;
+ struct property *prop;
+ char buf[MAX_BUF_LEN];
+ char *bufp;
+ const char *p;
+ int err;
+
+ if (!of_node_name_prefix(node, "event"))
+ return;
+
+ err = of_property_read_string(node, "event", &p);
+ if (err) {
+ pr_err("Failed to find event on %s\n", of_node_full_name(node));
+ return;
+ }
+
+ err = strlcpy(buf, p, ARRAY_SIZE(buf));
+ if (err >= ARRAY_SIZE(buf)) {
+ pr_err("Event name is too long: %s\n", p);
+ return;
+ }
+ bufp = buf;
+
+ p = strsep(&bufp, ":");
+ if (!bufp) {
+ pr_err("%s has no group name\n", buf);
+ return;
+ }
+
+ mutex_lock(&event_mutex);
+ file = find_event_file(tr, buf, bufp);
+ if (!file) {
+ pr_err("Failed to find event: %s\n", buf);
+ return;
+ }
+
+ err = of_property_read_string(node, "filter", &p);
+ if (!err) {
+ if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+ pr_err("filter string is too long: %s\n", p);
+ return;
+ }
+
+ if (apply_event_filter(file, buf) < 0) {
+ pr_err("Failed to apply filter: %s\n", buf);
+ return;
+ }
+ }
+
+ of_property_for_each_string(node, "actions", prop, p) {
+ if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+ pr_err("action string is too long: %s\n", p);
+ continue;
+ }
+
+ if (trigger_process_regex(file, buf) < 0)
+ pr_err("Failed to apply an action: %s\n", buf);
+ }
+
+ if (of_property_read_bool(node, "enable")) {
+ if (trace_event_enable_disable(file, 1, 0) < 0)
+ pr_err("Failed to enable event node: %s\n",
+ of_node_full_name(node));
+ }
+ mutex_unlock(&event_mutex);
+}
+
+static void __init
+trace_of_init_events(struct trace_array *tr, struct device_node *node)
+{
+ struct device_node *enode;
+
+ for_each_child_of_node(node, enode)
+ trace_of_init_one_event(tr, enode);
+}
#else
#define trace_of_enable_events(tr, node) do {} while (0)
+#define trace_of_init_events(tr, node) do {} while (0)
#endif
static void __init
@@ -139,6 +220,7 @@ static int __init trace_of_init(void)
goto end;
trace_of_set_ftrace_options(tr, trace_node);
+ trace_of_init_events(tr, trace_node);
trace_of_enable_events(tr, trace_node);
trace_of_enable_tracer(tr, trace_node);
^ permalink raw reply related
* [RFC PATCH v2 08/15] tracing: of: Add setup tracing by devicetree support
From: Masami Hiramatsu @ 2019-07-15 5:12 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Setup tracing options by devicetree instead of kernel parameters.
Since the kernel parameter is limited length, sometimes there is
no space to setup the tracing options. This will read the tracing
options from devicetree "ftrace" node and setup tracers at boot.
Note that this is not replacing the kernel parameters, because
this devicetree based setting is later than that. If you want to
trace earlier boot events, you still need kernel parameters.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Changes in v2:
- Move ftrace node under chosen node.
- Make events property available only when CONFIG_EVENT_TRACING=y
---
kernel/trace/Kconfig | 10 +++
kernel/trace/Makefile | 1
kernel/trace/trace.c | 38 ++++++++----
kernel/trace/trace_of.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 186 insertions(+), 13 deletions(-)
create mode 100644 kernel/trace/trace_of.c
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 07d22c61b634..025198bb2764 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -796,6 +796,16 @@ config GCOV_PROFILE_FTRACE
Note that on a kernel compiled with this config, ftrace will
run significantly slower.
+config OF_TRACING
+ bool "Boot Trace Programming by Devicetree"
+ depends on OF && TRACING
+ select OF_EARLY_FLATTREE
+ default y
+ help
+ Enable developer to setup ftrace subsystem via devicetree
+ at boot time for debugging (tracing) driver initialization
+ and boot process.
+
endif # FTRACE
endif # TRACING_SUPPORT
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index c2b2148bb1d2..6f68271f5c56 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -82,6 +82,7 @@ endif
obj-$(CONFIG_DYNAMIC_EVENTS) += trace_dynevent.o
obj-$(CONFIG_PROBE_EVENTS) += trace_probe.o
obj-$(CONFIG_UPROBE_EVENTS) += trace_uprobe.o
+obj-$(CONFIG_OF_TRACING) += trace_of.o
obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 30f09c058f79..6f50c58a3ad4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -158,7 +158,7 @@ union trace_eval_map_item {
static union trace_eval_map_item *trace_eval_maps;
#endif /* CONFIG_TRACE_EVAL_MAP_FILE */
-static int tracing_set_tracer(struct trace_array *tr, const char *buf);
+int tracing_set_tracer(struct trace_array *tr, const char *buf);
static void ftrace_trace_userstack(struct ring_buffer *buffer,
unsigned long flags, int pc);
@@ -178,6 +178,11 @@ static int __init set_cmdline_ftrace(char *str)
}
__setup("ftrace=", set_cmdline_ftrace);
+void __init trace_init_dump_on_oops(int mode)
+{
+ ftrace_dump_on_oops = mode;
+}
+
static int __init set_ftrace_dump_on_oops(char *str)
{
if (*str++ != '=' || !*str) {
@@ -4614,7 +4619,7 @@ int set_tracer_flag(struct trace_array *tr, unsigned int mask, int enabled)
return 0;
}
-static int trace_set_options(struct trace_array *tr, char *option)
+int trace_set_options(struct trace_array *tr, char *option)
{
char *cmp;
int neg = 0;
@@ -5505,8 +5510,8 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
return ret;
}
-static ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
- unsigned long size, int cpu_id)
+ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
+ unsigned long size, int cpu_id)
{
int ret = size;
@@ -5585,7 +5590,7 @@ static void add_tracer_options(struct trace_array *tr, struct tracer *t)
create_trace_option_files(tr, t);
}
-static int tracing_set_tracer(struct trace_array *tr, const char *buf)
+int tracing_set_tracer(struct trace_array *tr, const char *buf)
{
struct tracer *t;
#ifdef CONFIG_TRACER_MAX_TRACE
@@ -9160,16 +9165,23 @@ __init static int tracer_alloc_buffers(void)
return ret;
}
+void __init trace_init_tracepoint_printk(void)
+{
+ tracepoint_printk = 1;
+
+ tracepoint_print_iter =
+ kmalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
+ if (WARN_ON(!tracepoint_print_iter))
+ tracepoint_printk = 0;
+ else
+ static_key_enable(&tracepoint_printk_key.key);
+}
+
void __init early_trace_init(void)
{
- if (tracepoint_printk) {
- tracepoint_print_iter =
- kmalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
- if (WARN_ON(!tracepoint_print_iter))
- tracepoint_printk = 0;
- else
- static_key_enable(&tracepoint_printk_key.key);
- }
+ if (tracepoint_printk)
+ trace_init_tracepoint_printk();
+
tracer_alloc_buffers();
}
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
new file mode 100644
index 000000000000..5e108a1d5bb3
--- /dev/null
+++ b/kernel/trace/trace_of.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * trace_of.c
+ * devicetree tracing programming APIs
+ */
+
+#define pr_fmt(fmt) "trace_of: " fmt
+
+#include <linux/ftrace.h>
+#include <linux/init.h>
+#include <linux/of.h>
+
+#include "trace.h"
+
+#define MAX_BUF_LEN 256
+
+extern int trace_set_options(struct trace_array *tr, char *option);
+extern enum ftrace_dump_mode ftrace_dump_on_oops;
+extern int __disable_trace_on_warning;
+extern int tracing_set_tracer(struct trace_array *tr, const char *buf);
+extern void __init trace_init_tracepoint_printk(void);
+extern ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
+ unsigned long size, int cpu_id);
+
+static void __init
+trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
+{
+ struct property *prop;
+ const char *p;
+ char buf[MAX_BUF_LEN];
+ u32 v = 0;
+ int err;
+
+ /* Common ftrace options */
+ of_property_for_each_string(node, "options", prop, p) {
+ if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+ pr_err("String is too long: %s\n", p);
+ continue;
+ }
+
+ if (trace_set_options(tr, buf) < 0)
+ pr_err("Failed to set option: %s\n", buf);
+ }
+
+ err = of_property_read_string(node, "trace-clock", &p);
+ if (!err) {
+ if (tracing_set_clock(tr, p) < 0)
+ pr_err("Failed to set trace clock: %s\n", p);
+ }
+
+ /* Command line boot options */
+ if (of_find_property(node, "dump-on-oops", NULL)) {
+ err = of_property_read_u32_index(node, "dump-on-oops", 0, &v);
+ if (err || v == 1)
+ ftrace_dump_on_oops = DUMP_ALL;
+ else if (!err && v == 2)
+ ftrace_dump_on_oops = DUMP_ORIG;
+ }
+
+ if (of_find_property(node, "traceoff-on-warning", NULL))
+ __disable_trace_on_warning = 1;
+
+ if (of_find_property(node, "tp-printk", NULL))
+ trace_init_tracepoint_printk();
+
+ /* This accepts per-cpu buffer size in KB */
+ err = of_property_read_u32_index(node, "buffer-size-kb", 0, &v);
+ if (!err) {
+ v <<= 10; /* KB to Byte */
+ if (v < PAGE_SIZE)
+ pr_err("Buffer size is too small: %d KB\n", v >> 10);
+ if (tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0)
+ pr_err("Failed to resize trace buffer to %d KB\n",
+ v >> 10);
+ }
+
+ if (of_find_property(node, "alloc-snapshot", NULL))
+ if (tracing_alloc_snapshot() < 0)
+ pr_err("Failed to allocate snapshot buffer\n");
+}
+
+#ifdef CONFIG_EVENT_TRACING
+extern int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
+
+static void __init
+trace_of_enable_events(struct trace_array *tr, struct device_node *node)
+{
+ struct property *prop;
+ char buf[MAX_BUF_LEN];
+ const char *p;
+
+ of_property_for_each_string(node, "events", prop, p) {
+ if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+ pr_err("String is too long: %s\n", p);
+ continue;
+ }
+
+ if (ftrace_set_clr_event(tr, buf, 1) < 0)
+ pr_err("Failed to enable event: %s\n", p);
+ }
+}
+#else
+#define trace_of_enable_events(tr, node) do {} while (0)
+#endif
+
+static void __init
+trace_of_enable_tracer(struct trace_array *tr, struct device_node *node)
+{
+ const char *p;
+
+ if (!of_property_read_string(node, "tracer", &p)) {
+ if (tracing_set_tracer(tr, p) < 0)
+ pr_err("Failed to set given tracer: %s\n", p);
+
+ }
+}
+
+static struct device_node * __init trace_of_find_ftrace_node(void)
+{
+ if (!of_chosen)
+ return NULL;
+
+ return of_find_node_by_name(of_chosen, "linux,ftrace");
+}
+
+static int __init trace_of_init(void)
+{
+ struct device_node *trace_node;
+ struct trace_array *tr;
+
+ trace_node = trace_of_find_ftrace_node();
+ if (!trace_node)
+ return 0;
+
+ trace_node = of_node_get(trace_node);
+
+ tr = top_trace_array();
+ if (!tr)
+ goto end;
+
+ trace_of_set_ftrace_options(tr, trace_node);
+ trace_of_enable_events(tr, trace_node);
+ trace_of_enable_tracer(tr, trace_node);
+
+end:
+ of_node_put(trace_node);
+ return 0;
+}
+
+fs_initcall(trace_of_init);
^ permalink raw reply related
* [RFC PATCH v2 07/15] dt-bindings: tracing: Add ftrace binding document
From: Masami Hiramatsu @ 2019-07-15 5:12 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Add a devicetree binding document for ftrace node.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Changes in v2:
- Add cpumask, ftrace-filters, ftrace-notraces, fgraph-filters,
fgraph-notraces, fgraph-max-depth and instance node.
- Remove compatible and move file to bindings/chosen/linux,ftrace.yaml
---
.../devicetree/bindings/chosen/linux,ftrace.yaml | 306 ++++++++++++++++++++
1 file changed, 306 insertions(+)
create mode 100644 Documentation/devicetree/bindings/chosen/linux,ftrace.yaml
diff --git a/Documentation/devicetree/bindings/chosen/linux,ftrace.yaml b/Documentation/devicetree/bindings/chosen/linux,ftrace.yaml
new file mode 100644
index 000000000000..a5c60ac2f66d
--- /dev/null
+++ b/Documentation/devicetree/bindings/chosen/linux,ftrace.yaml
@@ -0,0 +1,306 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Linaro Ltd.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/tracing/ftrace.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Ftrace setting devicetree binding
+
+maintainers:
+ - Masami Hiramatsu <mhiramat@kernel.org>
+
+description: |
+ Boot-time ftrace tracing setting via devicetree. Users can use devicetree node
+ for programming ftrace boot-time tracing. This node must be placed at
+ /chosen/linux,ftrace.
+
+properties:
+ dump-on-oops:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - enum: [0, 1, 2]
+ description: |
+ A neumerical flag to enable ftrace dump on Kernel Oops. 0 means no dump,
+ 1 means dump on the origin cpu of the oops, and means dump on all cpus.
+
+ traceoff-on-warning:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: A flag to stop tracing on warning.
+
+ tp-printk:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: A flag to send tracing output to printk buffer too.
+
+ alloc-snapshot:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ A flag to allocate snapshot buffer at boot. This will be required if you
+ use "snapshot" action on some events.
+
+ trace-clock:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/string
+ - enum: [ global, local, counter, uptime, perf, mono, mono_raw, boot, ppc-tb, x86-tsc ]
+ description: Specify which clock method is used for trace-clock.
+
+ buffer-size-kb:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - minimum: 1
+ description: |
+ The size of per-cpu tracing buffer in KByte. Note that the size must be
+ larger than page size, and total size of buffers depends on the number
+ of CPUs.
+
+ events:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of enabling events (including wildcard patterns).
+ See Documentation/trace/events.rst for detail.
+
+ options:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of trace options for ftrace to control what gets printed
+ in the trace output or manipulate the tracers.
+ See Documentation/trace/ftrace.rst#trace_options for detail.
+
+ tracer:
+ default: nop
+ $ref: /schemas/types.yaml#/definitions/string
+ description: A string of the tracer to start up.
+
+ cpumask:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: |
+ A hexadecimal number of the cpu-mask value which is given as a string.
+ This is because the number of cores can be bigger than 64.
+
+ ftrace-filters:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of the functions traced by the function tracer at boot
+ up. The function can be given with wildcards. This list can be
+ specified in each instance.
+
+ ftrace-notraces:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of the functions NOT traced by the function tracer at
+ boot up. The function can be given with wildcards. This list can be
+ specified in each instance.
+
+ fgraph-filters:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of the top level callers functions traced by the
+ function graph tracer at boot up. The function can be given with
+ wildcards. This list is not available in instance node.
+
+ fgraph-notraces:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of the top level callers functions NOT traced by the
+ function graph tracer at boot up. The function can be given with
+ wildcards. This list is not available in instance node.
+
+ fgraph-max-depth:
+ default: 0
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ This is the max depth, the function graph tracer will trace into
+ a function. 0 means no limit for the trace depth.
+
+patternProperties:
+ "event[0-9a-fA-F]+$":
+ type: object
+
+ description: |
+ event* properties are child nodes for per-event settings. It is also
+ able to define new kprobe events and synthetic events. Note that you
+ can not define both "probes" and "fields" properties on same event.
+
+ properties:
+ event:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: |
+ Event name string formatted as "GROUP:EVENT". For synthetic event,
+ you must use "synthetic" for group name. For kprobe and synthetic
+ event, you can ommit the group name.
+
+ probes:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of kprobe event definitions, including location
+ (symbol+offset) and event arguments.
+ See Documentation/trace/kprobetrace.rst for detail.
+
+ fields:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string of synthetic event's fields definition. Note that you
+ don't need to repeat event name.
+
+ filter:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: A string of event filter rule
+
+ actions:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: A string array of event trigger actions.
+
+ enable:
+ type: boolean
+ description: |
+ A flag to enable event. Note that the event is not enabled by
+ default. (But actions will set the event soft-disabled)
+
+ oneOf:
+ - required:
+ - event
+ - required:
+ - event
+ - probes
+ - required:
+ - event
+ - fields
+
+ "instance[0-9a-fA-F]+$":
+ type: object
+
+ description: |
+ instance* properties are child nodes for per-instance settings.
+ It is also able to have event nodes as linux,ftrace node does.
+
+ properties:
+ instance:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: The name of new instance.
+
+ trace-clock:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/string
+ - enum: [ global, local, counter, uptime, perf, mono, mono_raw, boot, ppc-tb, x86-tsc ]
+ description: Specify which clock method is used for trace-clock.
+
+ buffer-size-kb:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - minimum: 1
+ description: |
+ The size of per-cpu tracing buffer in KByte. Note that the size must be
+ larger than page size, and total size of buffers depends on the number
+ of CPUs.
+
+ events:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of enabling events (including wildcard patterns).
+ See Documentation/trace/events.rst for detail.
+
+ options:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of trace options for ftrace to control what gets printed
+ in the trace output or manipulate the tracers.
+ See Documentation/trace/ftrace.rst#trace_options for detail.
+
+ tracer:
+ default: nop
+ $ref: /schemas/types.yaml#/definitions/string
+ description: A string of the tracer to start up.
+
+ cpumask:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: |
+ A hexadecimal number of the cpu-mask value which is given as a string.
+ This is because the number of cores can be bigger than 64.
+
+ ftrace-filters:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of the functions traced by the function tracer at boot
+ up. The function can be given with wildcards. This list can be
+ specified in each instance.
+
+ ftrace-notraces:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of the functions NOT traced by the function tracer at
+ boot up. The function can be given with wildcards. This list can be
+ specified in each instance.
+
+ fgraph-filters:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of the top level callers functions traced by the
+ function graph tracer at boot up. The function can be given with
+ wildcards. This list is not available in instance node.
+
+ fgraph-notraces:
+ minItems: 1
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description: |
+ A string array of the top level callers functions NOT traced by the
+ function graph tracer at boot up. The function can be given with
+ wildcards. This list is not available in instance node.
+
+ fgraph-max-depth:
+ default: 0
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ This is the max depth, the function graph tracer will trace into
+ a function. 0 means no limit for the trace depth.
+
+ required:
+ - instance
+
+examples:
+ - |
+ ftrace {
+ events = "initcall:*";
+ tp-printk;
+ buffer-size-kb = <0x400>;
+ event1 {
+ event = "kprobes:vfs_read";
+ probes = "vfs_read $arg1 $arg2";
+ filter = "common_pid < 200";
+ };
+ event2 {
+ event = "initcall_latency";
+ fields = "unsigned long func", "u64 lat";
+ actions = "hist:keys=func.sym,lat:vals=lat:sort=lat";
+ };
+ event3 {
+ event = "initcall:initcall_start";
+ actions = "hist:keys=func:ts0=common_timestamp.usecs";
+ };
+ event4 {
+ event = "initcall:initcall_finish";
+ actions = "hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(initcall.initcall_start).initcall_latency(func,$lat)";
+ };
+ instance0 {
+ buffer-size-kb = <0x100>;
+ event5 {
+ event = "task:task_newtask";
+ filter = "pid < 128";
+ enable;
+ };
+ };
+
+ };
^ permalink raw reply related
* [RFC PATCH v2 06/15] tracing: Add NULL trace-array check in print_synth_event()
From: Masami Hiramatsu @ 2019-07-15 5:12 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Add NULL trace-array check in print_synth_event(), because
if we enable tp_printk option, iter->tr can be NULL.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_events_hist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index a7f447195143..db973928e580 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -822,7 +822,7 @@ static enum print_line_t print_synth_event(struct trace_iterator *iter,
fmt = synth_field_fmt(se->fields[i]->type);
/* parameter types */
- if (tr->trace_flags & TRACE_ITER_VERBOSE)
+ if (tr && tr->trace_flags & TRACE_ITER_VERBOSE)
trace_seq_printf(s, "%s ", fmt);
snprintf(print_fmt, sizeof(print_fmt), "%%s=%s%%s", fmt);
^ permalink raw reply related
* [RFC PATCH v2 05/15] tracing: Accept different type for synthetic event fields
From: Masami Hiramatsu @ 2019-07-15 5:12 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Make the synthetic event accepts a different type field to record.
However, the size and signed flag must be same.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_events_hist.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index ca6b0dff60c5..a7f447195143 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -4063,8 +4063,11 @@ static int check_synth_field(struct synth_event *event,
field = event->fields[field_pos];
- if (strcmp(field->type, hist_field->type) != 0)
- return -EINVAL;
+ if (strcmp(field->type, hist_field->type) != 0) {
+ if (field->size != hist_field->size ||
+ field->is_signed != hist_field->is_signed)
+ return -EINVAL;
+ }
return 0;
}
^ permalink raw reply related
* [RFC PATCH v2 04/15] tracing: kprobes: Register to dynevent earlier stage
From: Masami Hiramatsu @ 2019-07-15 5:11 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Register kprobe event to dynevent in subsys_initcall level.
This will allow kernel to register new kprobe events in
fs_initcall level via trace_run_command.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_kprobe.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 6c5145525f90..5135c07b6557 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1484,11 +1484,12 @@ static __init void setup_boot_kprobe_events(void)
enable_boot_kprobe_events();
}
-/* Make a tracefs interface for controlling probe points */
-static __init int init_kprobe_trace(void)
+/*
+ * Register dynevent at subsys_initcall. This allows kernel to setup kprobe
+ * events in fs_initcall without tracefs.
+ */
+static __init int init_kprobe_trace_early(void)
{
- struct dentry *d_tracer;
- struct dentry *entry;
int ret;
ret = dyn_event_register(&trace_kprobe_ops);
@@ -1498,6 +1499,16 @@ static __init int init_kprobe_trace(void)
if (register_module_notifier(&trace_kprobe_module_nb))
return -EINVAL;
+ return 0;
+}
+subsys_initcall(init_kprobe_trace_early);
+
+/* Make a tracefs interface for controlling probe points */
+static __init int init_kprobe_trace(void)
+{
+ struct dentry *d_tracer;
+ struct dentry *entry;
+
d_tracer = tracing_init_dentry();
if (IS_ERR(d_tracer))
return 0;
^ permalink raw reply related
* [RFC PATCH v2 03/15] tracing: Expose EXPORT_SYMBOL_GPL symbol
From: Masami Hiramatsu @ 2019-07-15 5:11 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Since ftrace_set_clr_event is already exported by EXPORT_SYMBOL_GPL,
it should not be static.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 22cf08bd2317..c2d38048edae 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -788,7 +788,7 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
return ret;
}
-static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
+int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
{
char *event = NULL, *sub = NULL, *match;
int ret;
^ permalink raw reply related
* [RFC PATCH v2 02/15] tracing: kprobes: Output kprobe event to printk buffer
From: Masami Hiramatsu @ 2019-07-15 5:11 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Since kprobe-events use event_trigger_unlock_commit_regs() directly,
that events doesn't show up in printk buffer if "tp_printk" is set.
Use trace_event_buffer_commit() in kprobe events so that it can
invoke output_printk() as same as other trace events.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
include/linux/trace_events.h | 1 +
kernel/trace/trace.c | 4 +--
kernel/trace/trace_events.c | 1 +
kernel/trace/trace_kprobe.c | 57 +++++++++++++++++++++---------------------
4 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 5150436783e8..8912ccdb3d4b 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -211,6 +211,7 @@ struct trace_event_buffer {
void *entry;
unsigned long flags;
int pc;
+ struct pt_regs *regs;
};
void *trace_event_buffer_reserve(struct trace_event_buffer *fbuffer,
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 047f37816bf1..30f09c058f79 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2572,9 +2572,9 @@ void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
if (static_key_false(&tracepoint_printk_key.key))
output_printk(fbuffer);
- event_trigger_unlock_commit(fbuffer->trace_file, fbuffer->buffer,
+ event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
fbuffer->event, fbuffer->entry,
- fbuffer->flags, fbuffer->pc);
+ fbuffer->flags, fbuffer->pc, fbuffer->regs);
}
EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c7506bc81b75..22cf08bd2317 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -271,6 +271,7 @@ void *trace_event_buffer_reserve(struct trace_event_buffer *fbuffer,
if (!fbuffer->event)
return NULL;
+ fbuffer->regs = NULL;
fbuffer->entry = ring_buffer_event_data(fbuffer->event);
return fbuffer->entry;
}
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 9d483ad9bb6c..6c5145525f90 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -988,10 +988,8 @@ __kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs,
struct trace_event_file *trace_file)
{
struct kprobe_trace_entry_head *entry;
- struct ring_buffer_event *event;
- struct ring_buffer *buffer;
- int size, dsize, pc;
- unsigned long irq_flags;
+ struct trace_event_buffer fbuffer;
+ int dsize;
struct trace_event_call *call = trace_probe_event_call(&tk->tp);
WARN_ON(call != trace_file->event_call);
@@ -999,24 +997,26 @@ __kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs,
if (trace_trigger_soft_disabled(trace_file))
return;
- local_save_flags(irq_flags);
- pc = preempt_count();
+ local_save_flags(fbuffer.flags);
+ fbuffer.pc = preempt_count();
+ fbuffer.trace_file = trace_file;
dsize = __get_data_size(&tk->tp, regs);
- size = sizeof(*entry) + tk->tp.size + dsize;
- event = trace_event_buffer_lock_reserve(&buffer, trace_file,
- call->event.type,
- size, irq_flags, pc);
- if (!event)
+ fbuffer.event =
+ trace_event_buffer_lock_reserve(&fbuffer.buffer, trace_file,
+ call->event.type,
+ sizeof(*entry) + tk->tp.size + dsize,
+ fbuffer.flags, fbuffer.pc);
+ if (!fbuffer.event)
return;
- entry = ring_buffer_event_data(event);
+ fbuffer.regs = regs;
+ entry = fbuffer.entry = ring_buffer_event_data(fbuffer.event);
entry->ip = (unsigned long)tk->rp.kp.addr;
store_trace_args(&entry[1], &tk->tp, regs, sizeof(*entry), dsize);
- event_trigger_unlock_commit_regs(trace_file, buffer, event,
- entry, irq_flags, pc, regs);
+ trace_event_buffer_commit(&fbuffer);
}
static void
@@ -1036,10 +1036,8 @@ __kretprobe_trace_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
struct trace_event_file *trace_file)
{
struct kretprobe_trace_entry_head *entry;
- struct ring_buffer_event *event;
- struct ring_buffer *buffer;
- int size, pc, dsize;
- unsigned long irq_flags;
+ struct trace_event_buffer fbuffer;
+ int dsize;
struct trace_event_call *call = trace_probe_event_call(&tk->tp);
WARN_ON(call != trace_file->event_call);
@@ -1047,25 +1045,26 @@ __kretprobe_trace_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
if (trace_trigger_soft_disabled(trace_file))
return;
- local_save_flags(irq_flags);
- pc = preempt_count();
+ local_save_flags(fbuffer.flags);
+ fbuffer.pc = preempt_count();
+ fbuffer.trace_file = trace_file;
dsize = __get_data_size(&tk->tp, regs);
- size = sizeof(*entry) + tk->tp.size + dsize;
-
- event = trace_event_buffer_lock_reserve(&buffer, trace_file,
- call->event.type,
- size, irq_flags, pc);
- if (!event)
+ fbuffer.event =
+ trace_event_buffer_lock_reserve(&fbuffer.buffer, trace_file,
+ call->event.type,
+ sizeof(*entry) + tk->tp.size + dsize,
+ fbuffer.flags, fbuffer.pc);
+ if (!fbuffer.event)
return;
- entry = ring_buffer_event_data(event);
+ fbuffer.regs = regs;
+ entry = fbuffer.entry = ring_buffer_event_data(fbuffer.event);
entry->func = (unsigned long)tk->rp.kp.addr;
entry->ret_ip = (unsigned long)ri->ret_addr;
store_trace_args(&entry[1], &tk->tp, regs, sizeof(*entry), dsize);
- event_trigger_unlock_commit_regs(trace_file, buffer, event,
- entry, irq_flags, pc, regs);
+ trace_event_buffer_commit(&fbuffer);
}
static void
^ permalink raw reply related
* [RFC PATCH v2 01/15] tracing: Apply soft-disabled and filter to tracepoints printk
From: Masami Hiramatsu @ 2019-07-15 5:11 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
In-Reply-To: <156316746861.23477.5815110570539190650.stgit@devnote2>
Apply soft-disabled and the filter rule of the trace events to
the printk output of tracepoints (a.k.a. tp_printk kernel parameter)
as same as trace buffer output.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6b62e1718548..047f37816bf1 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2502,6 +2502,7 @@ static DEFINE_MUTEX(tracepoint_printk_mutex);
static void output_printk(struct trace_event_buffer *fbuffer)
{
struct trace_event_call *event_call;
+ struct trace_event_file *file;
struct trace_event *event;
unsigned long flags;
struct trace_iterator *iter = tracepoint_print_iter;
@@ -2515,6 +2516,12 @@ static void output_printk(struct trace_event_buffer *fbuffer)
!event_call->event.funcs->trace)
return;
+ file = fbuffer->trace_file;
+ if (test_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags) ||
+ (unlikely(file->flags & EVENT_FILE_FL_FILTERED) &&
+ !filter_match_preds(file->filter, fbuffer->entry)))
+ return;
+
event = &fbuffer->trace_file->event_call->event;
spin_lock_irqsave(&tracepoint_iter_lock, flags);
^ permalink raw reply related
* [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree
From: Masami Hiramatsu @ 2019-07-15 5:11 UTC (permalink / raw)
To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
Tom Zanussi, linux-kernel, devicetree
Hello,
Here is the 2nd version of RFC series to add boot-time tracing using
devicetree. Previous thread is here.
https://lkml.kernel.org/r/156113387975.28344.16009584175308192243.stgit@devnote2
In this version, I moved the ftrace node under /chosen/linux,ftrace
and remove compatible property, because it must be in fixed place.
Also this version has following features;
- Introduce "instance" node, which can have events nodes for setting
events filters and actions for the trace instance.
- Introduce "cpumask" property
- Introduce "ftrace-filters" and "ftrace-notraces"
- Introduce "fgraph-filters", "fgraph-notraces" and "fgraph-max-depth"
At this moment, this feature is only available on the architecutre which
supports devicetree. For x86, we can use it on qemu with --dtb option,
or apply below patch on grub to add devicetree support on grub-x86.
https://github.com/mhiramat/grub/commit/644c35bfd2d18c772cc353b74215344f8264923a
Note that the devicetree for x86 must contain the nodes only under
/chosen/, or it may cause a problem if it conflicts with ACPI.
(Maybe we need to disable the FDT nodes except for nodes under /chosen
on boot if ACPI exists.)
This series can be applied on Steve's tracing tree (ftrace/core) or
available on below
https://github.com/mhiramat/linux.git ftrace-devicetree-v2
Usage
======
With this series, we can setup new kprobe and synthetic events, more
complicated event filters and trigger actions including histogram
via devicetree.
For example, following kernel parameters
trace_options=sym-addr trace_event=initcall:* tp_printk trace_buf_size=1M ftrace=function ftrace_filter="vfs*"
it can be written in devicetree like below.
/{
chosen {
...
ftrace {
options = "sym-addr";
events = "initcall:*";
tp-printk;
buffer-size-kb = <0x400>; // 1024KB == 1MB
ftrace-filters = "vfs*";
};
Moreover, now we can expand it to add filters for events, kprobe events,
and synthetic events with histogram like below.
ftrace {
...
event0 {
event = "task:task_newtask";
filter = "pid < 128"; // adding filters
enable;
};
event1 {
event = "kprobes:vfs_read";
probes = "vfs_read $arg1 $arg2"; // add kprobes
filter = "common_pid < 200";
enable;
};
event2 {
event = "initcall_latency"; // add synth event
fields = "unsigned long func", "u64 lat";
// with histogram
actions = "hist:keys=func.sym,lat:vals=lat:sort=lat";
};
// and synthetic event callers
event3 {
event = "initcall:initcall_start";
actions = "hist:keys=func:ts0=common_timestamp.usecs";
};
event4 {
event = "initcall:initcall_finish";
actions = "hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(initcall.initcall_start).initcall_latency(func,$lat)";
};
};
Also, this version supports "instance" node, which allows us to
run several tracers for different purpose at once. For example,
one tracer is for tracing functions in module alpha, and others
tracing module beta, you can write followings.
ftrace {
instance0 {
instance = "foo";
tracer = "function";
ftrace-filters = "*:mod:alpha";
};
instance1 {
instance = "bar";
tracer = "function";
ftrace-filters = "*:mod:beta";
};
};
The instance node also accepts event nodes so that each instance
can customize its event tracing.
Discussion
=====
On the previous thread, we discussed that the this devicetree usage
itself was acceptable or not. Fortunately, I had a chance to discuss
it in a F2F meeting with Frank and Tim last week.
I think the advantages of using devicetree are,
- reuse devicetree's structured syntax for complicated tracefs settings
- reuse OF-APIs in linux kernel to accept and parse it
- reuse dtc complier to compile it and validate syntax. (with yaml schema,
we can enhance it)
- reuse current bootloader (and qemu) to load it
And we talked about some other ideas to avoid using devicetree.
- expand kernel command line (ascii command strings)
- expand kernel command line with base64 encoded comressed ascii command
strings
- load (compressed) ascii command strings to somewhere on memory and pass
the address via kernel cmdline
- load (compressed) ascii command strings to somewhere on memory and pass
the address via /chosen node (as same as initrd)
- load binary C data and point it from kernel cmdline
- load binary C data and point it from /chosen node (as same as initrd)
- load binary C data as a section of kernel image
The first 2 ideas expand the kernel's cmdline to pass some "magic" command
to setup ftrace. In both case, the problems are the maximal size of cmdline
and the issues related to the complexity of commands.
My example showed that the ftrace settings becomes long even if making one
histogram, which can be longer than 256 bytes. The long and complex data
can easily lead mis-typing, but cmdline has no syntax validator, it just
ignores the mis-typed commands.
(Of course even with the devicetree, it must be smaller than 2 pages)
Next 2 ideas are similar, but load the commands on some other memory area
and pass only address via cmdline. This solves the size limitation issue,
but still no syntax validation. Of course we can make a new structured
syntax validator similar to (or just forked from) dt-validate.
The problem (or disadvantage) of these (and following) ideas, is to change
the kernel and boot loaders to load another binary blobs on memory.
Maybe if we introduce a generic structured kernel boot arguments, which is
a kind of /chosen node of devicetree. (But if there is already such hook,
why we make another one...?)
Also, this "GSKBA" may introduce a parser and access APIs which will be
very similar to OF-APIs. This also seems redundant to me.
So the last 3 ideas will avoid introducing new parser and APIs, we just
compile the data as C data and point it from cmdline or somewhere else.
With these ideas, we still need to expand boot loaders to support
loading new binary blobs. (And the last one requires to add elf header
parser/modifier to boot loader too)
>From the above reasons, I think using devicetree's /chosen node is
the least intrusive way to introduce this boot-time tracing feature.
Any suggestions, thoughts?
Thank you,
---
Masami Hiramatsu (15):
tracing: Apply soft-disabled and filter to tracepoints printk
tracing: kprobes: Output kprobe event to printk buffer
tracing: Expose EXPORT_SYMBOL_GPL symbol
tracing: kprobes: Register to dynevent earlier stage
tracing: Accept different type for synthetic event fields
tracing: Add NULL trace-array check in print_synth_event()
dt-bindings: tracing: Add ftrace binding document
tracing: of: Add setup tracing by devicetree support
tracing: of: Add trace event settings
tracing: of: Add kprobe event support
tracing: of: Add synthetic event support
tracing: of: Add instance node support
tracing: of: Add cpumask property support
tracing: of: Add function tracer filter properties
tracing: of: Add function-graph tracer option properties
.../devicetree/bindings/chosen/linux,ftrace.yaml | 306 ++++++++++++
include/linux/trace_events.h | 1
kernel/trace/Kconfig | 10
kernel/trace/Makefile | 1
kernel/trace/ftrace.c | 85 ++-
kernel/trace/trace.c | 90 ++--
kernel/trace/trace_events.c | 3
kernel/trace/trace_events_hist.c | 14 -
kernel/trace/trace_events_trigger.c | 2
kernel/trace/trace_kprobe.c | 81 ++-
kernel/trace/trace_of.c | 507 ++++++++++++++++++++
11 files changed, 1004 insertions(+), 96 deletions(-)
create mode 100644 Documentation/devicetree/bindings/chosen/linux,ftrace.yaml
create mode 100644 kernel/trace/trace_of.c
--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: mmc: Document Aspeed SD controller
From: Andrew Jeffery @ 2019-07-15 2:30 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-mmc, Ulf Hansson, Rob Herring, mark.rutland, Joel Stanley,
Adrian Hunter, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, Ryan Chen
In-Reply-To: <20190712131028.ba4d4jetg4btsx4u@flea>
On Fri, 12 Jul 2019, at 22:41, Maxime Ripard wrote:
> Hi,
>
> On Fri, Jul 12, 2019 at 01:02:13PM +0930, Andrew Jeffery wrote:
> > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the
> > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit
> > data bus if only a single slot is enabled.
> >
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > In v2:
> >
> > * Rename to aspeed,sdhci.yaml
> > * Rename sd-controller compatible
> > * Add `maxItems: 1` for reg properties
> > * Move sdhci subnode description to patternProperties
> > * Drop sdhci compatible requirement
> > * #address-cells and #size-cells are required
> > * Prevent additional properties
> > * Implement explicit ranges in example
> > * Remove slot property
> >
> > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++
> > 1 file changed, 90 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > new file mode 100644
> > index 000000000000..67a691c3348c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > @@ -0,0 +1,90 @@
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ASPEED SD/SDIO/eMMC Controller
> > +
> > +maintainers:
> > + - Andrew Jeffery <andrew@aj.id.au>
> > + - Ryan Chen <ryanchen.aspeed@gmail.com>
> > +
> > +description: |+
> > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO
> > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if
> > + only a single slot is enabled.
> > +
> > + The two slots are supported by a common configuration area. As the SDHCIs for
> > + the slots are dependent on the common configuration area, they are described
> > + as child nodes.
> > +
> > +properties:
> > + compatible:
> > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ]
> > + reg:
> > + maxItems: 1
> > + description: Common configuration registers
> > + ranges: true
> > + clocks:
> > + maxItems: 1
> > + description: The SD/SDIO controller clock gate
>
> #address-cells and #size-cells have not been described here.
>
> > +patternProperties:
> > + "^sdhci@[0-9a-f]+$":
> > + type: object
> > + properties:
> > + compatible:
> > + enum: [ aspeed,ast2400-sdhci, aspeed,ast2500-sdhci ]
> > + reg:
> > + maxItems: 1
> > + description: The SDHCI registers
> > + clocks:
> > + maxItems: 1
> > + description: The SD bus clock
> > + interrupts:
> > + maxItems: 1
> > + description: The SD interrupt shared between both slots
> > + required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - interrupts
> > +
> > +additionalProperties: false
>
> But that means that it will generate a warning in your DT if you ever
> use them.
>
> > +required:
> > + - compatible
> > + - reg
> > + - "#address-cells"
> > + - "#size-cells"
> > + - ranges
> > + - clocks
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/aspeed-clock.h>
> > + sdc@1e740000 {
> > + compatible = "aspeed,ast2500-sd-controller";
> > + reg = <0x1e740000 0x100>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
>
> Starting with your example.
Heh, right. Thanks. I was inspecting the output of the `dt_binding_check` and
`dtbs_check` make targets, though maybe I overlooked this. The aspeed dtsis
do generate a quite a number of warnings which make it hard to parse, so I'm
going to send a series to clean that up too.
Andrew
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
> Attachments:
> * signature.asc
^ permalink raw reply
* Re: [PATCH 2/3 v2] dt-bindings: gpio: aspeed: Add SGPIO support
From: Andrew Jeffery @ 2019-07-15 2:25 UTC (permalink / raw)
To: Hongwei Zhang, Joel Stanley, Linus Walleij, devicetree
Cc: Mark Rutland, linux-aspeed, linux-gpio, linux-kernel,
Bartosz Golaszewski, Rob Herring, linux-arm-kernel
In-Reply-To: <1562962440-15908-1-git-send-email-hongweiz@ami.com>
Hello Hongwei,
On Sat, 13 Jul 2019, at 05:44, Hongwei Zhang wrote:
> Add bindings to support SGPIO on AST2400 or AST2500.
>
> Signed-off-by: Hongwei Zhang <hongweiz@ami.com>
> ---
> .../devicetree/bindings/gpio/sgpio-aspeed.txt | 43 ++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
> create mode 100755 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> new file mode 100755
> index 0000000..3ae2b79
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> @@ -0,0 +1,43 @@
> +Aspeed SGPIO controller Device Tree Bindings
> +-------------------------------------------
> +
> +Required properties:
> +- compatible : Either "aspeed,ast2400-sgpio" or "aspeed,ast2500-sgpio"
> +
> +- #gpio-cells : Should be two
> + - First cell is the GPIO line number
> + - Second cell is used to specify optional
> + parameters (unused)
> +
> +- reg : Address and length of the register set for the device
> +- gpio-controller : Marks the device node as a GPIO controller.
> +- interrupts : Interrupt specifier (see interrupt bindings for
> + details)
> +
> +- interrupt-controller : Mark the GPIO controller as an
> interrupt-controller
> +
> +- nr-gpios : number of GPIO pins to serialise. (should be multiple of
> 8, up to 80 pins)
> + if not specified, defaults to 80.
This appears to be a statement about the driver implementation, but bindings
documents are about describing hardware. Reading the datasheet it actually
appears the ASPEED SGPIO hardware comes up in what is "technically" a
forbidden state (equivalent to `nr-gpios = <0>;`), though the device is also
disabled at this point, so it's probably moot. The point is the true default
value from a hardware perspective is 0, not 80, so if we're going to talk about
default values, 0 would be more appropriate. However:
You've also listed nr-gpios under the "Required properties" header, but the
description suggests it's optional. It's either one or the other, please lets be
clear about it. On that front, lets make it nr-gpios *not* optional (i.e. make it
required) thus force the specification of how many SGPIOs we want to emit
on the bus. This value is coupled to the platform design, so I don't think
there's ever a scenario where we want nr-gpios to take a default value.
> +
> +- clocks : A phandle to the APB clock for SGPM clock
> division
> +
> +- bus-frequency : SGPM CLK frequency, if not specified defaults to 1
> MHz
Again here with the default value - SGPM CLK period is derived from PCLK by
the expression `period = PCLK * 2 *(GPIO254[31:16] + 1)`, where GPIO254's
initialisation state is `GPIO254[31:16] = 0`, which gives a default SGPM bus
frequency of PCLK / 2. This is likely not going to be 1MHz (more like ~12MHz).
Lets just make the property required. That way we avoid any ambiguity about
the bus frequency and thus don't need words about defaults that turn out to
be about the driver, not about the hardware.
Finally, when updating patches in response to feedback, please send the full
series again, and bump the series version number. That way people can review
a coherent set of patches and not have to hunt around and (fail to) collate the
correct combination. It makes it easier to say "Reviewed-by:" on your patches :)
Cheers,
Andrew
^ permalink raw reply
* Re: [PATCH 4/8] PCI: Add quirk to disable MSI support for Amazon's Annapurna Labs host bridge
From: Benjamin Herrenschmidt @ 2019-07-14 22:54 UTC (permalink / raw)
To: Chocron, Jonathan, helgaas@kernel.org
Cc: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
jingoohan1@gmail.com, Woodhouse, David, Hanoch, Uri,
devicetree@vger.kernel.org, lorenzo.pieralisi@arm.com,
gustavo.pimentel@synopsys.com, Wasserstrom, Barak, Saidi, Ali,
mark.rutland@arm.com, Hawa, Hanna, Shenhar, Talel, Krupnik, Ronen,
linux-pci@vger.kernel.org
In-Reply-To: <2cac43401f0cf76acb645b98c6543204f12d5c05.camel@amazon.com>
On Sun, 2019-07-14 at 15:09 +0000, Chocron, Jonathan wrote:
> > s/host bridge/Root Port/, if I understand correctly.
> >
>
> Ack.
>
> BTW, what is the main difference between the 2 terms, since they seem
> to be (mistakenly?) used interchangeably?
The host bridge is the parent of the root port. You can have several
root ports under a host bridge in fact. They tend to be part of the
same silicon and somewhat intimately linked but they are distinct
logical entities. The root port appears as a PCIe p2p bridge sitting on
the top level bus provided by the host bridge. The Host Bridge doesn't
have to have a representation in config space (it sometimes does
historically, but as a sibling of the devices on that top level bus. In
PCIe land, these are chipset built-in devices).
Ben.
^ 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