Re: [PATCH v3 3/3] perf: xgene: Add support for SoC PMU version 3

2017-06-22 Thread Hoan Tran
On Thu, Jun 22, 2017 at 11:17 AM, Mark Rutland  wrote:
> On Thu, Jun 22, 2017 at 11:13:08AM -0700, Hoan Tran wrote:
>> On Thu, Jun 22, 2017 at 10:52 AM, Mark Rutland  wrote:
>> > On Tue, Jun 06, 2017 at 11:02:26AM -0700, Hoan Tran wrote:
>> > >  static inline void
>> > > +xgene_pmu_write_counter64(struct xgene_pmu_dev *pmu_dev, int idx, u64 
>> > > val)
>> > > +{
>> > > + u32 cnt_lo, cnt_hi;
>> > > +
>> > > + cnt_hi = upper_32_bits(val);
>> > > + cnt_lo = lower_32_bits(val);
>> > > +
>> > > + /* v3 has 64-bit counter registers composed by 2 32-bit registers 
>> > > */
>> > > + xgene_pmu_write_counter32(pmu_dev, 2 * idx, cnt_lo);
>> > > + xgene_pmu_write_counter32(pmu_dev, 2 * idx + 1, cnt_hi);
>> > > +}
>> >
>> > For this to be atomic, we need to disable the counters for the duration
>> > of the IRQ handler, which we don't do today.
>> >
>> > Regardless, we should do that to ensure that groups are self-consistent.
>> >
>> > i.e. in xgene_pmu_isr() we should call ops->stop_counters() just after
>> > taking the pmu lock, and we should call ops->start_counters() just
>> > before releasing it.
>>
>> Thanks for your comments. I'll fix them and send another version of
>> patch set soon.
>
> No need; I'm picking these up now, and I'll apply the fixups locally.

Thanks!

Hoan

>
> Thanks,
> Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 3/3] perf: xgene: Add support for SoC PMU version 3

2017-06-22 Thread Hoan Tran
Hi Mark,

On Thu, Jun 22, 2017 at 11:18 AM, Mark Rutland  wrote:
> On Thu, Jun 22, 2017 at 06:52:56PM +0100, Mark Rutland wrote:
>> Hi Hoan,
>>
>> This largely looks good; I have one minor comment.
>>
>> On Tue, Jun 06, 2017 at 11:02:26AM -0700, Hoan Tran wrote:
>> >  static inline void
>> > +xgene_pmu_write_counter64(struct xgene_pmu_dev *pmu_dev, int idx, u64 val)
>> > +{
>> > +   u32 cnt_lo, cnt_hi;
>> > +
>> > +   cnt_hi = upper_32_bits(val);
>> > +   cnt_lo = lower_32_bits(val);
>> > +
>> > +   /* v3 has 64-bit counter registers composed by 2 32-bit registers */
>> > +   xgene_pmu_write_counter32(pmu_dev, 2 * idx, cnt_lo);
>> > +   xgene_pmu_write_counter32(pmu_dev, 2 * idx + 1, cnt_hi);
>> > +}
>>
>> For this to be atomic, we need to disable the counters for the duration
>> of the IRQ handler, which we don't do today.
>>
>> Regardless, we should do that to ensure that groups are self-consistent.
>>
>> i.e. in xgene_pmu_isr() we should call ops->stop_counters() just after
>> taking the pmu lock, and we should call ops->start_counters() just
>> before releasing it.
>>
>> With that:
>>
>> Acked-by: Mark Rutland 
>
> Actually, that should be in _xgene_pmu_isr, given we have to do it for each
> pmu_dev.
>
> I'll apply the diff below; this also avoids a race on V1 where an
> overflow could be lost (as we clear the whole OVSR rather than only the
> set bits).

Yes, I'm good about that. Thanks

Regards
Hoan

>
> Thanks,
> Mark.
>
> diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
> index 84c32e0..a9659cb 100644
> --- a/drivers/perf/xgene_pmu.c
> +++ b/drivers/perf/xgene_pmu.c
> @@ -1217,13 +1217,15 @@ static void _xgene_pmu_isr(int irq, struct 
> xgene_pmu_dev *pmu_dev)
> u32 pmovsr;
> int idx;
>
> +   xgene_pmu->ops->stop_counters(pmu_dev);
> +
> if (xgene_pmu->version == PCP_PMU_V3)
> pmovsr = readl(csr + PMU_PMOVSSET) & PMU_OVERFLOW_MASK;
> else
> pmovsr = readl(csr + PMU_PMOVSR) & PMU_OVERFLOW_MASK;
>
> if (!pmovsr)
> -   return;
> +   goto out;
>
> /* Clear interrupt flag */
> if (xgene_pmu->version == PCP_PMU_V1)
> @@ -1243,6 +1245,9 @@ static void _xgene_pmu_isr(int irq, struct 
> xgene_pmu_dev *pmu_dev)
> xgene_perf_event_update(event);
> xgene_perf_event_set_period(event);
> }
> +
> +out:
> +   xgene_pmu->ops->start_counters(pmu_dev);
>  }
>
>  static irqreturn_t xgene_pmu_isr(int irq, void *dev_id)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 3/3] perf: xgene: Add support for SoC PMU version 3

2017-06-22 Thread Hoan Tran
Hi Mark,

On Thu, Jun 22, 2017 at 10:52 AM, Mark Rutland  wrote:
>
> Hi Hoan,
>
> This largely looks good; I have one minor comment.
>
> On Tue, Jun 06, 2017 at 11:02:26AM -0700, Hoan Tran wrote:
> >  static inline void
> > +xgene_pmu_write_counter64(struct xgene_pmu_dev *pmu_dev, int idx, u64 val)
> > +{
> > + u32 cnt_lo, cnt_hi;
> > +
> > + cnt_hi = upper_32_bits(val);
> > + cnt_lo = lower_32_bits(val);
> > +
> > + /* v3 has 64-bit counter registers composed by 2 32-bit registers */
> > + xgene_pmu_write_counter32(pmu_dev, 2 * idx, cnt_lo);
> > + xgene_pmu_write_counter32(pmu_dev, 2 * idx + 1, cnt_hi);
> > +}
>
> For this to be atomic, we need to disable the counters for the duration
> of the IRQ handler, which we don't do today.
>
> Regardless, we should do that to ensure that groups are self-consistent.
>
> i.e. in xgene_pmu_isr() we should call ops->stop_counters() just after
> taking the pmu lock, and we should call ops->start_counters() just
> before releasing it.


Thanks for your comments. I'll fix them and send another version of
patch set soon.

Thanks
Hoan


>
>
> With that:
>
> Acked-by: Mark Rutland 
>
> Thanks,
> Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/3] perf: xgene: Add support for SoC PMU version 3

2017-06-20 Thread Hoan Tran
Hi Mark and All,

Do you have any comments on this patch set?

Thank you!
Hoan

On Tue, Jun 6, 2017 at 11:02 AM, Hoan Tran  wrote:
> This patch set adds support for SoC-wide (AKA uncore) Performance Monitoring
> Unit version 3.
>
> It can support up to
>  - 2 IOB PMU instances
>  - 8 L3C PMU instances
>  - 2 MCB PMU instances
>  - 8 MCU PMU instances
> and these PMUs support 64 bit counter
>
> v3:
>  * Seperate acpi_pmu_probe_active_mcb_mcu_l3c for v3
>  * Consistent PMU event name
>  * Update comment
>  * Correct active MCU detection
>
> v2:
>  * Split into separate patches
>  * Use the function pointers for the PMU leaf functions
>  * Parse PMU subnode by the match table
>  * Dont allow user change agent id by config1 for SoC PMU v3
>
> Hoan Tran (3):
>   perf: xgene: Parse PMU subnode from the match table
>   perf: xgene: Move PMU leaf functions into function pointer structure
>   perf: xgene: Add support for SoC PMU version 3
>
>  drivers/perf/xgene_pmu.c | 677 
> +++
>  1 file changed, 622 insertions(+), 55 deletions(-)
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/3] perf: xgene: Parse PMU subnode from the match table

2017-06-06 Thread Hoan Tran
This patch parses PMU Subnode from a match table.

Signed-off-by: Hoan Tran 
---
 drivers/perf/xgene_pmu.c | 40 ++--
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 35b5289..5ffd580 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -1047,9 +1047,35 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu 
*xgene_pmu,
return NULL;
 }
 
+static const struct acpi_device_id xgene_pmu_acpi_type_match[] = {
+   {"APMC0D5D", PMU_TYPE_L3C},
+   {"APMC0D5E", PMU_TYPE_IOB},
+   {"APMC0D5F", PMU_TYPE_MCB},
+   {"APMC0D60", PMU_TYPE_MC},
+   {},
+};
+
+static const struct acpi_device_id *xgene_pmu_acpi_match_type(
+   const struct acpi_device_id *ids,
+   struct acpi_device *adev)
+{
+   const struct acpi_device_id *match_id = NULL;
+   const struct acpi_device_id *id;
+
+   for (id = ids; id->id[0] || id->cls; id++) {
+   if (!acpi_match_device_ids(adev, id))
+   match_id = id;
+   else if (match_id)
+   break;
+   }
+
+   return match_id;
+}
+
 static acpi_status acpi_pmu_dev_add(acpi_handle handle, u32 level,
void *data, void **return_value)
 {
+   const struct acpi_device_id *acpi_id;
struct xgene_pmu *xgene_pmu = data;
struct xgene_pmu_dev_ctx *ctx;
struct acpi_device *adev;
@@ -1059,17 +1085,11 @@ static acpi_status acpi_pmu_dev_add(acpi_handle handle, 
u32 level,
if (acpi_bus_get_status(adev) || !adev->status.present)
return AE_OK;
 
-   if (!strcmp(acpi_device_hid(adev), "APMC0D5D"))
-   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_L3C);
-   else if (!strcmp(acpi_device_hid(adev), "APMC0D5E"))
-   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_IOB);
-   else if (!strcmp(acpi_device_hid(adev), "APMC0D5F"))
-   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MCB);
-   else if (!strcmp(acpi_device_hid(adev), "APMC0D60"))
-   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MC);
-   else
-   ctx = NULL;
+   acpi_id = xgene_pmu_acpi_match_type(xgene_pmu_acpi_type_match, adev);
+   if (!acpi_id)
+   return AE_OK;
 
+   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, (u32)acpi_id->driver_data);
if (!ctx)
return AE_OK;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/3] perf: xgene: Move PMU leaf functions into function pointer structure

2017-06-06 Thread Hoan Tran
This patch moves PMU leaf functions into a function pointer structure.
It helps code maintain and expasion easier.

Signed-off-by: Hoan Tran 
---
 drivers/perf/xgene_pmu.c | 85 +---
 1 file changed, 66 insertions(+), 19 deletions(-)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 5ffd580..f34fc78 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -96,6 +96,23 @@ struct xgene_pmu_dev {
struct perf_event *pmu_counter_event[PMU_MAX_COUNTERS];
 };
 
+struct xgene_pmu_ops {
+   void (*mask_int)(struct xgene_pmu *pmu);
+   void (*unmask_int)(struct xgene_pmu *pmu);
+   u64 (*read_counter)(struct xgene_pmu_dev *pmu, int idx);
+   void (*write_counter)(struct xgene_pmu_dev *pmu, int idx, u64 val);
+   void (*write_evttype)(struct xgene_pmu_dev *pmu_dev, int idx, u32 val);
+   void (*write_agentmsk)(struct xgene_pmu_dev *pmu_dev, u32 val);
+   void (*write_agent1msk)(struct xgene_pmu_dev *pmu_dev, u32 val);
+   void (*enable_counter)(struct xgene_pmu_dev *pmu_dev, int idx);
+   void (*disable_counter)(struct xgene_pmu_dev *pmu_dev, int idx);
+   void (*enable_counter_int)(struct xgene_pmu_dev *pmu_dev, int idx);
+   void (*disable_counter_int)(struct xgene_pmu_dev *pmu_dev, int idx);
+   void (*reset_counters)(struct xgene_pmu_dev *pmu_dev);
+   void (*start_counters)(struct xgene_pmu_dev *pmu_dev);
+   void (*stop_counters)(struct xgene_pmu_dev *pmu_dev);
+};
+
 struct xgene_pmu {
struct device *dev;
int version;
@@ -104,6 +121,7 @@ struct xgene_pmu {
u32 mc_active_mask;
cpumask_t cpu;
raw_spinlock_t lock;
+   const struct xgene_pmu_ops *ops;
struct list_head l3cpmus;
struct list_head iobpmus;
struct list_head mcbpmus;
@@ -392,13 +410,14 @@ static inline void xgene_pmu_unmask_int(struct xgene_pmu 
*xgene_pmu)
writel(PCPPMU_INTCLRMASK, xgene_pmu->pcppmu_csr + PCPPMU_INTMASK_REG);
 }
 
-static inline u32 xgene_pmu_read_counter(struct xgene_pmu_dev *pmu_dev, int 
idx)
+static inline u64 xgene_pmu_read_counter32(struct xgene_pmu_dev *pmu_dev,
+  int idx)
 {
-   return readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
+   return (u64)readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
 }
 
 static inline void
-xgene_pmu_write_counter(struct xgene_pmu_dev *pmu_dev, int idx, u32 val)
+xgene_pmu_write_counter32(struct xgene_pmu_dev *pmu_dev, int idx, u64 val)
 {
writel(val, pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
 }
@@ -491,20 +510,22 @@ static inline void xgene_pmu_stop_counters(struct 
xgene_pmu_dev *pmu_dev)
 static void xgene_perf_pmu_enable(struct pmu *pmu)
 {
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(pmu);
+   struct xgene_pmu *xgene_pmu = pmu_dev->parent;
int enabled = bitmap_weight(pmu_dev->cntr_assign_mask,
pmu_dev->max_counters);
 
if (!enabled)
return;
 
-   xgene_pmu_start_counters(pmu_dev);
+   xgene_pmu->ops->start_counters(pmu_dev);
 }
 
 static void xgene_perf_pmu_disable(struct pmu *pmu)
 {
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(pmu);
+   struct xgene_pmu *xgene_pmu = pmu_dev->parent;
 
-   xgene_pmu_stop_counters(pmu_dev);
+   xgene_pmu->ops->stop_counters(pmu_dev);
 }
 
 static int xgene_perf_event_init(struct perf_event *event)
@@ -572,27 +593,32 @@ static int xgene_perf_event_init(struct perf_event *event)
 static void xgene_perf_enable_event(struct perf_event *event)
 {
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+   struct xgene_pmu *xgene_pmu = pmu_dev->parent;
 
-   xgene_pmu_write_evttype(pmu_dev, GET_CNTR(event), GET_EVENTID(event));
-   xgene_pmu_write_agentmsk(pmu_dev, ~((u32)GET_AGENTID(event)));
+   xgene_pmu->ops->write_evttype(pmu_dev, GET_CNTR(event),
+ GET_EVENTID(event));
+   xgene_pmu->ops->write_agentmsk(pmu_dev, ~((u32)GET_AGENTID(event)));
if (pmu_dev->inf->type == PMU_TYPE_IOB)
-   xgene_pmu_write_agent1msk(pmu_dev, ~((u32)GET_AGENT1ID(event)));
+   xgene_pmu->ops->write_agent1msk(pmu_dev,
+   ~((u32)GET_AGENT1ID(event)));
 
-   xgene_pmu_enable_counter(pmu_dev, GET_CNTR(event));
-   xgene_pmu_enable_counter_int(pmu_dev, GET_CNTR(event));
+   xgene_pmu->ops->enable_counter(pmu_dev, GET_CNTR(event));
+   xgene_pmu->ops->enable_counter_int(pmu_dev, GET_CNTR(event));
 }
 
 static void xgene_perf_disable_event(struct perf_event *event)
 {
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+   struct xgene_pmu *xgene_pmu = pmu_dev->parent;
 
-   xgene_pmu_disable_counter(pmu_dev, GET_CNTR(event));
-   xgene_pmu_d

[PATCH v3 0/3] perf: xgene: Add support for SoC PMU version 3

2017-06-06 Thread Hoan Tran
This patch set adds support for SoC-wide (AKA uncore) Performance Monitoring
Unit version 3.

It can support up to
 - 2 IOB PMU instances
 - 8 L3C PMU instances
 - 2 MCB PMU instances
 - 8 MCU PMU instances
and these PMUs support 64 bit counter

v3:
 * Seperate acpi_pmu_probe_active_mcb_mcu_l3c for v3
 * Consistent PMU event name
 * Update comment
 * Correct active MCU detection

v2:
 * Split into separate patches
 * Use the function pointers for the PMU leaf functions
 * Parse PMU subnode by the match table
 * Dont allow user change agent id by config1 for SoC PMU v3

Hoan Tran (3):
  perf: xgene: Parse PMU subnode from the match table
  perf: xgene: Move PMU leaf functions into function pointer structure
  perf: xgene: Add support for SoC PMU version 3

 drivers/perf/xgene_pmu.c | 677 +++
 1 file changed, 622 insertions(+), 55 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 3/3] perf: xgene: Add support for SoC PMU version 3

2017-06-06 Thread Hoan Tran
This patch adds support for SoC-wide (AKA uncore) Performance Monitoring
Unit version 3.

It can support up to
 - 2 IOB PMU instances
 - 8 L3C PMU instances
 - 2 MCB PMU instances
 - 8 MCU PMU instances
and these PMUs support 64 bit counter

Signed-off-by: Hoan Tran 
---
 drivers/perf/xgene_pmu.c | 558 ---
 1 file changed, 529 insertions(+), 29 deletions(-)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index f34fc78..84c32e0 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -37,6 +37,8 @@
 
 #define CSW_CSWCR   0x
 #define  CSW_CSWCR_DUALMCB_MASK BIT(0)
+#define  CSW_CSWCR_MCB0_ROUTING(x) (((x) & 0x0C) >> 2)
+#define  CSW_CSWCR_MCB1_ROUTING(x) (((x) & 0x30) >> 4)
 #define MCBADDRMR   0x
 #define  MCBADDRMR_DUALMCU_MODE_MASKBIT(2)
 
@@ -50,8 +52,17 @@
 #define  PCPPMU_INT_L3CBIT(2)
 #define  PCPPMU_INT_IOBBIT(3)
 
+#define  PCPPMU_V3_INTMASK 0x00FF33FF
+#define  PCPPMU_V3_INTENMASK   0x
+#define  PCPPMU_V3_INTCLRMASK  0xFF00CC00
+#define  PCPPMU_V3_INT_MCU 0x00FF
+#define  PCPPMU_V3_INT_MCB 0x0300
+#define  PCPPMU_V3_INT_L3C 0x00FF
+#define  PCPPMU_V3_INT_IOB 0x3000
+
 #define PMU_MAX_COUNTERS   4
-#define PMU_CNT_MAX_PERIOD 0x1ULL
+#define PMU_CNT_MAX_PERIOD 0xULL
+#define PMU_V3_CNT_MAX_PERIOD  0xULL
 #define PMU_OVERFLOW_MASK  0xF
 #define PMU_PMCR_E BIT(0)
 #define PMU_PMCR_P BIT(1)
@@ -73,6 +84,10 @@
 #define PMU_PMOVSR 0xC80
 #define PMU_PMCR   0xE04
 
+/* PMU registers for V3 */
+#define PMU_PMOVSCLR   0xC80
+#define PMU_PMOVSSET   0xCC0
+
 #define to_pmu_dev(p) container_of(p, struct xgene_pmu_dev, pmu)
 #define GET_CNTR(ev)  (ev->hw.idx)
 #define GET_EVENTID(ev)   (ev->hw.config & 0xFFULL)
@@ -119,6 +134,7 @@ struct xgene_pmu {
void __iomem *pcppmu_csr;
u32 mcb_active_mask;
u32 mc_active_mask;
+   u32 l3c_active_mask;
cpumask_t cpu;
raw_spinlock_t lock;
const struct xgene_pmu_ops *ops;
@@ -143,11 +159,13 @@ struct xgene_pmu_data {
 enum xgene_pmu_version {
PCP_PMU_V1 = 1,
PCP_PMU_V2,
+   PCP_PMU_V3,
 };
 
 enum xgene_pmu_dev_type {
PMU_TYPE_L3C = 0,
PMU_TYPE_IOB,
+   PMU_TYPE_IOB_SLOW,
PMU_TYPE_MCB,
PMU_TYPE_MC,
 };
@@ -213,6 +231,56 @@ static ssize_t xgene_pmu_format_show(struct device *dev,
.attrs = mc_pmu_format_attrs,
 };
 
+static struct attribute *l3c_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(l3c_eventid, "config:0-39"),
+   NULL,
+};
+
+static struct attribute *iob_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(iob_eventid, "config:0-47"),
+   NULL,
+};
+
+static struct attribute *iob_slow_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(iob_slow_eventid, "config:0-16"),
+   NULL,
+};
+
+static struct attribute *mcb_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(mcb_eventid, "config:0-35"),
+   NULL,
+};
+
+static struct attribute *mc_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(mc_eventid, "config:0-44"),
+   NULL,
+};
+
+static const struct attribute_group l3c_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = l3c_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group iob_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = iob_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group iob_slow_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = iob_slow_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group mcb_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = mcb_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group mc_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = mc_pmu_v3_format_attrs,
+};
+
 /*
  * sysfs event attributes
  */
@@ -329,6 +397,219 @@ static ssize_t xgene_pmu_event_show(struct device *dev,
.attrs = mc_pmu_events_attrs,
 };
 
+static struct attribute *l3c_pmu_v3_events_attrs[] = {
+   XGENE_PMU_EVENT_ATTR(cycle-count,   0x00),
+   XGENE_PMU_EVENT_ATTR(read-hit,  0x01),
+   XGENE_PMU_EVENT_ATTR(read-miss, 0x02),
+   XGENE_PMU_EVENT_ATTR(index-flush-eviction,  0x03),
+   XGENE_PMU_EVENT_ATTR(write-caused-replacement,  0x04),
+   XGENE_PMU_EVENT_ATTR(write-not-caused-replacement,  0x05),
+   XGENE_PMU_EVENT_ATTR(clean-eviction,0x06),
+   XGENE_PMU_EVENT_ATTR(dirty-eviction,0x07),
+   X

Re: [PATCH v2 3/3] perf: xgene: Add support for SoC PMU version 3

2017-06-02 Thread Hoan Tran
Hi Mark,

On Fri, Jun 2, 2017 at 9:04 AM, Mark Rutland  wrote:
> Hi Hoan,
>
> Apologies for the delay in getting to this.
>
> On Mon, Apr 03, 2017 at 09:47:57AM -0700, Hoan Tran wrote:
>> This patch adds support for SoC-wide (AKA uncore) Performance Monitoring
>> Unit version 3.
>>
>> It can support up to
>>  - 2 IOB PMU instances
>>  - 8 L3C PMU instances
>>  - 2 MCB PMU instances
>>  - 8 MCU PMU instances
>
> Please elaborate on the differences compared with prior versions.
>
> I see that counters are 64-bit. Are there any other important details to
> be aware of?

Yes, let me add 64 bit counter into the patch description. Beside of
that, I don't think anything else besides more PMU instances compared
with the previous version.

>
> [...]
>
>> +static struct attribute *l3c_pmu_v3_events_attrs[] = {
>
>> + XGENE_PMU_EVENT_ATTR(read-hit,  0x01),
>> + XGENE_PMU_EVENT_ATTR(read-miss, 0x02),
>
>> + XGENE_PMU_EVENT_ATTR(reads, 0x08),
>> + XGENE_PMU_EVENT_ATTR(writes,0x09),
>
>> +};
>
> Some of these are singular (e.g. "read-hit", "read-miss"), while others
> are plural (e.g. "reads", "writes").
>
> The existing driver use the singular form. Please remain consistent with
> that. e.g. turn "reads" into "read".

Will fix it.

>
>> + XGENE_PMU_EVENT_ATTR(PA-req-buf-alloc-all,  0x01),
>
> Likewise, please consistently use lower case.

Yes

>
>> + XGENE_PMU_EVENT_ATTR(pmu-act-sent,  0x01),
>
> Surely we don't need "pmu" in the event names?

Yes, will remove "pmu".

>
> [...]
>
>>  static inline u64 xgene_pmu_read_counter32(struct xgene_pmu_dev *pmu_dev,
>>  int idx)
>>  {
>>   return (u64)readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
>>  }
>
> I don't think the cast is necessary. Please remove it.
>
>>  static inline void
>> +xgene_pmu_write_counter64(struct xgene_pmu_dev *pmu_dev, int idx, u64 val)
>> +{
>> + u32 cnt_lo, cnt_hi;
>> +
>> + cnt_lo = val & 0x;
>> + cnt_hi = val >> 32;
>
> cnt_hi = upper_32_bits(val);
> cnt_lo = lower_32_bits(val);
>
> (both are in )
>
>> +
>> + /* v3 has 64-bit counter registers composed by 2 32-bit registers */
>> + xgene_pmu_write_counter32(pmu_dev, 2 * idx, val);
>> + xgene_pmu_write_counter32(pmu_dev, 2 * idx + 1, val >> 32);
>
> Please use cnt_hi and cnt_lo for the writes.
>
> [...]
>
>> @@ -621,7 +989,7 @@ static void xgene_perf_event_set_period(struct 
>> perf_event *event)
>>   struct xgene_pmu *xgene_pmu = pmu_dev->parent;
>>   struct hw_perf_event *hw = &event->hw;
>>   /*
>> -  * The X-Gene PMU counters have a period of 2^32. To account for the
>> +  * The X-Gene PMU counters have a period of 2^32 or more. To account 
>> for the
>>* possiblity of extreme interrupt latency we program for a period of
>>* half that. Hopefully we can handle the interrupt before another 2^31
>>* events occur and the counter overtakes its previous value.
>
> This comment is now a little out-of-date, as we don't start 64-bit
> counters at half their period.
>
> I guess we don't expect a 64-bit counter to overflow, so can we state
> that in the comment?

How about this one.

/*
 * For 32 bit counter, it has a period of 2^32. To account for the
 * possibility of extreme interrupt latency we program for a period of
 * half that. Hopefully, we can handle the interrupt before another 2^31
 * events occur and the counter overtakes its previous value.
 * For 64 bit counter, we don't expect it overflow.
 */

>
> [...]
>
>> -static int acpi_pmu_probe_active_mcb_mcu(struct xgene_pmu *xgene_pmu,
>> +static int acpi_pmu_probe_active_mcb_mcu_l3c(struct xgene_pmu *xgene_pmu,
>>struct platform_device *pdev)
>>  {
>>   void __iomem *csw_csr, *mcba_csr, *mcbb_csr;
>>   struct resource *res;
>>   unsigned int reg;
>> + u32 mcb0routing;
>> + u32 mcb1routing;
>>
>>   res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>   csw_csr = devm_ioremap_resource(&pdev->dev, res);
>> @@ -899,41 +1309,72 @@ static int acpi_pmu_probe_active_mcb_mcu(struct 
>&

Re: [PATCH v2 1/3] perf: xgene: Parse PMU subnode from the match table

2017-06-02 Thread Hoan Tran
Hi Mark,

On Fri, Jun 2, 2017 at 10:23 AM, Mark Rutland  wrote:
> On Fri, Jun 02, 2017 at 09:54:32AM -0700, Hoan Tran wrote:
>> On Fri, Jun 2, 2017 at 7:59 AM, Mark Rutland  wrote:
>> > On Mon, Apr 03, 2017 at 09:47:55AM -0700, Hoan Tran wrote:
>> >> +static const struct acpi_device_id *xgene_pmu_acpi_match_type(
>> >> + const struct acpi_device_id *ids,
>> >> + struct acpi_device *adev)
>> >> +{
>> >> + const struct acpi_device_id *match_id = NULL;
>> >> + const struct acpi_device_id *id;
>> >> +
>> >> + for (id = ids; id->id[0] || id->cls; id++) {
>> >> + if (!acpi_match_device_ids(adev, id))
>> >> + match_id = id;
>> >> + else if (match_id)
>> >> + break;
>> >> + }
>> >> +
>> >> + return match_id;
>> >> +}
>> >
>> > I don't believe this look is necessary. AFAICT, acpi_match_device_ids()
>> > already iterates over the id table it is given.
>>
>> The acpi_match_device_ids() function just returns if a device ID is
>> available on the given list. It does not return the first matching ID.
>> That's the reason I created this function to find the first matching ID.
>
> Ah, I see. Thanks for correcting me!
>
> Can we use acpi_match_device(ids, &adev->dev), or is that the wrong dev?

They are subnode device, so they don't have full dev. Because of that,
acpi_match_device doesn't work.

Thanks
Hoan

>
> Thanks,
> Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/3] perf: xgene: Parse PMU subnode from the match table

2017-06-02 Thread Hoan Tran
Hi Mark

On Fri, Jun 2, 2017 at 7:59 AM, Mark Rutland  wrote:
> Hi Hoan,
>
> Apologies for the last reply.
>
> On Mon, Apr 03, 2017 at 09:47:55AM -0700, Hoan Tran wrote:
>> +static const struct acpi_device_id xgene_pmu_acpi_type_match[] = {
>> + {"APMC0D5D", PMU_TYPE_L3C},
>> + {"APMC0D5E", PMU_TYPE_IOB},
>> + {"APMC0D5F", PMU_TYPE_MCB},
>> + {"APMC0D60", PMU_TYPE_MC},
>> + {},
>> +};
>> +
>> +static const struct acpi_device_id *xgene_pmu_acpi_match_type(
>> + const struct acpi_device_id *ids,
>> + struct acpi_device *adev)
>> +{
>> + const struct acpi_device_id *match_id = NULL;
>> + const struct acpi_device_id *id;
>> +
>> + for (id = ids; id->id[0] || id->cls; id++) {
>> + if (!acpi_match_device_ids(adev, id))
>> + match_id = id;
>> + else if (match_id)
>> + break;
>> + }
>> +
>> + return match_id;
>> +}
>
> I don't believe this look is necessary. AFAICT, acpi_match_device_ids()
> already iterates over the id table it is given.

The acpi_match_device_ids() function just returns if a device ID is
available on the given list. It does not return the first matching ID.
That's the reason I created this function to find the first matching ID.

>
>> +
>>  static acpi_status acpi_pmu_dev_add(acpi_handle handle, u32 level,
>>   void *data, void **return_value)
>>  {
>> + const struct acpi_device_id *acpi_id;
>>   struct xgene_pmu *xgene_pmu = data;
>>   struct xgene_pmu_dev_ctx *ctx;
>>   struct acpi_device *adev;
>> @@ -1059,17 +1085,11 @@ static acpi_status acpi_pmu_dev_add(acpi_handle 
>> handle, u32 level,
>>   if (acpi_bus_get_status(adev) || !adev->status.present)
>>   return AE_OK;
>>
>> - if (!strcmp(acpi_device_hid(adev), "APMC0D5D"))
>> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_L3C);
>> - else if (!strcmp(acpi_device_hid(adev), "APMC0D5E"))
>> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_IOB);
>> - else if (!strcmp(acpi_device_hid(adev), "APMC0D5F"))
>> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MCB);
>> - else if (!strcmp(acpi_device_hid(adev), "APMC0D60"))
>> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MC);
>> - else
>> - ctx = NULL;
>> + acpi_id = xgene_pmu_acpi_match_type(xgene_pmu_acpi_type_match, adev);
>> + if (!acpi_id)
>> + return AE_OK;
>
> As above, and as I covered in my reply to v1, I think the above should
> be:
>
> acpi_id = acpi_match_device_ids(adev, xgene_pmu_acpi_type_match);
> if (!acpi_id)
> return AE_OK;
>
> ... or am I missing something?

The same above.

Thanks
Hoan

>
> With that change:
>
> Acked-by: Mark Rutland 
>
> Thanks,
> Mark.
>
>>
>> + ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, (u32)acpi_id->driver_data);
>>   if (!ctx)
>>   return AE_OK;
>>
>> --
>> 1.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 0/3] perf: xgene: Add support for SoC PMU version 3

2017-05-30 Thread Hoan Tran
Hi All,

Ping again

Thanks
Hoan

On Fri, May 5, 2017 at 8:48 AM, Hoan Tran  wrote:
> Ping!
>
> Thanks
> Hoan
>
> On Thu, Apr 13, 2017 at 10:50 AM, Hoan Tran  wrote:
>> Hi All,
>>
>> Do you have any comments on this patch set?
>>
>> Thanks
>> Hoan
>>
>> On Mon, Apr 3, 2017 at 9:47 AM, Hoan Tran  wrote:
>>>
>>> This patch set adds support for SoC-wide (AKA uncore) Performance
>>> Monitoring
>>> Unit version 3.
>>>
>>> It can support up to
>>>  - 2 IOB PMU instances
>>>  - 8 L3C PMU instances
>>>  - 2 MCB PMU instances
>>>  - 8 MCU PMU instances
>>>
>>> v2:
>>>  * Split into separate patches
>>>  * Use the function pointers for the PMU leaf functions
>>>  * Parse PMU subnode by the match table
>>>  * Dont allow user change agent id by config1 for SoC PMU v3
>>>
>>> Hoan Tran (3):
>>>   perf: xgene: Parse PMU subnode from the match table
>>>   perf: xgene: Move PMU leaf functions into function pointer structure
>>>   perf: xgene: Add support for SoC PMU version 3
>>>
>>>  drivers/perf/xgene_pmu.c | 695
>>> +--
>>>  1 file changed, 619 insertions(+), 76 deletions(-)
>>>
>>> --
>>> 1.9.1
>>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 0/3] perf: xgene: Add support for SoC PMU version 3

2017-05-05 Thread Hoan Tran
Ping!

Thanks
Hoan

On Thu, Apr 13, 2017 at 10:50 AM, Hoan Tran  wrote:
> Hi All,
>
> Do you have any comments on this patch set?
>
> Thanks
> Hoan
>
> On Mon, Apr 3, 2017 at 9:47 AM, Hoan Tran  wrote:
>>
>> This patch set adds support for SoC-wide (AKA uncore) Performance
>> Monitoring
>> Unit version 3.
>>
>> It can support up to
>>  - 2 IOB PMU instances
>>  - 8 L3C PMU instances
>>  - 2 MCB PMU instances
>>  - 8 MCU PMU instances
>>
>> v2:
>>  * Split into separate patches
>>  * Use the function pointers for the PMU leaf functions
>>  * Parse PMU subnode by the match table
>>  * Dont allow user change agent id by config1 for SoC PMU v3
>>
>> Hoan Tran (3):
>>   perf: xgene: Parse PMU subnode from the match table
>>   perf: xgene: Move PMU leaf functions into function pointer structure
>>   perf: xgene: Add support for SoC PMU version 3
>>
>>  drivers/perf/xgene_pmu.c | 695
>> +--
>>  1 file changed, 619 insertions(+), 76 deletions(-)
>>
>> --
>> 1.9.1
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 0/3] perf: xgene: Add support for SoC PMU version 3

2017-04-17 Thread Hoan Tran
Hi All,

Do you have any comments on this patch set?

Thanks

On Mon, Apr 3, 2017 at 9:47 AM, Hoan Tran  wrote:
> This patch set adds support for SoC-wide (AKA uncore) Performance Monitoring
> Unit version 3.
>
> It can support up to
>  - 2 IOB PMU instances
>  - 8 L3C PMU instances
>  - 2 MCB PMU instances
>  - 8 MCU PMU instances
>
> v2:
>  * Split into separate patches
>  * Use the function pointers for the PMU leaf functions
>  * Parse PMU subnode by the match table
>  * Dont allow user change agent id by config1 for SoC PMU v3
>
> Hoan Tran (3):
>   perf: xgene: Parse PMU subnode from the match table
>   perf: xgene: Move PMU leaf functions into function pointer structure
>   perf: xgene: Add support for SoC PMU version 3
>
>  drivers/perf/xgene_pmu.c | 695 
> +--
>  1 file changed, 619 insertions(+), 76 deletions(-)
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/3] perf: xgene: Move PMU leaf functions into function pointer structure

2017-04-03 Thread Hoan Tran
This patch moves PMU leaf functions into a function pointer structure.
It helps code maintain and expasion easier.

Signed-off-by: Hoan Tran 
---
 drivers/perf/xgene_pmu.c | 85 +---
 1 file changed, 66 insertions(+), 19 deletions(-)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 5ffd580..f34fc78 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -96,6 +96,23 @@ struct xgene_pmu_dev {
struct perf_event *pmu_counter_event[PMU_MAX_COUNTERS];
 };
 
+struct xgene_pmu_ops {
+   void (*mask_int)(struct xgene_pmu *pmu);
+   void (*unmask_int)(struct xgene_pmu *pmu);
+   u64 (*read_counter)(struct xgene_pmu_dev *pmu, int idx);
+   void (*write_counter)(struct xgene_pmu_dev *pmu, int idx, u64 val);
+   void (*write_evttype)(struct xgene_pmu_dev *pmu_dev, int idx, u32 val);
+   void (*write_agentmsk)(struct xgene_pmu_dev *pmu_dev, u32 val);
+   void (*write_agent1msk)(struct xgene_pmu_dev *pmu_dev, u32 val);
+   void (*enable_counter)(struct xgene_pmu_dev *pmu_dev, int idx);
+   void (*disable_counter)(struct xgene_pmu_dev *pmu_dev, int idx);
+   void (*enable_counter_int)(struct xgene_pmu_dev *pmu_dev, int idx);
+   void (*disable_counter_int)(struct xgene_pmu_dev *pmu_dev, int idx);
+   void (*reset_counters)(struct xgene_pmu_dev *pmu_dev);
+   void (*start_counters)(struct xgene_pmu_dev *pmu_dev);
+   void (*stop_counters)(struct xgene_pmu_dev *pmu_dev);
+};
+
 struct xgene_pmu {
struct device *dev;
int version;
@@ -104,6 +121,7 @@ struct xgene_pmu {
u32 mc_active_mask;
cpumask_t cpu;
raw_spinlock_t lock;
+   const struct xgene_pmu_ops *ops;
struct list_head l3cpmus;
struct list_head iobpmus;
struct list_head mcbpmus;
@@ -392,13 +410,14 @@ static inline void xgene_pmu_unmask_int(struct xgene_pmu 
*xgene_pmu)
writel(PCPPMU_INTCLRMASK, xgene_pmu->pcppmu_csr + PCPPMU_INTMASK_REG);
 }
 
-static inline u32 xgene_pmu_read_counter(struct xgene_pmu_dev *pmu_dev, int 
idx)
+static inline u64 xgene_pmu_read_counter32(struct xgene_pmu_dev *pmu_dev,
+  int idx)
 {
-   return readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
+   return (u64)readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
 }
 
 static inline void
-xgene_pmu_write_counter(struct xgene_pmu_dev *pmu_dev, int idx, u32 val)
+xgene_pmu_write_counter32(struct xgene_pmu_dev *pmu_dev, int idx, u64 val)
 {
writel(val, pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
 }
@@ -491,20 +510,22 @@ static inline void xgene_pmu_stop_counters(struct 
xgene_pmu_dev *pmu_dev)
 static void xgene_perf_pmu_enable(struct pmu *pmu)
 {
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(pmu);
+   struct xgene_pmu *xgene_pmu = pmu_dev->parent;
int enabled = bitmap_weight(pmu_dev->cntr_assign_mask,
pmu_dev->max_counters);
 
if (!enabled)
return;
 
-   xgene_pmu_start_counters(pmu_dev);
+   xgene_pmu->ops->start_counters(pmu_dev);
 }
 
 static void xgene_perf_pmu_disable(struct pmu *pmu)
 {
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(pmu);
+   struct xgene_pmu *xgene_pmu = pmu_dev->parent;
 
-   xgene_pmu_stop_counters(pmu_dev);
+   xgene_pmu->ops->stop_counters(pmu_dev);
 }
 
 static int xgene_perf_event_init(struct perf_event *event)
@@ -572,27 +593,32 @@ static int xgene_perf_event_init(struct perf_event *event)
 static void xgene_perf_enable_event(struct perf_event *event)
 {
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+   struct xgene_pmu *xgene_pmu = pmu_dev->parent;
 
-   xgene_pmu_write_evttype(pmu_dev, GET_CNTR(event), GET_EVENTID(event));
-   xgene_pmu_write_agentmsk(pmu_dev, ~((u32)GET_AGENTID(event)));
+   xgene_pmu->ops->write_evttype(pmu_dev, GET_CNTR(event),
+ GET_EVENTID(event));
+   xgene_pmu->ops->write_agentmsk(pmu_dev, ~((u32)GET_AGENTID(event)));
if (pmu_dev->inf->type == PMU_TYPE_IOB)
-   xgene_pmu_write_agent1msk(pmu_dev, ~((u32)GET_AGENT1ID(event)));
+   xgene_pmu->ops->write_agent1msk(pmu_dev,
+   ~((u32)GET_AGENT1ID(event)));
 
-   xgene_pmu_enable_counter(pmu_dev, GET_CNTR(event));
-   xgene_pmu_enable_counter_int(pmu_dev, GET_CNTR(event));
+   xgene_pmu->ops->enable_counter(pmu_dev, GET_CNTR(event));
+   xgene_pmu->ops->enable_counter_int(pmu_dev, GET_CNTR(event));
 }
 
 static void xgene_perf_disable_event(struct perf_event *event)
 {
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+   struct xgene_pmu *xgene_pmu = pmu_dev->parent;
 
-   xgene_pmu_disable_counter(pmu_dev, GET_CNTR(event));
-   xgene_pmu_d

[PATCH v2 1/3] perf: xgene: Parse PMU subnode from the match table

2017-04-03 Thread Hoan Tran
This patch parses PMU Subnode from a match table.

Signed-off-by: Hoan Tran 
---
 drivers/perf/xgene_pmu.c | 40 ++--
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 35b5289..5ffd580 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -1047,9 +1047,35 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu 
*xgene_pmu,
return NULL;
 }
 
+static const struct acpi_device_id xgene_pmu_acpi_type_match[] = {
+   {"APMC0D5D", PMU_TYPE_L3C},
+   {"APMC0D5E", PMU_TYPE_IOB},
+   {"APMC0D5F", PMU_TYPE_MCB},
+   {"APMC0D60", PMU_TYPE_MC},
+   {},
+};
+
+static const struct acpi_device_id *xgene_pmu_acpi_match_type(
+   const struct acpi_device_id *ids,
+   struct acpi_device *adev)
+{
+   const struct acpi_device_id *match_id = NULL;
+   const struct acpi_device_id *id;
+
+   for (id = ids; id->id[0] || id->cls; id++) {
+   if (!acpi_match_device_ids(adev, id))
+   match_id = id;
+   else if (match_id)
+   break;
+   }
+
+   return match_id;
+}
+
 static acpi_status acpi_pmu_dev_add(acpi_handle handle, u32 level,
void *data, void **return_value)
 {
+   const struct acpi_device_id *acpi_id;
struct xgene_pmu *xgene_pmu = data;
struct xgene_pmu_dev_ctx *ctx;
struct acpi_device *adev;
@@ -1059,17 +1085,11 @@ static acpi_status acpi_pmu_dev_add(acpi_handle handle, 
u32 level,
if (acpi_bus_get_status(adev) || !adev->status.present)
return AE_OK;
 
-   if (!strcmp(acpi_device_hid(adev), "APMC0D5D"))
-   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_L3C);
-   else if (!strcmp(acpi_device_hid(adev), "APMC0D5E"))
-   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_IOB);
-   else if (!strcmp(acpi_device_hid(adev), "APMC0D5F"))
-   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MCB);
-   else if (!strcmp(acpi_device_hid(adev), "APMC0D60"))
-   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MC);
-   else
-   ctx = NULL;
+   acpi_id = xgene_pmu_acpi_match_type(xgene_pmu_acpi_type_match, adev);
+   if (!acpi_id)
+   return AE_OK;
 
+   ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, (u32)acpi_id->driver_data);
if (!ctx)
return AE_OK;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/3] perf: xgene: Add support for SoC PMU version 3

2017-04-03 Thread Hoan Tran
This patch set adds support for SoC-wide (AKA uncore) Performance Monitoring
Unit version 3.

It can support up to
 - 2 IOB PMU instances
 - 8 L3C PMU instances
 - 2 MCB PMU instances
 - 8 MCU PMU instances

v2:
 * Split into separate patches
 * Use the function pointers for the PMU leaf functions
 * Parse PMU subnode by the match table
 * Dont allow user change agent id by config1 for SoC PMU v3

Hoan Tran (3):
  perf: xgene: Parse PMU subnode from the match table
  perf: xgene: Move PMU leaf functions into function pointer structure
  perf: xgene: Add support for SoC PMU version 3

 drivers/perf/xgene_pmu.c | 695 +--
 1 file changed, 619 insertions(+), 76 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/3] perf: xgene: Add support for SoC PMU version 3

2017-04-03 Thread Hoan Tran
This patch adds support for SoC-wide (AKA uncore) Performance Monitoring
Unit version 3.

It can support up to
 - 2 IOB PMU instances
 - 8 L3C PMU instances
 - 2 MCB PMU instances
 - 8 MCU PMU instances

Signed-off-by: Hoan Tran 
---
 drivers/perf/xgene_pmu.c | 572 +++
 1 file changed, 524 insertions(+), 48 deletions(-)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index f34fc78..a72814d 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -37,6 +37,8 @@
 
 #define CSW_CSWCR   0x
 #define  CSW_CSWCR_DUALMCB_MASK BIT(0)
+#define  CSW_CSWCR_MCB0_ROUTING(x) (((x) & 0x0C) >> 2)
+#define  CSW_CSWCR_MCB1_ROUTING(x) (((x) & 0x30) >> 4)
 #define MCBADDRMR   0x
 #define  MCBADDRMR_DUALMCU_MODE_MASKBIT(2)
 
@@ -50,8 +52,17 @@
 #define  PCPPMU_INT_L3CBIT(2)
 #define  PCPPMU_INT_IOBBIT(3)
 
+#define  PCPPMU_V3_INTMASK 0x00FF33FF
+#define  PCPPMU_V3_INTENMASK   0x
+#define  PCPPMU_V3_INTCLRMASK  0xFF00CC00
+#define  PCPPMU_V3_INT_MCU 0x00FF
+#define  PCPPMU_V3_INT_MCB 0x0300
+#define  PCPPMU_V3_INT_L3C 0x00FF
+#define  PCPPMU_V3_INT_IOB 0x3000
+
 #define PMU_MAX_COUNTERS   4
-#define PMU_CNT_MAX_PERIOD 0x1ULL
+#define PMU_CNT_MAX_PERIOD 0xULL
+#define PMU_V3_CNT_MAX_PERIOD  0xULL
 #define PMU_OVERFLOW_MASK  0xF
 #define PMU_PMCR_E BIT(0)
 #define PMU_PMCR_P BIT(1)
@@ -73,6 +84,10 @@
 #define PMU_PMOVSR 0xC80
 #define PMU_PMCR   0xE04
 
+/* PMU registers for V3 */
+#define PMU_PMOVSCLR   0xC80
+#define PMU_PMOVSSET   0xCC0
+
 #define to_pmu_dev(p) container_of(p, struct xgene_pmu_dev, pmu)
 #define GET_CNTR(ev)  (ev->hw.idx)
 #define GET_EVENTID(ev)   (ev->hw.config & 0xFFULL)
@@ -119,6 +134,7 @@ struct xgene_pmu {
void __iomem *pcppmu_csr;
u32 mcb_active_mask;
u32 mc_active_mask;
+   u32 l3c_active_mask;
cpumask_t cpu;
raw_spinlock_t lock;
const struct xgene_pmu_ops *ops;
@@ -143,11 +159,13 @@ struct xgene_pmu_data {
 enum xgene_pmu_version {
PCP_PMU_V1 = 1,
PCP_PMU_V2,
+   PCP_PMU_V3,
 };
 
 enum xgene_pmu_dev_type {
PMU_TYPE_L3C = 0,
PMU_TYPE_IOB,
+   PMU_TYPE_IOB_SLOW,
PMU_TYPE_MCB,
PMU_TYPE_MC,
 };
@@ -213,6 +231,56 @@ static ssize_t xgene_pmu_format_show(struct device *dev,
.attrs = mc_pmu_format_attrs,
 };
 
+static struct attribute *l3c_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(l3c_eventid, "config:0-39"),
+   NULL,
+};
+
+static struct attribute *iob_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(iob_eventid, "config:0-47"),
+   NULL,
+};
+
+static struct attribute *iob_slow_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(iob_slow_eventid, "config:0-16"),
+   NULL,
+};
+
+static struct attribute *mcb_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(mcb_eventid, "config:0-35"),
+   NULL,
+};
+
+static struct attribute *mc_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(mc_eventid, "config:0-44"),
+   NULL,
+};
+
+static const struct attribute_group l3c_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = l3c_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group iob_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = iob_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group iob_slow_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = iob_slow_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group mcb_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = mcb_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group mc_pmu_v3_format_attr_group = {
+   .name = "format",
+   .attrs = mc_pmu_v3_format_attrs,
+};
+
 /*
  * sysfs event attributes
  */
@@ -329,6 +397,219 @@ static ssize_t xgene_pmu_event_show(struct device *dev,
.attrs = mc_pmu_events_attrs,
 };
 
+static struct attribute *l3c_pmu_v3_events_attrs[] = {
+   XGENE_PMU_EVENT_ATTR(cycle-count,   0x00),
+   XGENE_PMU_EVENT_ATTR(read-hit,  0x01),
+   XGENE_PMU_EVENT_ATTR(read-miss, 0x02),
+   XGENE_PMU_EVENT_ATTR(index-flush-eviction,  0x03),
+   XGENE_PMU_EVENT_ATTR(write-caused-replacement,  0x04),
+   XGENE_PMU_EVENT_ATTR(write-not-caused-replacement,  0x05),
+   XGENE_PMU_EVENT_ATTR(clean-eviction,0x06),
+   XGENE_PMU_EVENT_ATTR(dirty-eviction,0x07),
+   XGENE_PMU_EVENT_ATTR(reads,  

Re: [PATCH 2/2] perf: xgene: Add support for SoC PMU of next generation of X-Gene

2017-03-30 Thread Hoan Tran
Hi Mark,


On Tue, Mar 14, 2017 at 12:57 PM, Mark Rutland  wrote:
> On Tue, Mar 14, 2017 at 11:06:52AM -0700, Hoan Tran wrote:
>> This patch adds support for SoC-wide (AKA uncore) Performance Monitoring
>> Unit in the next generation of X-Gene SoC.
>>
>> Signed-off-by: Hoan Tran 
>> ---
>>  drivers/perf/xgene_pmu.c | 645 
>> ++-
>>  1 file changed, 575 insertions(+), 70 deletions(-)
>
> That's a very large number of additions, and a very short commit
> message.
>
> Please expand the commit message, outlining the differences in this new
> version of the PMU HW, and the structural changes made to the driver to
> accommodate this.
>
> Additionally, I think that this amount of change should be split into
> separate patches. More on that below.
>
> [...]
>
>>  static inline void xgene_pmu_mask_int(struct xgene_pmu *xgene_pmu)
>>  {
>> - writel(PCPPMU_INTENMASK, xgene_pmu->pcppmu_csr + PCPPMU_INTMASK_REG);
>> + if (xgene_pmu->version == PCP_PMU_V3) {
>> + writel(PCPPMU_V3_INTENMASK,
>> +xgene_pmu->pcppmu_csr + PCPPMU_INTMASK_REG);
>> + } else {
>> + writel(PCPPMU_INTENMASK,
>> +xgene_pmu->pcppmu_csr + PCPPMU_INTMASK_REG);
>> + }
>>  }
>
> Having all these version checks in the leaf functions is horrible,
> especially given that in cases like xgene_pmu_read_counter(), the v3
> behaviour is *substantially* different to the v1/v2 behaviour.
>
> Please use function pointers in the xgene_pmu/xgene_pmu_dev structures
> instead. That way you can clearly separate the v3 code and the v1/v2
> code, and only need to distinguish the two at init time.
>
> Please move the existing code over to function pointers with preparatory
> patches, with the v3 code introduced afterwards.
>
> That applies to almost all cases where you check xgene_pmu->version,
> excluding those that happen during probing.
>
>> -static inline u32 xgene_pmu_read_counter(struct xgene_pmu_dev *pmu_dev, int 
>> idx)
>> +static inline u64 xgene_pmu_read_counter(struct xgene_pmu_dev *pmu_dev, int 
>> idx)
>>  {
>> - return readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
>> + u32 cnt_lo, cnt_hi, cnt_hi2;
>> +
>> + if (pmu_dev->parent->version == PCP_PMU_V3) {
>> + /*
>> +  * v3 has 64-bit counter registers composed by 2 32-bit 
>> registers
>> +  * This can be a problem if the counter increases and carries
>> +  * out of bit [31] between 2 reads. The extra reads would help
>> +  * to prevent this issue.
>> +  */
>> + while (1) {
>> + cnt_hi = readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + 4 + 
>> (8 * idx));
>> + cnt_lo = readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (8 
>> * idx));
>> + cnt_hi2 = readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + 4 
>> + (8 * idx));
>> + if (cnt_hi == cnt_hi2)
>> + return (((u64)cnt_hi << 32) | cnt_lo);
>> + }
>> + }
>> +
>> + return ((u64)readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx)));
>>  }
>
> It would be far simpler and easier to follow, if we did something like:
>
> static inline u64 xgene_pmu_read_counter32(struct xgene_pmu_dev *pmu_dev, int 
> idx)
> {
> return readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
> }
>
> static inline u64 xgene_pmu_read_counter64(struct xgene_pmu_dev *pmu_dev, int 
> idx)
> {
> u32 lo, hi;
>
> do {
> hi = xgene_pmu_read_counter32(dev, 2 * idx);
> lo = xgene_pmu_read_counter32(dev, 2 * idx + 1);
> } while (hi = xgene_pmu_read_counter32(dev, 2 * idx));
>
> return ((u64)hi << 32) | lo;
> }
>
> ... with the prototypes the same, we can assign the pointer to the
> relevant pmu structure.
>
> [...]
>
>> @@ -595,7 +1008,7 @@ static void xgene_perf_event_set_period(struct 
>> perf_event *event)
>>   struct xgene_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
>>   struct hw_perf_event *hw = &event->hw;
>>   /*
>> -  * The X-Gene PMU counters have a period of 2^32. To account for the
>> +  * The X-Gene PMU counters have a period of 2^32 or more. To account 
>> for the
>>* possiblity of extreme interrupt latency we program for a period of
>>* half that. Hopefully 

[PATCH 0/2] perf: xgene: Add support for SoC PMU of next generation of X-Gene

2017-03-14 Thread Hoan Tran
This patch set adds support for SoC-wide (AKA uncore) Performance Monitoring
Unit in the next generation of X-Gene SoC.

Hoan Tran (2):
  Documentation: perf: xgene: Add support for SoC PMU of next generation of 
X-Gene
  perf: xgene: Add support for SoC PMU of next generation of X-Gene

 Documentation/perf/xgene-pmu.txt |  17 +-
 drivers/perf/xgene_pmu.c | 645 ++-
 2 files changed, 586 insertions(+), 76 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] Documentation: perf: xgene: Add support for SoC PMU of next generation of X-Gene

2017-03-14 Thread Hoan Tran
This patch adds support for SoC-wide (AKA uncore) Performance Monitoring
Unit in the next generation of X-Gene SoC.

Signed-off-by: Hoan Tran 
---
 Documentation/perf/xgene-pmu.txt | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/Documentation/perf/xgene-pmu.txt b/Documentation/perf/xgene-pmu.txt
index d7cff44..51f8179 100644
--- a/Documentation/perf/xgene-pmu.txt
+++ b/Documentation/perf/xgene-pmu.txt
@@ -23,12 +23,17 @@ equivalent of "l3c0/config=0x0b/".
 Most of the SoC PMU has a specific list of agent ID used for monitoring
 performance of a specific datapath. For example, agents of a L3 cache can be
 a specific CPU or an I/O bridge. Each PMU has a set of 2 registers capable of
-masking the agents from which the request come from. If the bit with
-the bit number corresponding to the agent is set, the event is counted only if
-it is caused by a request from that agent. Each agent ID bit is inversely 
mapped
-to a corresponding bit in "config1" field. By default, the event will be
-counted for all agent requests (config1 = 0x0). For all the supported agents of
-each PMU, please refer to APM X-Gene User Manual.
+masking the agents from which the request come from. If an agent is enabled,
+the event is counted only if it is caused by a request from that agent.
+ - With SoC PMU version 1 and 2, each agent ID has an enable bit which is
+inversely mapped to a corresponding bit in "config1" field. The value by
+default of config1 is 0.
+ - With Soc PMU version 3, agent ID enable mask is encoded and mapped into
+"config1" field without inversion. The value by default of "config1" is
+defined corresponding to each SoC PMU type.
+By default, the event will be counted for all agent requests. For all the
+supported agents of each PMU and agent configuration, please refer to
+APM X-Gene User Manual.
 
 Each perf driver also provides a "cpumask" sysfs attribute, which contains a
 single CPU ID of the processor which will be used to handle all the PMU events.
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] perf: xgene: Add support for SoC PMU of next generation of X-Gene

2017-03-14 Thread Hoan Tran
This patch adds support for SoC-wide (AKA uncore) Performance Monitoring
Unit in the next generation of X-Gene SoC.

Signed-off-by: Hoan Tran 
---
 drivers/perf/xgene_pmu.c | 645 ++-
 1 file changed, 575 insertions(+), 70 deletions(-)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 35b5289..21c7e7f 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -35,10 +35,13 @@
 #include 
 #include 
 
-#define CSW_CSWCR   0x
-#define  CSW_CSWCR_DUALMCB_MASK BIT(0)
-#define MCBADDRMR   0x
-#define  MCBADDRMR_DUALMCU_MODE_MASKBIT(2)
+#define CSW_CSWCR  0x
+#define  CSW_CSWCR_DUALMCB_MASKBIT(0)
+#define  CSW_CSWCR_MCB0_ROUTING(x) (((x) & 0x0C) >> 2)
+#define  CSW_CSWCR_MCB1_ROUTING(x) (((x) & 0x30) >> 4)
+
+#define MCBADDRMR  0x
+#define  MCBADDRMR_DUALMCU_MODE_MASK   BIT(2)
 
 #define PCPPMU_INTSTATUS_REG   0x000
 #define PCPPMU_INTMASK_REG 0x004
@@ -50,8 +53,17 @@
 #define  PCPPMU_INT_L3CBIT(2)
 #define  PCPPMU_INT_IOBBIT(3)
 
+#define  PCPPMU_V3_INTMASK 0x00FF33FF
+#define  PCPPMU_V3_INTENMASK   0x
+#define  PCPPMU_V3_INTCLRMASK  0xFF00CC00
+#define  PCPPMU_V3_INT_MCU 0x00FF
+#define  PCPPMU_V3_INT_MCB 0x0300
+#define  PCPPMU_V3_INT_L3C 0x00FF
+#define  PCPPMU_V3_INT_IOB 0x3000
+
 #define PMU_MAX_COUNTERS   4
-#define PMU_CNT_MAX_PERIOD 0x1ULL
+#define PMU_CNT_MAX_PERIOD 0xULL
+#define PMU_V3_CNT_MAX_PERIOD  0xULL
 #define PMU_OVERFLOW_MASK  0xF
 #define PMU_PMCR_E BIT(0)
 #define PMU_PMCR_P BIT(1)
@@ -73,11 +85,26 @@
 #define PMU_PMOVSR 0xC80
 #define PMU_PMCR   0xE04
 
-#define to_pmu_dev(p) container_of(p, struct xgene_pmu_dev, pmu)
-#define GET_CNTR(ev)  (ev->hw.idx)
-#define GET_EVENTID(ev)   (ev->hw.config & 0xFFULL)
-#define GET_AGENTID(ev)   (ev->hw.config_base & 0xUL)
-#define GET_AGENT1ID(ev)  ((ev->hw.config_base >> 32) & 0xUL)
+/* PMU registers for V3 */
+#define PMU_PMOVSCLR   0xC80
+#define PMU_PMOVSSET   0xCC0
+#define PMU_PMAUXR00xD80
+#define PMU_PMAUXR10xD84
+#define PMU_PMAUXR20xD88
+#define PMU_PMAUXR30xD8C
+
+/* Default PMU agent ID values for V3 */
+#define PCPPMU_V3_L3C_DEFAULT_AGENTID  0x0ULL
+#define PCPPMU_V3_IOB_DEFAULT_AGENTID  0x0ULL
+#define PCPPMU_V3_IOB_SLOW_DEFAULT_AGENTID 0x0ULL
+#define PCPPMU_V3_MCB_DEFAULT_AGENTID  0x1ULL
+#define PCPPMU_V3_MC_DEFAULT_AGENTID   0xFF00ULL
+
+#define to_pmu_dev(p)  container_of(p, struct xgene_pmu_dev, pmu)
+#define GET_CNTR(ev)   (ev->hw.idx)
+#define GET_EVENTID(ev)(ev->hw.config & 0xFFULL)
+#define GET_AGENTID(ev)(ev->hw.config_base & 0xUL)
+#define GET_AGENT1ID(ev)   ((ev->hw.config_base >> 32) & 0xUL)
 
 struct hw_pmu_info {
u32 type;
@@ -92,6 +119,7 @@ struct xgene_pmu_dev {
u8 max_counters;
DECLARE_BITMAP(cntr_assign_mask, PMU_MAX_COUNTERS);
u64 max_period;
+   u64 default_agentid;
const struct attribute_group **attr_groups;
struct perf_event *pmu_counter_event[PMU_MAX_COUNTERS];
 };
@@ -102,6 +130,7 @@ struct xgene_pmu {
void __iomem *pcppmu_csr;
u32 mcb_active_mask;
u32 mc_active_mask;
+   u32 l3c_active_mask;
cpumask_t cpu;
raw_spinlock_t lock;
struct list_head l3cpmus;
@@ -125,11 +154,13 @@ struct xgene_pmu_data {
 enum xgene_pmu_version {
PCP_PMU_V1 = 1,
PCP_PMU_V2,
+   PCP_PMU_V3,
 };
 
 enum xgene_pmu_dev_type {
PMU_TYPE_L3C = 0,
PMU_TYPE_IOB,
+   PMU_TYPE_IOB_SLOW,
PMU_TYPE_MCB,
PMU_TYPE_MC,
 };
@@ -195,6 +226,60 @@ static ssize_t xgene_pmu_format_show(struct device *dev,
.attrs = mc_pmu_format_attrs,
 };
 
+static struct attribute *l3c_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(l3c_eventid, "config:0-39"),
+   XGENE_PMU_FORMAT_ATTR(l3c_agentid, "config1:0-31"),
+   NULL,
+};
+
+static struct attribute *iob_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(iob_eventid, "config:0-47"),
+   XGENE_PMU_FORMAT_ATTR(iob_agentid, "config1:0-63"),
+   NULL,
+};
+
+static struct attribute *iob_slow_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(iob_slow_eventid, "config:0-16"),
+   XGENE_PMU_FORMAT_ATTR(iob_slow_agentid, "config1:0-63"),
+   NULL,
+};
+
+static struct attribute *mcb_pmu_v3_format_attrs[] = {
+   XGENE_PMU_FORMAT_ATTR(mcb_eventid, "config:0-35"),
+   XGENE_PMU_FORMAT_

Re: [PATCH] hwmon: xgene: access mailbox as RAM

2016-09-09 Thread Hoan Tran
On Fri, Sep 9, 2016 at 1:50 PM, Arnd Bergmann  wrote:
> On Friday, September 9, 2016 1:43:17 PM CEST Hoan Tran wrote:
>>
>> > * Are you sure you don't need any smp_rmb()/smp_wmb() barriers
>> > between the accesses?
>>
>> No, we don't need a strict read/write during access PCC subspace. Just
>> make sure all access is committed before PCC send message to the
>> platform which done by PCC mailbox driver.
>>
>
> Ok, got it. The PCC mailbox driver presumably uses writel() to
> send the message, and that implies the necessary barrier
> (unlike writel_relaxed), right?

Yes,

Hoan
>
> Arnd
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] hwmon: xgene: access mailbox as RAM

2016-09-09 Thread Hoan Tran
Hi Arnd,

On Fri, Sep 9, 2016 at 1:10 PM, Arnd Bergmann  wrote:
> The newly added hwmon driver fails to build in an allmodconfig
> kernel:
>
>   ERROR: "memblock_is_memory" [drivers/hwmon/xgene-hwmon.ko] undefined!
>
> According to comments in the code, the mailbox is a shared memory region,
> not a set of MMIO registers, so we should use memremap() for mapping it
> instead of ioremap or acpi_os_ioremap, and pointer dereferences instead
> of readl/writel.
>
> The driver already uses plain kernel pointers, so it's a bit unusual
> to work with functions that operate on __iomem pointers, and this
> fixes that part too.
>
> I'm using READ_ONCE/WRITE_ONCE here to keep the existing behavior
> regarding the ordering of the accesses from the CPU, but note that
> there are no barriers (also unchanged from before).
>
> I'm also keeping the endianess behavior, though I'm unsure whether
> the message data was supposed to be in LE32 format in the first
> place, it's possible this was meant to be interpreted as a byte
> stream instead.
>
> Signed-off-by: Arnd Bergmann 
> ---
> v2: use write-back mapping instead of write-thru,
> minor coding style changes
>
> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
> index bc78a5d10182..e5470bd49067 100644
> --- a/drivers/hwmon/xgene-hwmon.c
> +++ b/drivers/hwmon/xgene-hwmon.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -34,7 +35,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +
>  #include 
>
>  /* SLIMpro message defines */
> @@ -126,10 +127,10 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>  {
> u16 ret, val;
>
> -   val = readw_relaxed(addr);
> +   val = le16_to_cpu(READ_ONCE(*addr));
> ret = val & mask;
> val &= ~mask;
> -   writew_relaxed(val, addr);
> +   WRITE_ONCE(*addr, cpu_to_le16(val));
>
> return ret;
>  }
> @@ -137,7 +138,7 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>  static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg)
>  {
> struct acpi_pcct_shared_memory *generic_comm_base = 
> ctx->pcc_comm_addr;
> -   void *ptr = generic_comm_base + 1;
> +   u32 *ptr = (void *)(generic_comm_base + 1);
> int rc, i;
> u16 val;
>
> @@ -146,21 +147,21 @@ static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev 
> *ctx, u32 *msg)
> ctx->resp_pending = true;
>
> /* Write signature for subspace */
> -   writel_relaxed(PCC_SIGNATURE_MASK | ctx->mbox_idx,
> -  &generic_comm_base->signature);
> +   WRITE_ONCE(generic_comm_base->signature,
> +  cpu_to_le32(PCC_SIGNATURE_MASK | ctx->mbox_idx));
>
> /* Write to the shared command region */
> -   writew_relaxed(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT,
> -  &generic_comm_base->command);
> +   WRITE_ONCE(generic_comm_base->command,
> +  cpu_to_le16(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT));
>
> /* Flip CMD COMPLETE bit */
> -   val = readw_relaxed(&generic_comm_base->status);
> +   val = le16_to_cpu(READ_ONCE(generic_comm_base->status));
> val &= ~PCCS_CMD_COMPLETE;
> -   writew_relaxed(val, &generic_comm_base->status);
> +   WRITE_ONCE(generic_comm_base->status, cpu_to_le16(val));
>
> /* Copy the message to the PCC comm space */
> for (i = 0; i < sizeof(struct slimpro_resp_msg) / 4; i++)
> -   writel_relaxed(msg[i], ptr + i * 4);
> +   WRITE_ONCE(ptr[i], cpu_to_le32(msg[i]));
>
> /* Ring the doorbell */
> rc = mbox_send_message(ctx->mbox_chan, msg);
> @@ -652,9 +653,9 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
>  */
> ctx->comm_base_addr = cppc_ss->base_address;
> if (ctx->comm_base_addr) {
> -   ctx->pcc_comm_addr =
> -   acpi_os_ioremap(ctx->comm_base_addr,
> -       cppc_ss->length);
> +   ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
> +   cppc_ss->length,
> +   MEMREMAP_WB);
> } else {
> dev_err(&pdev->dev, "Failed to get PCC comm 
> region\n");
> rc = -ENODEV;
>

Acked-by: Hoan Tran 
Tested-by: Hoan Tran 

Thanks
Hoan
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] hwmon: xgene: access mailbox as RAM

2016-09-09 Thread Hoan Tran
On Fri, Sep 9, 2016 at 12:58 PM, Arnd Bergmann  wrote:
> On Friday, September 9, 2016 12:24:32 PM CEST Hoan Tran wrote:
>> On Fri, Sep 9, 2016 at 8:38 AM, Arnd Bergmann  wrote:
>> > The newly added hwmon driver fails to build in an allmodconfig
>> > index bc78a5d10182..e834dfb3acca 100644
>> > --- a/drivers/hwmon/xgene-hwmon.c
>> > +++ b/drivers/hwmon/xgene-hwmon.c
>> > @@ -34,7 +34,8 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > -#include 
>> > +#include 
>>
>> Alphabetical order.
>>
>> > struct acpi_pcct_shared_memory *generic_comm_base = 
>> > ctx->pcc_comm_addr;
>> > -   void *ptr = generic_comm_base + 1;
>> > +   u32 *ptr = (void*)(generic_comm_base + 1);
>>
>> Space before "*".
>
> Ok.
>
>> > @@ -652,9 +653,9 @@ static int xgene_hwmon_probe(struct platform_device 
>> > *pdev)
>> >  */
>> > ctx->comm_base_addr = cppc_ss->base_address;
>> > if (ctx->comm_base_addr) {
>> > -   ctx->pcc_comm_addr =
>> > -   
>> > acpi_os_ioremap(ctx->comm_base_addr,
>> > -   cppc_ss->length);
>> > +   ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
>> > +   cppc_ss->length,
>> > +   MEMREMAP_WT);
>>
>> It should be MEMREMAP_WB. As mailbox shared memory is on RAM and our
>> co-processor is also in the coherency domain.
>
> Right, I was wondering about this, since I could not figure out what
> the other side is (hardware, service processor or firmware).
> So MEMREMAP_WB makes sense here.
>
> Two more questions:
>
> * Any comment on the byte ordering of the data in this line:
>
> /* Copy the message to the PCC comm space */
> for (i = 0; i < sizeof(struct slimpro_resp_msg) / 4; i++)
> -   writel_relaxed(msg[i], ptr + i * 4);
> +   WRITE_ONCE(ptr[i], cpu_to_le32(msg[i]));
>
> This assumes that the old code was correct even when running on
> big-endian kernels and the message data consists of 32-bit data words.
> If the message has some other format instead, we would need to treat
> this as a byte stream and not do swapping here but instead do it
> (if any) in the code that reads or writes the actual data here.

This is 32-bit data words.

>
> * Are you sure you don't need any smp_rmb()/smp_wmb() barriers
> between the accesses?

No, we don't need a strict read/write during access PCC subspace. Just
make sure all access is committed before PCC send message to the
platform which done by PCC mailbox driver.

Thanks
Hoan

>
> Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] hwmon: xgene: access mailbox as RAM

2016-09-09 Thread Hoan Tran
Hi Arnd,

On Fri, Sep 9, 2016 at 8:38 AM, Arnd Bergmann  wrote:
> The newly added hwmon driver fails to build in an allmodconfig
> kernel:
>
>   1  ERROR: "memblock_is_memory" [drivers/hwmon/xgene-hwmon.ko] undefined!
>
> According to comments in the code, the mailbox is a shared memory region,
> not a set of MMIO registers, so we should use memremap() for mapping it
> instead of ioremap or acpi_os_ioremap, and pointer dereferences instead
> of readl/writel.
>
> The driver already uses plain kernel pointers, so it's a bit unusual
> to work with functions that operate on __iomem pointers, and this
> fixes that part too.
>
> I'm using READ_ONCE/WRITE_ONCE here to keep the existing behavior
> regarding the ordering of the accesses from the CPU, but note that
> there are no barriers (also unchanged from before).
>
> I'm also keeping the endianess behavior, though I'm unsure whether
> the message data was supposed to be in LE32 format in the first
> place, it's possible this was meant to be interpreted as a byte
> stream instead.
>
> Signed-off-by: Arnd Bergmann 
>
> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
> index bc78a5d10182..e834dfb3acca 100644
> --- a/drivers/hwmon/xgene-hwmon.c
> +++ b/drivers/hwmon/xgene-hwmon.c
> @@ -34,7 +34,8 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 

Alphabetical order.

> +
>  #include 
>
>  /* SLIMpro message defines */
> @@ -126,10 +127,10 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>  {
> u16 ret, val;
>
> -   val = readw_relaxed(addr);
> +   val = le16_to_cpu(READ_ONCE(*addr));
> ret = val & mask;
> val &= ~mask;
> -   writew_relaxed(val, addr);
> +   WRITE_ONCE(*addr, cpu_to_le16(val));
>
> return ret;
>  }
> @@ -137,7 +138,7 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>  static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg)
>  {
> struct acpi_pcct_shared_memory *generic_comm_base = 
> ctx->pcc_comm_addr;
> -   void *ptr = generic_comm_base + 1;
> +   u32 *ptr = (void*)(generic_comm_base + 1);

Space before "*".

> int rc, i;
> u16 val;
>
> @@ -146,21 +147,21 @@ static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev 
> *ctx, u32 *msg)
> ctx->resp_pending = true;
>
> /* Write signature for subspace */
> -   writel_relaxed(PCC_SIGNATURE_MASK | ctx->mbox_idx,
> -  &generic_comm_base->signature);
> +   WRITE_ONCE(generic_comm_base->signature,
> +  cpu_to_le32(PCC_SIGNATURE_MASK | ctx->mbox_idx));
>
> /* Write to the shared command region */
> -   writew_relaxed(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT,
> -  &generic_comm_base->command);
> +   WRITE_ONCE(generic_comm_base->command,
> +  cpu_to_le16(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT));
>
> /* Flip CMD COMPLETE bit */
> -   val = readw_relaxed(&generic_comm_base->status);
> +   val = le16_to_cpu(READ_ONCE(generic_comm_base->status));
> val &= ~PCCS_CMD_COMPLETE;
> -   writew_relaxed(val, &generic_comm_base->status);
> +   WRITE_ONCE(generic_comm_base->status, cpu_to_le16(val));
>
> /* Copy the message to the PCC comm space */
> for (i = 0; i < sizeof(struct slimpro_resp_msg) / 4; i++)
> -   writel_relaxed(msg[i], ptr + i * 4);
> +   WRITE_ONCE(ptr[i], cpu_to_le32(msg[i]));
>
> /* Ring the doorbell */
> rc = mbox_send_message(ctx->mbox_chan, msg);
> @@ -652,9 +653,9 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
>  */
> ctx->comm_base_addr = cppc_ss->base_address;
> if (ctx->comm_base_addr) {
> -   ctx->pcc_comm_addr =
> -   acpi_os_ioremap(ctx->comm_base_addr,
> -   cppc_ss->length);
> +   ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
> +   cppc_ss->length,
> +   MEMREMAP_WT);

It should be MEMREMAP_WB. As mailbox shared memory is on RAM and our
co-processor is also in the coherency domain.

Thanks
Hoan

> } else {
> dev_err(&pdev->dev, "Failed to get PCC comm 
> region\n");
> rc = -ENODEV;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] hwmon: xgene: access mailbox as RAM

2016-09-09 Thread Hoan Tran
On Fri, Sep 9, 2016 at 9:58 AM, Guenter Roeck  wrote:
> Hi Arnd,
>
> On Fri, Sep 09, 2016 at 05:38:58PM +0200, Arnd Bergmann wrote:
>> The newly added hwmon driver fails to build in an allmodconfig
>> kernel:
>>
>>   1  ERROR: "memblock_is_memory" [drivers/hwmon/xgene-hwmon.ko] 
>> undefined!
>>
>> According to comments in the code, the mailbox is a shared memory region,
>> not a set of MMIO registers, so we should use memremap() for mapping it
>> instead of ioremap or acpi_os_ioremap, and pointer dereferences instead
>> of readl/writel.
>>
>> The driver already uses plain kernel pointers, so it's a bit unusual
>> to work with functions that operate on __iomem pointers, and this
>> fixes that part too.
>>
>> I'm using READ_ONCE/WRITE_ONCE here to keep the existing behavior
>> regarding the ordering of the accesses from the CPU, but note that
>> there are no barriers (also unchanged from before).
>>
>> I'm also keeping the endianess behavior, though I'm unsure whether
>> the message data was supposed to be in LE32 format in the first
>> place, it's possible this was meant to be interpreted as a byte
>> stream instead.
>>
>> Signed-off-by: Arnd Bergmann 
>>
>
> Thanks a lot for looking into this.
>
> I'll apply this patch to address the build problem. Much better than
> my rude "depends on BROKEN". It would be great to get a Tested-by:
> from someone with access to the hardware.
>

Hi Arnd and Guenter,

Thanks for the patch. I'm testing it out.

Hoan

> Guenter
>
>> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
>> index bc78a5d10182..e834dfb3acca 100644
>> --- a/drivers/hwmon/xgene-hwmon.c
>> +++ b/drivers/hwmon/xgene-hwmon.c
>> @@ -34,7 +34,8 @@
>>  #include 
>>  #include 
>>  #include 
>> -#include 
>> +#include 
>> +
>>  #include 
>>
>>  /* SLIMpro message defines */
>> @@ -126,10 +127,10 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>>  {
>>   u16 ret, val;
>>
>> - val = readw_relaxed(addr);
>> + val = le16_to_cpu(READ_ONCE(*addr));
>>   ret = val & mask;
>>   val &= ~mask;
>> - writew_relaxed(val, addr);
>> + WRITE_ONCE(*addr, cpu_to_le16(val));
>>
>>   return ret;
>>  }
>> @@ -137,7 +138,7 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>>  static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg)
>>  {
>>   struct acpi_pcct_shared_memory *generic_comm_base = ctx->pcc_comm_addr;
>> - void *ptr = generic_comm_base + 1;
>> + u32 *ptr = (void*)(generic_comm_base + 1);
>>   int rc, i;
>>   u16 val;
>>
>> @@ -146,21 +147,21 @@ static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev 
>> *ctx, u32 *msg)
>>   ctx->resp_pending = true;
>>
>>   /* Write signature for subspace */
>> - writel_relaxed(PCC_SIGNATURE_MASK | ctx->mbox_idx,
>> -&generic_comm_base->signature);
>> + WRITE_ONCE(generic_comm_base->signature,
>> +cpu_to_le32(PCC_SIGNATURE_MASK | ctx->mbox_idx));
>>
>>   /* Write to the shared command region */
>> - writew_relaxed(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT,
>> -&generic_comm_base->command);
>> + WRITE_ONCE(generic_comm_base->command,
>> +cpu_to_le16(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT));
>>
>>   /* Flip CMD COMPLETE bit */
>> - val = readw_relaxed(&generic_comm_base->status);
>> + val = le16_to_cpu(READ_ONCE(generic_comm_base->status));
>>   val &= ~PCCS_CMD_COMPLETE;
>> - writew_relaxed(val, &generic_comm_base->status);
>> + WRITE_ONCE(generic_comm_base->status, cpu_to_le16(val));
>>
>>   /* Copy the message to the PCC comm space */
>>   for (i = 0; i < sizeof(struct slimpro_resp_msg) / 4; i++)
>> - writel_relaxed(msg[i], ptr + i * 4);
>> + WRITE_ONCE(ptr[i], cpu_to_le32(msg[i]));
>>
>>   /* Ring the doorbell */
>>   rc = mbox_send_message(ctx->mbox_chan, msg);
>> @@ -652,9 +653,9 @@ static int xgene_hwmon_probe(struct platform_device 
>> *pdev)
>>*/
>>   ctx->comm_base_addr = cppc_ss->base_address;
>>   if (ctx->comm_base_addr) {
>> - ctx->pcc_comm_addr =
>> - acpi_os_ioremap(ctx->comm_base_addr,
>> - cppc_ss->length);
>> + ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
>> + cppc_ss->length,
>> + MEMREMAP_WT);
>>   } else {
>>   dev_err(&pdev->dev, "Failed to get PCC comm region\n");
>>   rc = -ENODEV;
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/3] hwmon: xgene: Add hwmon driver

2016-08-01 Thread Hoan Tran
Hi,

On Mon, Aug 1, 2016 at 6:21 AM, kbuild test robot  wrote:
> Hi Hoan,
>
> [auto build test ERROR on hwmon/hwmon-next]
> [also build test ERROR on v4.7]
> [cannot apply to next-20160801]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Hoan-Tran/hwmon-xgene-Add-support-for-X-Gene-hwmon-driver/20160725-015356
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git 
> hwmon-next
> config: arm64-allmodconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
> reproduce:
> wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>  -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm64
>
> All errors (new ones prefixed by >>):
>
>>> drivers/hwmon/xgene-hwmon.c:38:22: fatal error: acpi/pcc.h: No such file or 
>>> directory
>compilation terminated.

This driver should be built with kernel 4.8 and above where the
"pcc.h" file is available.

Thanks
Hoan

>
> vim +38 drivers/hwmon/xgene-hwmon.c
>
> 32  #include 
> 33  #include 
> 34  #include 
> 35  #include 
> 36  #include 
> 37  #include 
>   > 38  #include 
> 39
> 40  /* SLIMpro message defines */
> 41  #define MSG_TYPE_DBG0
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: hwmon: xgene: Add support for X-Gene hwmon driver

2016-07-23 Thread Hoan Tran
Hi Itaru,

On Sat, Jul 23, 2016 at 10:04 PM, Itaru Kitayama
 wrote:
> Hi Hoan,
>
> I didn't mention in my previous post, though I think I'm using fairly recent
> version of Tianocore F/W, and I can confirm there's the PCCT table,
>
> [0.00] ACPI: PCCT 0x0047FA826000 000300 (v01 APMXGENE
> 0003  0113)
>
> is this still not enough? If that's the case, I'll talk to APM for
> their latest development Tianocore version.

Your version is missing the "pcc-channel" property of hwmon inside
ACPI Table. You need a newer version of Tianocore.

> Also, if you or your folks can fold your patch set into APM's xgene-next
> GitHub repo, that would help me to keep up with the development.

xgene-next is currently used for device tree patch only.

Thanks
Hoan

>
> Itaru
>
>
> On 7/24/16 12:00 PM, Hoan Tran wrote:
>>
>> You need the new firmware and BIOS which support ACPI PCC mailbox.
>>
>> Thanks
>> Hoan
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: hwmon: xgene: Add support for X-Gene hwmon driver

2016-07-23 Thread Hoan Tran
Hi Itaru,


> On Jul 23, 2016, at 16:51, Itaru Kitayama  wrote:
> 
> Hi Hoan,
> 
> I've been testing your patch set on a Rev B0 system with ACPI, in dmesg
> I see:
> 
> [1.546444] xgene-slimpro-i2c APMC0D40:00: i2c mailbox channel request 
> failed
> [1.570062] xgene-slimpro-hwmon APMC0D29:00: no pcc-channel property
> 
> what other packages or subsystems do we need to make this work on ACPI-based 
> systems?

You need the new firmware and BIOS which support ACPI PCC mailbox.

Thanks
Hoan

> 
> Itaru
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 0/3] hwmon: xgene: Add support for X-Gene hwmon driver

2016-07-22 Thread Hoan Tran
Hi Guenter,

On Thu, Jul 21, 2016 at 6:30 PM, Guenter Roeck  wrote:
> On 07/21/2016 03:37 PM, Hoan Tran wrote:
>>
>> This patch set adds hardware temperature and power reading support for
>> APM X-Gene SoC using the mailbox communication interface.
>> For device tree, it is the standard DT mailbox.
>> For ACPI, it is the PCC mailbox.
>>
>> For ACPI, this patch is built on top and depends on patch[1]:
>> [1] http://www.spinics.net/lists/linux-acpi/msg66041.html
>>   - [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace
>> type 2
>>
>
> One thought: You might consider taking ACPI support out of the driver for
> now,
> and add it back in after the dependent patch was accepted.
>

Thanks for your suggestion. Rafael agreed to apply this patch as the
link below but he didn't do it yet.
http://www.spinics.net/lists/linux-acpi/msg67100.html
Let me check with him again.

Thanks
Hoan

> Guenter
>
>
>> v4
>>   - Return 0 if driver registers successfully
>>
>> v3
>>   - Order include files alphabetically
>>   - Use sign_extend32() to decode temperature
>>   - Use DEVICE_ATTR_RO() for temperature and power attributes
>>   - Temperature and power attributes start with index 1
>>   - Use !!amsg->param2
>>   - Fix checkpatch WARNING with --strict flag
>>   - Use hwmon_device_register_with_groups()
>>   - Check invalid sensor data
>>
>> v2
>>   - Increase power reading accurateness by using 2 registers
>> (a register for Watt, another for milli-Watt)
>>   - Remove power reading for SoC
>>   - Fix review comments from Guenter
>>
>> v1
>>   - Initial
>>
>> Hoan Tran (3):
>>Documentation: dtb: xgene: Add hwmon dts binding documentation
>>hwmon: xgene: Add hwmon driver
>>arm64: dts: apm: Add X-Gene SoC hwmon to device tree
>>
>>   .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  |  14 +
>>   Documentation/hwmon/xgene-hwmon|  30 +
>>   arch/arm64/boot/dts/apm/apm-shadowcat.dtsi |   5 +
>>   arch/arm64/boot/dts/apm/apm-storm.dtsi |   5 +
>>   drivers/hwmon/Kconfig  |   7 +
>>   drivers/hwmon/Makefile |   1 +
>>   drivers/hwmon/xgene-hwmon.c| 755
>> +
>>   7 files changed, 817 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
>>   create mode 100644 Documentation/hwmon/xgene-hwmon
>>   create mode 100644 drivers/hwmon/xgene-hwmon.c
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 2/3] hwmon: xgene: Add hwmon driver

2016-07-21 Thread Hoan Tran
This patch adds hardware temperature and power reading support for
APM X-Gene SoC using the mailbox communication interface.

Signed-off-by: Hoan Tran 
Reviewed-by: Guenter Roeck 
---
 Documentation/hwmon/xgene-hwmon |  30 ++
 drivers/hwmon/Kconfig   |   7 +
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/xgene-hwmon.c | 755 
 4 files changed, 793 insertions(+)
 create mode 100644 Documentation/hwmon/xgene-hwmon
 create mode 100644 drivers/hwmon/xgene-hwmon.c

diff --git a/Documentation/hwmon/xgene-hwmon b/Documentation/hwmon/xgene-hwmon
new file mode 100644
index 000..6ec50ed
--- /dev/null
+++ b/Documentation/hwmon/xgene-hwmon
@@ -0,0 +1,30 @@
+Kernel driver xgene-hwmon
+
+
+Supported chips:
+ * APM X-Gene SoC
+
+Description
+---
+
+This driver adds hardware temperature and power reading support for
+APM X-Gene SoC using the mailbox communication interface.
+For device tree, it is the standard DT mailbox.
+For ACPI, it is the PCC mailbox.
+
+The following sensors are supported
+
+  * Temperature
+- SoC on-die temperature in milli-degree C
+- Alarm when high/over temperature occurs
+  * Power
+- CPU power in uW
+- IO power in uW
+
+sysfs-Interface
+---
+
+temp0_input - SoC on-die temperature (milli-degree C)
+temp0_critical_alarm - An 1 would indicates on-die temperature exceeded 
threshold
+power0_input - CPU power in (uW)
+power1_input - IO power in (uW)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ff94007..55218c6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1787,6 +1787,13 @@ config SENSORS_ULTRA45
  This driver provides support for the Ultra45 workstation environmental
  sensors.
 
+config SENSORS_XGENE
+   tristate "APM X-Gene SoC hardware monitoring driver"
+   depends on XGENE_SLIMPRO_MBOX || PCC
+   help
+ If you say yes here you get support for the temperature
+ and power sensors for APM X-Gene SoC.
+
 if ACPI
 
 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 2ef5b7c..a2460dd 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -162,6 +162,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)+= w83l786ng.o
 obj-$(CONFIG_SENSORS_WM831X)   += wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)   += wm8350-hwmon.o
+obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
 
 obj-$(CONFIG_PMBUS)+= pmbus/
 
diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
new file mode 100644
index 000..bc78a5d
--- /dev/null
+++ b/drivers/hwmon/xgene-hwmon.c
@@ -0,0 +1,755 @@
+/*
+ * APM X-Gene SoC Hardware Monitoring Driver
+ *
+ * Copyright (c) 2016, Applied Micro Circuits Corporation
+ * Author: Loc Ho 
+ * Hoan Tran 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * This driver provides the following features:
+ *  - Retrieve CPU total power (uW)
+ *  - Retrieve IO total power (uW)
+ *  - Retrieve SoC temperature (milli-degree C) and alarm
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* SLIMpro message defines */
+#define MSG_TYPE_DBG   0
+#define MSG_TYPE_ERR   7
+#define MSG_TYPE_PWRMGMT   9
+
+#define MSG_TYPE(v)(((v) & 0xF000) >> 28)
+#define MSG_TYPE_SET(v)(((v) << 28) & 0xF000)
+#define MSG_SUBTYPE(v) (((v) & 0x0F00) >> 24)
+#define MSG_SUBTYPE_SET(v) (((v) << 24) & 0x0F00)
+
+#define DBG_SUBTYPE_SENSOR_READ4
+#define SENSOR_RD_MSG  0x04FFE902
+#define SENSOR_RD_EN_ADDR(a)   ((a) & 0x000F)
+#define PMD_PWR_REG0x20
+#define PMD_PWR_MW_REG 0x26
+#define SOC_PWR_REG0x21
+#define SOC_PWR_MW_REG 0x27
+#define SOC_TEMP_REG   0x10
+
+#define TEMP_NEGATIVE_BIT  8
+#define SENSOR_INVALID_DATABIT(15)
+
+#define PWRMGMT_SUBTYPE_TPC1
+#define TPC_ALARM  2
+#define TPC_GET_ALARM   

[PATCH v4 3/3] arm64: dts: apm: Add X-Gene SoC hwmon to device tree

2016-07-21 Thread Hoan Tran
This patch adds DT node to enable hwmon driver for APM X-Gene SoC.

Signed-off-by: Hoan Tran 
Acked-by: Guenter Roeck 
---
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 5 +
 arch/arm64/boot/dts/apm/apm-storm.dtsi | 5 +
 2 files changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi 
b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
index c569f76..a5935f5 100644
--- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
@@ -472,6 +472,11 @@
mboxes = <&mailbox 0>;
};
 
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
+
serial0: serial@1060 {
device_type = "serial";
compatible = "ns16550";
diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi 
b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index 5147d76..f8ea5b5 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -716,6 +716,11 @@
mboxes = <&mailbox 0>;
};
 
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
+
serial0: serial@1c02 {
status = "disabled";
device_type = "serial";
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 0/3] hwmon: xgene: Add support for X-Gene hwmon driver

2016-07-21 Thread Hoan Tran
This patch set adds hardware temperature and power reading support for
APM X-Gene SoC using the mailbox communication interface.
For device tree, it is the standard DT mailbox.
For ACPI, it is the PCC mailbox.

For ACPI, this patch is built on top and depends on patch[1]:
[1] http://www.spinics.net/lists/linux-acpi/msg66041.html
 - [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2

v4
 - Return 0 if driver registers successfully
 
v3
 - Order include files alphabetically
 - Use sign_extend32() to decode temperature
 - Use DEVICE_ATTR_RO() for temperature and power attributes
 - Temperature and power attributes start with index 1
 - Use !!amsg->param2
 - Fix checkpatch WARNING with --strict flag
 - Use hwmon_device_register_with_groups()
 - Check invalid sensor data

v2
 - Increase power reading accurateness by using 2 registers
(a register for Watt, another for milli-Watt)
 - Remove power reading for SoC
 - Fix review comments from Guenter

v1
 - Initial

Hoan Tran (3):
  Documentation: dtb: xgene: Add hwmon dts binding documentation
  hwmon: xgene: Add hwmon driver
  arm64: dts: apm: Add X-Gene SoC hwmon to device tree

 .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  |  14 +
 Documentation/hwmon/xgene-hwmon|  30 +
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi |   5 +
 arch/arm64/boot/dts/apm/apm-storm.dtsi |   5 +
 drivers/hwmon/Kconfig  |   7 +
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/xgene-hwmon.c| 755 +
 7 files changed, 817 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
 create mode 100644 Documentation/hwmon/xgene-hwmon
 create mode 100644 drivers/hwmon/xgene-hwmon.c

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 1/3] Documentation: dtb: xgene: Add hwmon dts binding documentation

2016-07-21 Thread Hoan Tran
This patch adds the APM X-Gene hwmon device tree node documentation.

Signed-off-by: Hoan Tran 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  | 14 ++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt

diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt 
b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
new file mode 100644
index 000..59b3855
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
@@ -0,0 +1,14 @@
+APM X-Gene hwmon driver
+
+APM X-Gene SOC sensors are accessed over the "SLIMpro" mailbox.
+
+Required properties :
+ - compatible : should be "apm,xgene-slimpro-hwmon"
+ - mboxes : use the label reference for the mailbox as the first parameter.
+   The second parameter is the channel number.
+
+Example :
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/3] hwmon: xgene: Add hwmon driver

2016-07-21 Thread Hoan Tran
Hi Guenter,

On Thu, Jul 21, 2016 at 3:09 PM, Guenter Roeck  wrote:
> On Thu, Jul 21, 2016 at 01:55:56PM -0700, Hoan Tran wrote:
>> This patch adds hardware temperature and power reading support for
>> APM X-Gene SoC using the mailbox communication interface.
>>
>> Signed-off-by: Hoan Tran 
>> ---
>
> [ ... ]
>
>> +
>> + dev_info(&pdev->dev, "APM X-Gene SoC HW monitor driver registered\n");
>> +
>> + return rc;
>> +
>
> Nitpick: rc == 0 here, so return 0; is a better choice.
> Otherwise looks good.

Yes, I'll fix it then send v4 shortly.

>
> Reviewed-by: Guenter Roeck 
>
> Note that I can not apply the patch at this time due to its dependency.

Thanks ! I'll let you know when Rafael applies that dependent patch
into linux-next.

Thanks
Hoan

>
> Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 3/3] arm64: dts: apm: Add X-Gene SoC hwmon to device tree

2016-07-21 Thread Hoan Tran
This patch adds DT node to enable hwmon driver for APM X-Gene SoC.

Signed-off-by: Hoan Tran 
---
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 5 +
 arch/arm64/boot/dts/apm/apm-storm.dtsi | 5 +
 2 files changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi 
b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
index c569f76..a5935f5 100644
--- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
@@ -472,6 +472,11 @@
mboxes = <&mailbox 0>;
};
 
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
+
serial0: serial@1060 {
device_type = "serial";
compatible = "ns16550";
diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi 
b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index 5147d76..f8ea5b5 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -716,6 +716,11 @@
mboxes = <&mailbox 0>;
};
 
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
+
serial0: serial@1c02 {
status = "disabled";
device_type = "serial";
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/3] Documentation: dtb: xgene: Add hwmon dts binding documentation

2016-07-21 Thread Hoan Tran
This patch adds the APM X-Gene hwmon device tree node documentation.

Signed-off-by: Hoan Tran 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  | 14 ++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt

diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt 
b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
new file mode 100644
index 000..59b3855
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
@@ -0,0 +1,14 @@
+APM X-Gene hwmon driver
+
+APM X-Gene SOC sensors are accessed over the "SLIMpro" mailbox.
+
+Required properties :
+ - compatible : should be "apm,xgene-slimpro-hwmon"
+ - mboxes : use the label reference for the mailbox as the first parameter.
+   The second parameter is the channel number.
+
+Example :
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/3] hwmon: xgene: Add hwmon driver

2016-07-21 Thread Hoan Tran
This patch adds hardware temperature and power reading support for
APM X-Gene SoC using the mailbox communication interface.

Signed-off-by: Hoan Tran 
---
 Documentation/hwmon/xgene-hwmon |  30 ++
 drivers/hwmon/Kconfig   |   7 +
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/xgene-hwmon.c | 755 
 4 files changed, 793 insertions(+)
 create mode 100644 Documentation/hwmon/xgene-hwmon
 create mode 100644 drivers/hwmon/xgene-hwmon.c

diff --git a/Documentation/hwmon/xgene-hwmon b/Documentation/hwmon/xgene-hwmon
new file mode 100644
index 000..6ec50ed
--- /dev/null
+++ b/Documentation/hwmon/xgene-hwmon
@@ -0,0 +1,30 @@
+Kernel driver xgene-hwmon
+
+
+Supported chips:
+ * APM X-Gene SoC
+
+Description
+---
+
+This driver adds hardware temperature and power reading support for
+APM X-Gene SoC using the mailbox communication interface.
+For device tree, it is the standard DT mailbox.
+For ACPI, it is the PCC mailbox.
+
+The following sensors are supported
+
+  * Temperature
+- SoC on-die temperature in milli-degree C
+- Alarm when high/over temperature occurs
+  * Power
+- CPU power in uW
+- IO power in uW
+
+sysfs-Interface
+---
+
+temp0_input - SoC on-die temperature (milli-degree C)
+temp0_critical_alarm - An 1 would indicates on-die temperature exceeded 
threshold
+power0_input - CPU power in (uW)
+power1_input - IO power in (uW)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ff94007..55218c6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1787,6 +1787,13 @@ config SENSORS_ULTRA45
  This driver provides support for the Ultra45 workstation environmental
  sensors.
 
+config SENSORS_XGENE
+   tristate "APM X-Gene SoC hardware monitoring driver"
+   depends on XGENE_SLIMPRO_MBOX || PCC
+   help
+ If you say yes here you get support for the temperature
+ and power sensors for APM X-Gene SoC.
+
 if ACPI
 
 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 2ef5b7c..a2460dd 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -162,6 +162,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)+= w83l786ng.o
 obj-$(CONFIG_SENSORS_WM831X)   += wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)   += wm8350-hwmon.o
+obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
 
 obj-$(CONFIG_PMBUS)+= pmbus/
 
diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
new file mode 100644
index 000..c263e7e
--- /dev/null
+++ b/drivers/hwmon/xgene-hwmon.c
@@ -0,0 +1,755 @@
+/*
+ * APM X-Gene SoC Hardware Monitoring Driver
+ *
+ * Copyright (c) 2016, Applied Micro Circuits Corporation
+ * Author: Loc Ho 
+ * Hoan Tran 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * This driver provides the following features:
+ *  - Retrieve CPU total power (uW)
+ *  - Retrieve IO total power (uW)
+ *  - Retrieve SoC temperature (milli-degree C) and alarm
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* SLIMpro message defines */
+#define MSG_TYPE_DBG   0
+#define MSG_TYPE_ERR   7
+#define MSG_TYPE_PWRMGMT   9
+
+#define MSG_TYPE(v)(((v) & 0xF000) >> 28)
+#define MSG_TYPE_SET(v)(((v) << 28) & 0xF000)
+#define MSG_SUBTYPE(v) (((v) & 0x0F00) >> 24)
+#define MSG_SUBTYPE_SET(v) (((v) << 24) & 0x0F00)
+
+#define DBG_SUBTYPE_SENSOR_READ4
+#define SENSOR_RD_MSG  0x04FFE902
+#define SENSOR_RD_EN_ADDR(a)   ((a) & 0x000F)
+#define PMD_PWR_REG0x20
+#define PMD_PWR_MW_REG 0x26
+#define SOC_PWR_REG0x21
+#define SOC_PWR_MW_REG 0x27
+#define SOC_TEMP_REG   0x10
+
+#define TEMP_NEGATIVE_BIT  8
+#define SENSOR_INVALID_DATABIT(15)
+
+#define PWRMGMT_SUBTYPE_TPC1
+#define TPC_ALARM  2
+#define TPC_GET_ALARM  3
+#define TPC_CMD(v)

[PATCH v3 0/3] hwmon: xgene: Add support for X-Gene hwmon driver

2016-07-21 Thread Hoan Tran
This patch set adds hardware temperature and power reading support for
APM X-Gene SoC using the mailbox communication interface.
For device tree, it is the standard DT mailbox.
For ACPI, it is the PCC mailbox.

For ACPI, this patch is built on top and depends on patch[1]:
[1] http://www.spinics.net/lists/linux-acpi/msg66041.html
 - [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2

v3
 - Order include files alphabetically
 - Use sign_extend32() to decode temperature
 - Use DEVICE_ATTR_RO() for temperature and power attributes
 - Temperature and power attributes start with index 1
 - Use !!amsg->param2
 - Fix checkpatch WARNING with --strict flag
 - Use hwmon_device_register_with_groups()
 - Check invalid sensor data

v2
 - Increase power reading accurateness by using 2 registers
(a register for Watt, another for milli-Watt)
 - Remove power reading for SoC
 - Fix review comments from Guenter

v1
 - Initial

Hoan Tran (3):
  Documentation: dtb: xgene: Add hwmon dts binding documentation
  hwmon: xgene: Add hwmon driver
  arm64: dts: apm: Add X-Gene SoC hwmon to device tree

 .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  |  14 +
 Documentation/hwmon/xgene-hwmon|  30 +
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi |   5 +
 arch/arm64/boot/dts/apm/apm-storm.dtsi |   5 +
 drivers/hwmon/Kconfig  |   7 +
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/xgene-hwmon.c| 755 +
 7 files changed, 817 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
 create mode 100644 Documentation/hwmon/xgene-hwmon
 create mode 100644 drivers/hwmon/xgene-hwmon.c

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/3] hwmon: xgene: Add hwmon driver

2016-07-18 Thread Hoan Tran
Hi Guenter,

On Sat, Jul 16, 2016 at 9:35 AM, Guenter Roeck  wrote:
> On 07/11/2016 05:30 PM, Hoan Tran wrote:
>>
>> This patch adds hardware temperature and power reading support for
>> APM X-Gene SoC using the mailbox communication interface.
>>
>> Signed-off-by: Hoan Tran 
>> ---
>>   Documentation/hwmon/xgene-hwmon |  30 ++
>>   drivers/hwmon/Kconfig   |   7 +
>>   drivers/hwmon/Makefile  |   1 +
>>   drivers/hwmon/xgene-hwmon.c | 772
>> 
>>   4 files changed, 810 insertions(+)
>>   create mode 100644 Documentation/hwmon/xgene-hwmon
>>   create mode 100644 drivers/hwmon/xgene-hwmon.c
>>
>> diff --git a/Documentation/hwmon/xgene-hwmon
>> b/Documentation/hwmon/xgene-hwmon
>> new file mode 100644
>> index 000..6ec50ed
>> --- /dev/null
>> +++ b/Documentation/hwmon/xgene-hwmon
>> @@ -0,0 +1,30 @@
>> +Kernel driver xgene-hwmon
>> +
>> +
>> +Supported chips:
>> + * APM X-Gene SoC
>> +
>> +Description
>> +---
>> +
>> +This driver adds hardware temperature and power reading support for
>> +APM X-Gene SoC using the mailbox communication interface.
>> +For device tree, it is the standard DT mailbox.
>> +For ACPI, it is the PCC mailbox.
>> +
>> +The following sensors are supported
>> +
>> +  * Temperature
>> +- SoC on-die temperature in milli-degree C
>> +- Alarm when high/over temperature occurs
>> +  * Power
>> +- CPU power in uW
>> +- IO power in uW
>> +
>> +sysfs-Interface
>> +---
>> +
>> +temp0_input - SoC on-die temperature (milli-degree C)
>> +temp0_critical_alarm - An 1 would indicates on-die temperature exceeded
>> threshold
>> +power0_input - CPU power in (uW)
>> +power1_input - IO power in (uW)
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index ff94007..55218c6 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -1787,6 +1787,13 @@ config SENSORS_ULTRA45
>>   This driver provides support for the Ultra45 workstation
>> environmental
>>   sensors.
>>
>> +config SENSORS_XGENE
>> +   tristate "APM X-Gene SoC hardware monitoring driver"
>> +   depends on XGENE_SLIMPRO_MBOX || PCC
>> +   help
>> + If you say yes here you get support for the temperature
>> + and power sensors for APM X-Gene SoC.
>> +
>>   if ACPI
>>
>>   comment "ACPI drivers"
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 2ef5b7c..a2460dd 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -162,6 +162,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
>>   obj-$(CONFIG_SENSORS_W83L786NG)   += w83l786ng.o
>>   obj-$(CONFIG_SENSORS_WM831X)  += wm831x-hwmon.o
>>   obj-$(CONFIG_SENSORS_WM8350)  += wm8350-hwmon.o
>> +obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
>>
>>   obj-$(CONFIG_PMBUS)   += pmbus/
>>
>> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
>> new file mode 100644
>> index 000..03917e3
>> --- /dev/null
>> +++ b/drivers/hwmon/xgene-hwmon.c
>> @@ -0,0 +1,772 @@
>> +/*
>> + * APM X-Gene SoC Hardware Monitoring Driver
>> + *
>> + * Copyright (c) 2016, Applied Micro Circuits Corporation
>> + * Author: Loc Ho 
>> + * Hoan Tran 
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + *
>> + * This driver provides the following features:
>> + *  - Retrieve CPU total power (uW)
>> + *  - Retrieve IO total power (uW)
>> + *  - Retrieve SoC temperature (milli-degree C) and alarm
>> + */
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>

[PATCH v2 1/3] Documentation: dtb: xgene: Add hwmon dts binding documentation

2016-07-11 Thread Hoan Tran
This patch adds the APM X-Gene hwmon device tree node documentation.

Signed-off-by: Hoan Tran 
---
 .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  | 14 ++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt

diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt 
b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
new file mode 100644
index 000..59b3855
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
@@ -0,0 +1,14 @@
+APM X-Gene hwmon driver
+
+APM X-Gene SOC sensors are accessed over the "SLIMpro" mailbox.
+
+Required properties :
+ - compatible : should be "apm,xgene-slimpro-hwmon"
+ - mboxes : use the label reference for the mailbox as the first parameter.
+   The second parameter is the channel number.
+
+Example :
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/3] arm64: dts: apm: Add X-Gene SoC hwmon to device tree

2016-07-11 Thread Hoan Tran
This patch adds DT node to enable hwmon driver for APM X-Gene SoC.

Signed-off-by: Hoan Tran 
---
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 5 +
 arch/arm64/boot/dts/apm/apm-storm.dtsi | 5 +
 2 files changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi 
b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
index c569f76..a5935f5 100644
--- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
@@ -472,6 +472,11 @@
mboxes = <&mailbox 0>;
};
 
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
+
serial0: serial@1060 {
device_type = "serial";
compatible = "ns16550";
diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi 
b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index 5147d76..f8ea5b5 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -716,6 +716,11 @@
mboxes = <&mailbox 0>;
};
 
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
+
serial0: serial@1c02 {
status = "disabled";
device_type = "serial";
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/3] hwmon: xgene: Add hwmon driver

2016-07-11 Thread Hoan Tran
This patch adds hardware temperature and power reading support for
APM X-Gene SoC using the mailbox communication interface.

Signed-off-by: Hoan Tran 
---
 Documentation/hwmon/xgene-hwmon |  30 ++
 drivers/hwmon/Kconfig   |   7 +
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/xgene-hwmon.c | 772 
 4 files changed, 810 insertions(+)
 create mode 100644 Documentation/hwmon/xgene-hwmon
 create mode 100644 drivers/hwmon/xgene-hwmon.c

diff --git a/Documentation/hwmon/xgene-hwmon b/Documentation/hwmon/xgene-hwmon
new file mode 100644
index 000..6ec50ed
--- /dev/null
+++ b/Documentation/hwmon/xgene-hwmon
@@ -0,0 +1,30 @@
+Kernel driver xgene-hwmon
+
+
+Supported chips:
+ * APM X-Gene SoC
+
+Description
+---
+
+This driver adds hardware temperature and power reading support for
+APM X-Gene SoC using the mailbox communication interface.
+For device tree, it is the standard DT mailbox.
+For ACPI, it is the PCC mailbox.
+
+The following sensors are supported
+
+  * Temperature
+- SoC on-die temperature in milli-degree C
+- Alarm when high/over temperature occurs
+  * Power
+- CPU power in uW
+- IO power in uW
+
+sysfs-Interface
+---
+
+temp0_input - SoC on-die temperature (milli-degree C)
+temp0_critical_alarm - An 1 would indicates on-die temperature exceeded 
threshold
+power0_input - CPU power in (uW)
+power1_input - IO power in (uW)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ff94007..55218c6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1787,6 +1787,13 @@ config SENSORS_ULTRA45
  This driver provides support for the Ultra45 workstation environmental
  sensors.
 
+config SENSORS_XGENE
+   tristate "APM X-Gene SoC hardware monitoring driver"
+   depends on XGENE_SLIMPRO_MBOX || PCC
+   help
+ If you say yes here you get support for the temperature
+ and power sensors for APM X-Gene SoC.
+
 if ACPI
 
 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 2ef5b7c..a2460dd 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -162,6 +162,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)+= w83l786ng.o
 obj-$(CONFIG_SENSORS_WM831X)   += wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)   += wm8350-hwmon.o
+obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
 
 obj-$(CONFIG_PMBUS)+= pmbus/
 
diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
new file mode 100644
index 000..03917e3
--- /dev/null
+++ b/drivers/hwmon/xgene-hwmon.c
@@ -0,0 +1,772 @@
+/*
+ * APM X-Gene SoC Hardware Monitoring Driver
+ *
+ * Copyright (c) 2016, Applied Micro Circuits Corporation
+ * Author: Loc Ho 
+ * Hoan Tran 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * This driver provides the following features:
+ *  - Retrieve CPU total power (uW)
+ *  - Retrieve IO total power (uW)
+ *  - Retrieve SoC temperature (milli-degree C) and alarm
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* SLIMpro message defines */
+#define MSG_TYPE_DBG   0
+#define MSG_TYPE_ERR   7
+#define MSG_TYPE_PWRMGMT   9
+
+#define MSG_TYPE(v)(((v) & 0xF000) >> 28)
+#define MSG_TYPE_SET(v)(((v) << 28) & 0xF000)
+#define MSG_SUBTYPE(v) (((v) & 0x0F00) >> 24)
+#define MSG_SUBTYPE_SET(v) (((v) << 24) & 0x0F00)
+
+#define DBG_SUBTYPE_SENSOR_READ4
+#define SENSOR_RD_MSG  0x04FFE902
+#define SENSOR_RD_EN_ADDR(a)   ((a) & 0x000F)
+#define PMD_PWR_REG0x20
+#define PMD_PWR_MW_REG 0x26
+#define SOC_PWR_REG0x21
+#define SOC_PWR_MW_REG 0x27
+#define SOC_TEMP_REG   0x10
+
+#define TEMP_NEGATIVE_BIT  8
+
+#define PWRMGMT_SUBTYPE_TPC1
+#define TPC_ALARM  2
+#define TPC_GET_ALARM  3
+#define TPC_CMD(v) (((v) & 0x00FF

[PATCH v2 0/3] hwmon: xgene: Add support for X-Gene hwmon driver

2016-07-11 Thread Hoan Tran
This patch set adds hardware temperature and power reading support ​for
APM X-Gene SoC using the mailbox communication interface.
For device tree, it is the standard DT mailbox.
For ACPI, it is the PCC mailbox.

For ACPI, tested with this patch[1] which supports PCC subspace 2
[1] http://www.spinics.net/lists/linux-acpi/msg66041.html
 - [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2

v2
 - Increase power reading accurateness by using 2 registers
(a register for Watt, another for milli-Watt)
 - Remove power reading for SoC
 - Fix review comments from Guenter

v1
 - Initial

Hoan Tran (3):
  Documentation: dtb: xgene: Add hwmon dts binding documentation
  hwmon: xgene: Add hwmon driver
  arm64: dts: apm: Add X-Gene SoC hwmon to device tree

 .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  |  14 +
 Documentation/hwmon/xgene-hwmon|  30 +
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi |   5 +
 arch/arm64/boot/dts/apm/apm-storm.dtsi |   5 +
 drivers/hwmon/Kconfig  |   7 +
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/xgene-hwmon.c| 772 +
 7 files changed, 834 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
 create mode 100644 Documentation/hwmon/xgene-hwmon
 create mode 100644 drivers/hwmon/xgene-hwmon.c

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Documentation: dtb: xgene: Add hwmon dts binding documentation

2016-06-23 Thread Hoan Tran
On Tue, Jun 7, 2016 at 11:05 AM, Hoan Tran  wrote:
> Hi Jassi,
>
> Thanks for your reply !
>
> On Tue, Jun 7, 2016 at 10:20 AM, Jassi Brar  wrote:
>> On Tue, May 24, 2016 at 6:31 AM, Hoan Tran  wrote:
>>> Hi Rob,
>>>
>>> Thanks for your review !
>>>
>>> On Mon, May 23, 2016 at 1:30 PM, Rob Herring  wrote:
>>>>
>>>> On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote:
>>>> > This patch adds the APM X-Gene hwmon device tree node documentation.
>>>> >
>>>> > Signed-off-by: Hoan Tran 
>>>> > ---
>>>> >  .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  | 14 
>>>> > ++
>>>> >  1 file changed, 14 insertions(+)
>>>> >  create mode 100644 
>>>> > Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
>>>> >
>>>> > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt 
>>>> > b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
>>>> > new file mode 100644
>>>> > index 000..49a482e
>>>> > --- /dev/null
>>>> > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
>>>> > @@ -0,0 +1,14 @@
>>>> > +APM X-Gene hwmon driver
>>>> > +
>>>> > +Hwmon driver accesses sensors over the "SLIMpro" mailbox.
>>>>
>>>> DT bindings describe h/w, not driver data.
>>> How about this description: "APM X-Gene SOC sensors are accessed over
>>> the "SLIMpro" mailbox" ?
>>>> I'm not sure this belongs in
>>>> DT and perhaps the devices for the mailbox should be created by the
>>>> mailbox driver.
>>> I don't think the current mailbox supports it.
>>>>
>>>> > +
>>>> > +Required properties :
>>>> > + - compatible : should be "apm,xgene-slimpro-hwmon"
>>>> > + - mboxes : use the label reference for the mailbox as the first 
>>>> > parameter.
>>>> > + The second parameter is the channel number.
>>>>
>>>> When do you expect this to be different mailbox numbers?
>>> No, this number is not changed. This "mboxes" property is used and
>>> required by mailbox.c when hwmon driver requests a mailbox channel
>>>
>> I think that's inaccurate.
>>
>> The h/w and the firmware combined is the "platform" from Linux POV.
>> Channels are physical resources provided by a mailbox controller.
>> Currently the firmware listens on Channel-7 but some future revision
>> might switch to, say, Channel-9.  Or say the same firmware on next
>> revision of h/w may have to switch to Channel-3 because it has only 4
>> channels. So I see the mailbox channel number as a hardware property
>> just like an IRQ (which very often change with SoC iterations).
>
> Agree about that. I suppose this number is not changed. But as you
> said, the mailbox channel number can be changed based on SoC or
> Firmware. It would be better if this channel number is specified
> inside a DT node.
>
> Hi Rob, do you have any comments ?
>
> Thanks
> Hoan
>
>>
>> Cheers.

Hi Rob,

Do you have any comments on Jassi's reply ?
If not, I'll send another version which included the binding document
and DT node.

Thanks
Hoan
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Documentation: dtb: xgene: Add hwmon dts binding documentation

2016-06-07 Thread Hoan Tran
Hi Jassi,

Thanks for your reply !

On Tue, Jun 7, 2016 at 10:20 AM, Jassi Brar  wrote:
> On Tue, May 24, 2016 at 6:31 AM, Hoan Tran  wrote:
>> Hi Rob,
>>
>> Thanks for your review !
>>
>> On Mon, May 23, 2016 at 1:30 PM, Rob Herring  wrote:
>>>
>>> On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote:
>>> > This patch adds the APM X-Gene hwmon device tree node documentation.
>>> >
>>> > Signed-off-by: Hoan Tran 
>>> > ---
>>> >  .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  | 14 
>>> > ++
>>> >  1 file changed, 14 insertions(+)
>>> >  create mode 100644 
>>> > Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt 
>>> > b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
>>> > new file mode 100644
>>> > index 000..49a482e
>>> > --- /dev/null
>>> > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
>>> > @@ -0,0 +1,14 @@
>>> > +APM X-Gene hwmon driver
>>> > +
>>> > +Hwmon driver accesses sensors over the "SLIMpro" mailbox.
>>>
>>> DT bindings describe h/w, not driver data.
>> How about this description: "APM X-Gene SOC sensors are accessed over
>> the "SLIMpro" mailbox" ?
>>> I'm not sure this belongs in
>>> DT and perhaps the devices for the mailbox should be created by the
>>> mailbox driver.
>> I don't think the current mailbox supports it.
>>>
>>> > +
>>> > +Required properties :
>>> > + - compatible : should be "apm,xgene-slimpro-hwmon"
>>> > + - mboxes : use the label reference for the mailbox as the first 
>>> > parameter.
>>> > + The second parameter is the channel number.
>>>
>>> When do you expect this to be different mailbox numbers?
>> No, this number is not changed. This "mboxes" property is used and
>> required by mailbox.c when hwmon driver requests a mailbox channel
>>
> I think that's inaccurate.
>
> The h/w and the firmware combined is the "platform" from Linux POV.
> Channels are physical resources provided by a mailbox controller.
> Currently the firmware listens on Channel-7 but some future revision
> might switch to, say, Channel-9.  Or say the same firmware on next
> revision of h/w may have to switch to Channel-3 because it has only 4
> channels. So I see the mailbox channel number as a hardware property
> just like an IRQ (which very often change with SoC iterations).

Agree about that. I suppose this number is not changed. But as you
said, the mailbox channel number can be changed based on SoC or
Firmware. It would be better if this channel number is specified
inside a DT node.

Hi Rob, do you have any comments ?

Thanks
Hoan

>
> Cheers.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Documentation: dtb: xgene: Add hwmon dts binding documentation

2016-06-07 Thread Hoan Tran
Hi Rob,

On Wed, May 25, 2016 at 10:09 AM, Rob Herring  wrote:
>
> On Mon, May 23, 2016 at 06:01:14PM -0700, Hoan Tran wrote:
> > Hi Rob,
> >
> > Thanks for your review !
> >
> > On Mon, May 23, 2016 at 1:30 PM, Rob Herring  wrote:
> > >
> > > On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote:
> > > > This patch adds the APM X-Gene hwmon device tree node documentation.
> > > >
> > > > Signed-off-by: Hoan Tran 
> > > > ---
> > > >  .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  | 14 
> > > > ++
> > > >  1 file changed, 14 insertions(+)
> > > >  create mode 100644 
> > > > Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
> > > >
> > > > diff --git 
> > > > a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt 
> > > > b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
> > > > new file mode 100644
> > > > index 000..49a482e
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
> > > > @@ -0,0 +1,14 @@
> > > > +APM X-Gene hwmon driver
> > > > +
> > > > +Hwmon driver accesses sensors over the "SLIMpro" mailbox.
> > >
> > > DT bindings describe h/w, not driver data.
> > How about this description: "APM X-Gene SOC sensors are accessed over
> > the "SLIMpro" mailbox" ?
> > > I'm not sure this belongs in
> > > DT and perhaps the devices for the mailbox should be created by the
> > > mailbox driver.
> > I don't think the current mailbox supports it.
>
> Then add it. The mailbox binding is really only needed when other h/w
> blocks use a mailbox. As this is purely a firmware interface then the
> mailbox driver can create any devices it needs. The devices don't have
> to be in DT to be created.

Yes, I'll create a function inside mailbox. It allows client request a
channel by mailbox name and index without DT node.

Thanks
Hoan


>
>
> Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [2/3] hwmon: xgene: Adds hwmon driver

2016-05-31 Thread Hoan Tran
Hi Guenter,

Thanks for your review !

On Sun, May 29, 2016 at 10:25 PM, Guenter Roeck  wrote:
> On Mon, May 16, 2016 at 09:17:26AM -0700, hotran wrote:
>> This patch adds hardware temperature and power reading support for
>> APM X-Gene SoC's using the mailbox communication interface.
>>
> Please drop the "'".

Yes, will remove it.

>
>>
>> Signed-off-by: Hoan Tran 
>> ---
>>  Documentation/hwmon/xgene-hwmon |  32 ++
>>  drivers/hwmon/Kconfig   |   7 +
>>  drivers/hwmon/Makefile  |   1 +
>>  drivers/hwmon/xgene-hwmon.c | 801 
>> 
>>  4 files changed, 841 insertions(+)
>>  create mode 100644 Documentation/hwmon/xgene-hwmon
>>  create mode 100644 drivers/hwmon/xgene-hwmon.c
>>
>> diff --git a/Documentation/hwmon/xgene-hwmon 
>> b/Documentation/hwmon/xgene-hwmon
>> new file mode 100644
>> index 000..040a7f2
>> --- /dev/null
>> +++ b/Documentation/hwmon/xgene-hwmon
>> @@ -0,0 +1,32 @@
>> +Kernel driver xgene-hwmon
>> +
>> +
>> +Supported chips:
>> + * APM X-Gene SoC
>> +
>> +Description
>> +---
>> +
>> +This driver adds hardware temperature and power reading support for
>> +APM X-Gene SoC's using the mailbox communication interface.
>>
> No "'".

Yes,

>>
>> +For device tree, it is the standard DT mailbox.
>> +For ACPI, it is the PCC mailbox.
>> +
>> +The following sensors are supported
>> +
>> +  * Temperature
>> +- SoC on-die temperature in milli-degree
>> +- Alarm when high/over temperature occurs
>> +  * Power
>> +- CPU power in uW
>> +- IO power in uW
>> +- CPU and IO power in uW
>> +
>> +sysfs-Interface
>> +---
>> +
>> +temp1_input - SoC on-die temperature (milli-degree C)
>> +temp1_critical_alarm - An 1 would indicates on-die temperature exceeded 
>> threshold
>> +power1_input - CPU power in (uW)
>> +power2_input - IO power in (uW)
>> +power3_input - CPU and IO power in (uW)
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 5c2d13a..91c3056 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -1776,6 +1776,13 @@ config SENSORS_ULTRA45
>> This driver provides support for the Ultra45 workstation 
>> environmental
>> sensors.
>>
>> +config SENSORS_XGENE
>> + tristate "APM X-Gene SoC hardware monitoring driver"
>> + depends on XGENE_SLIMPRO_MBOX || PCC
>> + help
>> +   If you say yes here you get support for the temperature
>> +   and power sensors for APM X-Gene SoC.
>> +
>>  if ACPI
>>
>>  comment "ACPI drivers"
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 58cc3ac..668d0f1 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -161,6 +161,7 @@ obj-$(CONFIG_SENSORS_W83L785TS)   += w83l785ts.o
>>  obj-$(CONFIG_SENSORS_W83L786NG)  += w83l786ng.o
>>  obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
>>  obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
>> +obj-$(CONFIG_SENSORS_XGENE)  += xgene-hwmon.o
>>
>>  obj-$(CONFIG_PMBUS)  += pmbus/
>>
>> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
>> new file mode 100644
>> index 000..bc4ad29
>> --- /dev/null
>> +++ b/drivers/hwmon/xgene-hwmon.c
>> @@ -0,0 +1,801 @@
>> +/*
>> + * APM X-Gene SoC Hardware Monitoring Driver
>> + *
>> + * Copyright (c) 2016, Applied Micro Circuits Corporation
>> + * Author: Loc Ho 
>> + * Hoan Tran 
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + *
>> + * This driver provides the following features:
>> + *  - Retrieve CPU'

Re: [PATCH 1/3] Documentation: dtb: xgene: Add hwmon dts binding documentation

2016-05-23 Thread Hoan Tran
Hi Rob,

Thanks for your review !

On Mon, May 23, 2016 at 1:30 PM, Rob Herring  wrote:
>
> On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote:
> > This patch adds the APM X-Gene hwmon device tree node documentation.
> >
> > Signed-off-by: Hoan Tran 
> > ---
> >  .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  | 14 
> > ++
> >  1 file changed, 14 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt 
> > b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
> > new file mode 100644
> > index 000..49a482e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
> > @@ -0,0 +1,14 @@
> > +APM X-Gene hwmon driver
> > +
> > +Hwmon driver accesses sensors over the "SLIMpro" mailbox.
>
> DT bindings describe h/w, not driver data.
How about this description: "APM X-Gene SOC sensors are accessed over
the "SLIMpro" mailbox" ?
> I'm not sure this belongs in
> DT and perhaps the devices for the mailbox should be created by the
> mailbox driver.
I don't think the current mailbox supports it.
>
> > +
> > +Required properties :
> > + - compatible : should be "apm,xgene-slimpro-hwmon"
> > + - mboxes : use the label reference for the mailbox as the first parameter.
> > + The second parameter is the channel number.
>
> When do you expect this to be different mailbox numbers?
No, this number is not changed. This "mboxes" property is used and
required by mailbox.c when hwmon driver requests a mailbox channel

Thanks and Regards
Hoan
>
>
> > +
> > +Example :
> > + hwmonslimpro {
> > + compatible = "apm,xgene-slimpro-hwmon";
> > + mboxes = <&mailbox 7>;
> > + };
> > --
> > 1.9.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] Documentation: dtb: xgene: Add hwmon dts binding documentation

2016-05-16 Thread Hoan Tran
This patch adds the APM X-Gene hwmon device tree node documentation.

Signed-off-by: Hoan Tran 
---
 .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  | 14 ++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt

diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt 
b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
new file mode 100644
index 000..49a482e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
@@ -0,0 +1,14 @@
+APM X-Gene hwmon driver
+
+Hwmon driver accesses sensors over the "SLIMpro" mailbox.
+
+Required properties :
+ - compatible : should be "apm,xgene-slimpro-hwmon"
+ - mboxes : use the label reference for the mailbox as the first parameter.
+   The second parameter is the channel number.
+
+Example :
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] arm64: dts: apm: Add X-Gene SoC hwmon to device tree

2016-05-16 Thread Hoan Tran
This patch adds DT node to enable hwmon driver for APM X-Gene SoC.

Signed-off-by: Hoan Tran 
---
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 5 +
 arch/arm64/boot/dts/apm/apm-storm.dtsi | 5 +
 2 files changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi 
b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
index a055a5d..672c0ce 100644
--- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
@@ -472,6 +472,11 @@
mboxes = <&mailbox 0>;
};
 
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
+
serial0: serial@1060 {
device_type = "serial";
compatible = "ns16550";
diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi 
b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index ae4a173..7a59159 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -716,6 +716,11 @@
mboxes = <&mailbox 0>;
};
 
+   hwmonslimpro {
+   compatible = "apm,xgene-slimpro-hwmon";
+   mboxes = <&mailbox 7>;
+   };
+
serial0: serial@1c02 {
status = "disabled";
device_type = "serial";
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/3] Add support for X-Gene hwmon driver

2016-05-16 Thread Hoan Tran
This patch set adds hardware temperature and power reading support ​for
APM X-Gene SoC's using the mailbox communication interface.
For device tree, it is the standard DT mailbox.
For ACPI, it is the PCC mailbox.

For ACPI, tested with this patch[1] which supports PCC subspace 2
[1] https://lkml.org/lkml/2016/5/6/482
 - [PATCH v2] mailbox: pcc: Support HW-Reduced Communication Subspace Type 2

Hoan Tran (3):
  Documentation: dtb: xgene: Add hwmon dts binding documentation
  hwmon: xgene: Adds hwmon driver
  arm64: dts: apm: Add X-Gene SoC hwmon to device tree

 .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt  |  14 +
 Documentation/hwmon/xgene-hwmon|  32 +
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi |   5 +
 arch/arm64/boot/dts/apm/apm-storm.dtsi |   5 +
 drivers/hwmon/Kconfig  |   7 +
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/xgene-hwmon.c| 801 +
 7 files changed, 865 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt
 create mode 100644 Documentation/hwmon/xgene-hwmon
 create mode 100644 drivers/hwmon/xgene-hwmon.c

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/3] hwmon: xgene: Adds hwmon driver

2016-05-16 Thread Hoan Tran
This patch adds hardware temperature and power reading support for
APM X-Gene SoC's using the mailbox communication interface.

Signed-off-by: Hoan Tran 
---
 Documentation/hwmon/xgene-hwmon |  32 ++
 drivers/hwmon/Kconfig   |   7 +
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/xgene-hwmon.c | 801 
 4 files changed, 841 insertions(+)
 create mode 100644 Documentation/hwmon/xgene-hwmon
 create mode 100644 drivers/hwmon/xgene-hwmon.c

diff --git a/Documentation/hwmon/xgene-hwmon b/Documentation/hwmon/xgene-hwmon
new file mode 100644
index 000..040a7f2
--- /dev/null
+++ b/Documentation/hwmon/xgene-hwmon
@@ -0,0 +1,32 @@
+Kernel driver xgene-hwmon
+
+
+Supported chips:
+ * APM X-Gene SoC
+
+Description
+---
+
+This driver adds hardware temperature and power reading support for
+APM X-Gene SoC's using the mailbox communication interface.
+For device tree, it is the standard DT mailbox.
+For ACPI, it is the PCC mailbox.
+
+The following sensors are supported
+
+  * Temperature
+- SoC on-die temperature in milli-degree
+- Alarm when high/over temperature occurs
+  * Power
+- CPU power in uW
+- IO power in uW
+- CPU and IO power in uW
+
+sysfs-Interface
+---
+
+temp1_input - SoC on-die temperature (milli-degree C)
+temp1_critical_alarm - An 1 would indicates on-die temperature exceeded 
threshold
+power1_input - CPU power in (uW)
+power2_input - IO power in (uW)
+power3_input - CPU and IO power in (uW)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 5c2d13a..91c3056 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1776,6 +1776,13 @@ config SENSORS_ULTRA45
  This driver provides support for the Ultra45 workstation environmental
  sensors.
 
+config SENSORS_XGENE
+   tristate "APM X-Gene SoC hardware monitoring driver"
+   depends on XGENE_SLIMPRO_MBOX || PCC
+   help
+ If you say yes here you get support for the temperature
+ and power sensors for APM X-Gene SoC.
+
 if ACPI
 
 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 58cc3ac..668d0f1 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -161,6 +161,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)+= w83l786ng.o
 obj-$(CONFIG_SENSORS_WM831X)   += wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)   += wm8350-hwmon.o
+obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
 
 obj-$(CONFIG_PMBUS)+= pmbus/
 
diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
new file mode 100644
index 000..bc4ad29
--- /dev/null
+++ b/drivers/hwmon/xgene-hwmon.c
@@ -0,0 +1,801 @@
+/*
+ * APM X-Gene SoC Hardware Monitoring Driver
+ *
+ * Copyright (c) 2016, Applied Micro Circuits Corporation
+ * Author: Loc Ho 
+ * Hoan Tran 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * This driver provides the following features:
+ *  - Retrieve CPU's total power (uW)
+ *  - Retrieve IO's total power (uW)
+ *  - Retrieve SoC total power (uW)
+ *  - Retrieve SoC temperature (milli-degree C) and alarm
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* SLIMpro message defines */
+#define SLIMPRO_MSG_TYPE_DBG_ID0
+#define SLIMPRO_MSG_TYPE_ERR_ID7
+#define SLIMPRO_MSG_TYPE_PWRMGMT_ID9
+
+#define SLIMPRO_MSG_TYPE(v)(((v) & 0xF000) >> 28)
+#define SLIMPRO_MSG_TYPE_SET(v)(((v) << 28) & 0xF000)
+#define SLIMPRO_MSG_SUBTYPE(v) (((v) & 0x0F00) >> 24)
+#define SLIMPRO_MSG_SUBTYPE_SET(v) (((v) << 24) & 0x0F00)
+
+#define SLIMPRO_DBG_SUBTYPE_SENSOR_READ4
+#define SLIMPRO_SENSOR_READ_MSG0x04FFE902
+#define SLIMPRO_SENSOR_READ_ENCODE_ADDR(a) \
+   ((a) & 0x000F)
+#define PMD_PWR_MW_REG 0x26
+#define SOC_PWR_REG0x21
+#define SOC_TEMP_REG   0x10
+
+#define SLIMPRO_PWRMGMT_SUBTYPE_TPC1
+#define SLIMPRO_TPC_ALARM  2
+#define SLIMPRO_TPC_GET_ALARM  3
+#define SLIMPRO_TPC_CMD(v) (((v) &