On 26-Sep-18 7:23 PM, Andy Shevchenko wrote:
On Mon, Sep 3, 2018 at 9:05 PM Rajneesh Bhardwaj
<rajneesh.bhardwaj@xxxxxxxxxxxxxxx> 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.
I can change it as per your suggestion.
+{
+ /*
+ * As per PCIE specification supprting document
supporting
oops. Will fix.
+ * 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.
Ok
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.
Fine.
}
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.
I chose union over masks and shifts to reduce code size and ensured
correct endian-ness. Just for my understanding, can you please let me
know why you feel masks/shift are better suited here?
