lkml.org 
[lkml]   [2018]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 3/4] platform/x86: intel_pmc_core: Decode Snoop / Non Snoop LTR
On Mon, Sep 3, 2018 at 9:05 PM Rajneesh Bhardwaj
<rajneesh.bhardwaj@linux.intel.com> wrote:
>
> The LTR values follow PCIE LTR encoding format and can be decoded as per
> https://pcisig.com/sites/default/files/specification_documents/ECN_LatencyTolnReporting_14Aug08.pdf
>
> This adds support to translate the raw LTR values as read from the PMC
> to meaningful values in nanosecond units of time.

> +static void get_ltr_scale(u32 *val)

What's wrong to return converted value? Actually the name should
reflect what it does, ie *convert* value.

> +{
> + /*
> + * As per PCIE specification supprting document

supporting

> + * ECN_LatencyTolnReporting_14Aug08.pdf the Latency
> + * Tolerance Reporting data payload is encoded in a
> + * 3 bit scale and 10 bit value fields. Values are
> + * multiplied by the indicated scale to yield an absolute time
> + * value, expressible in a range from 1 nanosecond to
> + * 2^25*(2^10-1) = 34,326,183,936 nanoseconds.
> + *
> + * scale encoding is as follows:
> + *
> + * ----------------------------------------------
> + * |scale factor | Multiplier (ns) |
> + * ----------------------------------------------
> + * | 0 | 1 |
> + * | 1 | 32 |
> + * | 2 | 1024 |
> + * | 3 | 32768 |
> + * | 4 | 1048576 |
> + * | 5 | 33554432 |
> + * | 6 | Invalid |
> + * | 7 | Invalid |
> + * ----------------------------------------------
> + */
> + if (*val > 5) {
> + *val = 0;
> + pr_warn("Invalid LTR scale factor.\n");
> + } else {
> + *val = 1U << (5 * (*val));
> + }
> +}

> +
> static int pmc_core_ltr_show(struct seq_file *s, void *unused)
> {
> struct pmc_dev *pmcdev = s->private;
> const struct pmc_bit_map *map = pmcdev->map->ltr_show_sts;
> + u64 decoded_snoop_ltr = 0, decoded_non_snoop_ltr = 0;
> + union ltr_payload ltr_data;

> + u32 scale = 0;

Redundant assignment.

> int index;
>
> for (index = 0; map[index].name ; index++) {
> - seq_printf(s, "%-32s\tRAW LTR: 0x%x\n",
> + ltr_data.raw_data = pmc_core_reg_read(pmcdev,
> + map[index].bit_mask);
> +
> + if (ltr_data.bits.non_snoop_req) {
> + scale = ltr_data.bits.non_snoop_scale;
> + get_ltr_scale(&scale);
> + decoded_non_snoop_ltr =
> + ltr_data.bits.non_snoop_val * scale;
> + }
> +
> + if (ltr_data.bits.snoop_req) {
> + scale = ltr_data.bits.snoop_scale;
> + get_ltr_scale(&scale);
> + decoded_snoop_ltr =
> + ltr_data.bits.snoop_val * scale;
> + }
> +
> + seq_printf(s, "%-24s\tRaw LTR: 0x%-16x\t Non-Snoop LTR (ns): %-16llu\t Snoop LTR (ns): %-16llu\n",
> map[index].name,
> - pmc_core_reg_read(pmcdev, map[index].bit_mask));
> + ltr_data.raw_data,
> + decoded_non_snoop_ltr,
> + decoded_snoop_ltr);
> +

> + decoded_snoop_ltr = decoded_non_snoop_ltr = 0;

You may do this at the beginning of the loop and get rid of assignment
in the definition block.

> }
> return 0;
> }

> +union ltr_payload {
> + u32 raw_data;
> + struct {
> + u32 snoop_val : 10;
> + u32 snoop_scale : 3;
> + u32 snoop_res : 2;
> + u32 snoop_req : 1;
> + u32 non_snoop_val : 10;
> + u32 non_snoop_scale : 3;
> + u32 non_snoop_res : 2;
> + u32 non_snoop_req : 1;
> + } bits;
> +};

Just use normal masks and shifts.

--
With Best Regards,
Andy Shevchenko

\
 
 \ /
  Last update: 2018-09-26 15:54    [W:0.134 / U:0.600 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site