[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5ea36d22-d6c2-92a2-77e4-f9017d061f50@partner.samsung.com>
Date: Fri, 19 Apr 2019 15:34:16 +0200
From: Lukasz Luba <l.luba@...tner.samsung.com>
To: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Cc: b.zolnierkie@...sung.com, krzk@...nel.org, robh+dt@...nel.org,
mark.rutland@....com, cw00.choi@...sung.com,
kyungmin.park@...sung.com, m.szyprowski@...sung.com,
s.nawrocki@...sung.com, myungjoo.ham@...sung.com, kgene@...nel.org,
willy.mh.wolff.ml@...il.com
Subject: Re: [PATCH v2 2/4] drivers: devfreq: events: extend events by type
of counted data
Hi Chanwoo,
please ignore this patch there is a comment below.
It wrongly sets the default value.
On 4/18/19 3:38 PM, Lukasz Luba wrote:
> This patch adds posibility to choose what type of data should be counted
> by the PPMU counter. Now the type comes from DT where the event has been
> defined. When there is no 'event-data-type' the default value is used,
> which is 'read data in bytes'.
> It is needed when you want to know not only read+write data bytes but
> i.e. only write data in byte, or number of read requests, etc.
>
> Signed-off-by: Lukasz Luba <l.luba@...tner.samsung.com>
> ---
> drivers/devfreq/event/exynos-ppmu.c | 61 +++++++++++++++++++++++++------------
> include/linux/devfreq-event.h | 6 ++++
> 2 files changed, 48 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c
> index c61de0b..9f12201 100644
> --- a/drivers/devfreq/event/exynos-ppmu.c
> +++ b/drivers/devfreq/event/exynos-ppmu.c
> @@ -154,9 +154,9 @@ static int exynos_ppmu_set_event(struct devfreq_event_dev *edev)
> if (ret < 0)
> return ret;
>
> - /* Set the event of Read/Write data count */
> + /* Set the event of proper data type monitoring */
> ret = regmap_write(info->regmap, PPMU_BEVTxSEL(id),
> - PPMU_RO_DATA_CNT | PPMU_WO_DATA_CNT);
> + edev->desc->data_type);
> if (ret < 0)
> return ret;
>
> @@ -368,23 +368,11 @@ static int exynos_ppmu_v2_set_event(struct devfreq_event_dev *edev)
> if (ret < 0)
> return ret;
>
> - /* Set the event of Read/Write data count */
> - switch (id) {
> - case PPMU_PMNCNT0:
> - case PPMU_PMNCNT1:
> - case PPMU_PMNCNT2:
> - ret = regmap_write(info->regmap, PPMU_V2_CH_EVx_TYPE(id),
> - PPMU_V2_RO_DATA_CNT | PPMU_V2_WO_DATA_CNT);
> - if (ret < 0)
> - return ret;
> - break;
> - case PPMU_PMNCNT3:
> - ret = regmap_write(info->regmap, PPMU_V2_CH_EVx_TYPE(id),
> - PPMU_V2_EVT3_RW_DATA_CNT);
> - if (ret < 0)
> - return ret;
> - break;
> - }
> + /* Set the event of proper data type monitoring */
> + ret = regmap_write(info->regmap, PPMU_V2_CH_EVx_TYPE(id),
> + edev->desc->data_type);
> + if (ret < 0)
> + return ret;
>
> /* Reset cycle counter/performance counter and enable PPMU */
> ret = regmap_read(info->regmap, PPMU_V2_PMNC, &pmnc);
> @@ -508,6 +496,7 @@ static int of_get_devfreq_events(struct device_node *np,
> struct device *dev = info->dev;
> struct device_node *events_np, *node;
> int i, j, count;
> + int ret;
>
> events_np = of_get_child_by_name(np, "events");
> if (!events_np) {
> @@ -544,6 +533,40 @@ static int of_get_devfreq_events(struct device_node *np,
> desc[j].driver_data = info;
>
> of_property_read_string(node, "event-name", &desc[j].name);
> + ret = of_property_read_u32(node, "event-data-type",
> + &desc[j].data_type);
> + if (!ret) {
It should be: if (ret)
then it means we do not have a value in DT and we are using
our defaults taking into account the right register type.
I am going to send v3 with this fix.
Regards,
Lukasz
> + /* Set the event of proper data type counting.
> + * Check if the data type has been defined in DT,
> + * use default if not.
> + */
> + if (of_device_is_compatible(np,
> + "samsung,exynos-ppmu-v2")) {
> + struct devfreq_event_dev edev;
> + int id;
> + /* Not all registers take the same value for
> + * read+write data count.
> + */
> + edev.desc = &desc[j];
> + id = exynos_ppmu_find_ppmu_id(&edev);
> +
> + switch (id) {
> + case PPMU_PMNCNT0:
> + case PPMU_PMNCNT1:
> + case PPMU_PMNCNT2:
> + desc[j].data_type = PPMU_V2_RO_DATA_CNT
> + | PPMU_V2_WO_DATA_CNT;
> + break;
> + case PPMU_PMNCNT3:
> + desc[j].data_type =
> + PPMU_V2_EVT3_RW_DATA_CNT;
> + break;
> + }
> + } else {
> + desc[j].data_type = PPMU_RO_DATA_CNT |
> + PPMU_WO_DATA_CNT;
> + }
> + }
>
> j++;
> }
> diff --git a/include/linux/devfreq-event.h b/include/linux/devfreq-event.h
> index 4db00b0..cc160b1 100644
> --- a/include/linux/devfreq-event.h
> +++ b/include/linux/devfreq-event.h
> @@ -81,14 +81,20 @@ struct devfreq_event_ops {
> * struct devfreq_event_desc - the descriptor of devfreq-event device
> *
> * @name : the name of devfreq-event device.
> + * @data_type : the data type which is going to be counted in the register.
> * @driver_data : the private data for devfreq-event driver.
> * @ops : the operation to control devfreq-event device.
> *
> * Each devfreq-event device is described with a this structure.
> * This structure contains the various data for devfreq-event device.
> + * The data_type describes what is going to be counted in the register.
> + * It might choose to count e.g. read requests, write data in bytes, etc.
> + * The full supported list of types is present in specyfic header in:
> + * include/dt-bindings/pmu/.
> */
> struct devfreq_event_desc {
> const char *name;
> + u32 data_type;
> void *driver_data;
>
> const struct devfreq_event_ops *ops;
>
Powered by blists - more mailing lists