Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1513922 > unrolled thread

[PATCH v1 00/11] perf: arm64: Support for Hisilicon SoC Hardware event counters

Back to article view | Back to linux.kernel

Contents

  [PATCH v1 00/11] perf: arm64: Support for Hisilicon SoC Hardware event counters Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
    [PATCH v1 09/11] perf: hisi: Miscellanous node(MN) event counting in perf Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
    [PATCH v1 02/11] dt-bindings: hisi: Add Hisilicon HiP05/06/07 Sysctrl and Djtag dts bindings Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
    [PATCH v1 06/11] perf: hisi: Update Kconfig for Hisilicon PMU support Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
    [PATCH v1 10/11] perf: hisi: Support for Hisilicon DDRC PMU. Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
    [PATCH v1 08/11] perf: hisi: Add sysfs attributes for L3 cache(L3C) PMU Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
    [PATCH v1 04/11] Documentation: perf: hisi: Documentation for HIP05/06/07 PMU event counting. Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
    [PATCH v1 01/11] arm64: MAINTAINERS: hisi: Add hisilicon SoC PMU support Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
    [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
      Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver Arnd Bergmann <arnd@arndb.de> - 2016-11-07 14:30 +0100
        Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver John Garry <john.garry@huawei.com> - 2016-11-07 15:20 +0100
          Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver Arnd Bergmann <arnd@arndb.de> - 2016-11-07 21:10 +0100
            Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver John Garry <john.garry@huawei.com> - 2016-11-08 12:40 +0100
              Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver Arnd Bergmann <arnd@arndb.de> - 2016-11-08 12:50 +0100
                Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver John Garry <john.garry@huawei.com> - 2016-11-08 15:00 +0100
                  Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver Arnd Bergmann <arnd@arndb.de> - 2016-11-08 16:20 +0100
                    Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver John Garry <john.garry@huawei.com> - 2016-11-08 16:20 +0100
                    Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver Anurup M <anurupvasu@gmail.com> - 2016-11-09 12:00 +0100
                Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver Anurup M <anurupvasu@gmail.com> - 2016-11-08 17:00 +0100
                  Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver John Garry <john.garry@huawei.com> - 2016-11-09 10:10 +0100
                    Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver Anurup M <anurupvasu@gmail.com> - 2016-11-11 11:40 +0100
        Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver Tan Xiaojun <tanxiaojun@huawei.com> - 2016-11-08 08:10 +0100
          Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver Anurup M <anurupvasu@gmail.com> - 2016-11-08 08:40 +0100
            Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver Arnd Bergmann <arnd@arndb.de> - 2016-11-08 12:50 +0100
              Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver Anurup M <anurupvasu@gmail.com> - 2016-11-08 14:50 +0100
                Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver Arnd Bergmann <arnd@arndb.de> - 2016-11-08 16:20 +0100
                  Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver Anurup M <anurupvasu@gmail.com> - 2016-11-09 05:30 +0100
                    Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver Arnd Bergmann <arnd@arndb.de> - 2016-11-09 22:50 +0100
                      Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon  Djtag driver Anurup M <anurupvasu@gmail.com> - 2016-11-11 11:30 +0100
    [PATCH v1 07/11] perf: hisi: Add support for Hisilicon SoC event counters Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100
    [PATCH v1 05/11] dt-bindings: perf: hisi: Add Devicetree bindings for Hisilicon SoC PMU Anurup M <anurupvasu@gmail.com> - 2016-11-02 16:50 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1519680 — Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver

On Wednesday 09 November 2016 02:36 PM, John Garry wrote:
>
>>>>> I'd suggest requiring #address-cells=<1> and #size-cells=<0> in the
>>>>> master
>>>>> node, and listing the children by reg property. If the address is not
>>>>> easily expressed as a single integer, use a larger #address-cells
>>>>> value.
>>>> We already have something equivalent to reg in "module-id" (see patch
>>>> 02/11), which is the slave device bus address; here's a sample:
>>>> +        /* For L3 cache PMU */
>>>> +        pmul3c0 {
>>>> +            compatible = "hisilicon,hisi-pmu-l3c-v1";
>>>> +            scl-id = <0x02>;
>>>> +            num-events = <0x16>;
>>>> +            num-counters = <0x08>;
>>>> +            module-id = <0x04>;
>>>> +            num-banks = <0x04>;
>>>> +            cfgen-map = <0x02 0x04 0x01 0x08>;
>>>> +            counter-reg = <0x170>;
>>>> +            evctrl-reg = <0x04>;
>>>> +            event-en = <0x1000000>;
>>>> +            evtype-reg = <0x140>;
>>>> +        };
>>>>
>>>> FYI, "module-id" is our own internal hw nomenclature.
>>> Yes, that was my interpretation as well. Please use the standard
>>> "reg" property for this then.
>> Hi Arnd,
>>
>> Firstly my apologies for a mistake in the bindings example in ([PATCH
>> 02/11 ..]).
>> The module-id property is a list as defined in the PMU bindings patch
>> ([PATCH v1 05/11] dt-bindings .. <https://lkml.org/lkml/2016/11/2/323>).
>>
>> +    djtag0: djtag@0 {
>> +        compatible = "hisilicon,hip05-cpu-djtag-v1";
>> +            pmul3c0 {
>> +                compatible = "hisilicon,hisi-pmu-l3c-v1";
>> +                scl-id = <0x02>;
>> +                num-events = <0x16>;
>> +                num-counters = <0x08>;
>> +                module-id = <0x04 0x04 0x04 0x04>;
>> +                num-banks = <0x04>;
>> +                cfgen-map = <0x02 0x04 0x01 0x08>;
>> +                counter-reg = <0x170>;
>> +                evctrl-reg = <0x04>;
>> +                event-en = <0x1000000>;
>> +                evtype-reg = <0x140>;
>> +            };
>>
>>
>> The L3 cache in hip05/06/07 chips consist of 4 banks (each bank has PMU
>> registers).
>>
>> In hip05/06 all L3 cache banks are identified with same module-id.
>> module-id = <0x04 0x04 0x04 0x04>;
>>
>> But in the case hip07 chip(djtag v2), each L3 cache bank has different
>> module-id
>> module-id = <0x01 0x02 0x03 0x04>;
>>
>> So in this case Please share your opinion on how to model it.
>>
>
> My suggestion is to have a single PMU per module, whether that is 4 
> banks or 1 bank per module, as this makes the driver simpler.
>
> I think you mentioned that a separate PMU per bank does not make much 
> sense, and you would rather treat all banks as a single bank and 
> aggregrate their perf statstics under a single PMU: Can you just use a 
> script in userspace which can do this aggregration work if you have 
> separate PMUs?
Hi John,

Mark also suggest the same view.
I have some concerns or doubts in having separate PMU for each L3 cache 
bank.
We can discuss it in the same thread [[RESEND PATCH v1 07/11]] 
<http://www.spinics.net/lists/arm-kernel/msg541938.html>].

Thanks,
Anurup
>
> Maybe perf guys have a view on this also.
>
> John
>
>> Some more detail of L3 cache PMU.
>> ------------------------------------------------
>> The hip05/06/07 chips consists of a multiple Super CPU cluster (16 CPU
>> cores). we call it SCCL.
>> The L3 cache( 4 banks) is shared by all CPU cores in a SCCL.
>> Each L3 cache bank has PMU registers. We always take the sum of the
>> counters to show in perf.
>> Taking individual L3 cache count is not meaningful as there is no
>> mapping of CPU cores to individual
>> L3 cache banks.
>>
>> Please share your suggestion.
>>
>> Thanks,
>> Anurup
>

[toc] | [prev] | [next] | [standalone]

#1516891 — Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver

On 2016/11/7 21:26, Arnd Bergmann wrote:
> On Wednesday, November 2, 2016 11:42:46 AM CET Anurup M wrote:
>> From: Tan Xiaojun <tanxiaojun@huawei.com>
>>
>> 	The Hisilicon Djtag is an independent component which connects
>> 	with some other components in the SoC by Debug Bus. This driver
>> 	can be configured to access the registers of connecting components
>> 	(like L3 cache) during real time debugging.
> 
> The formatting of the text seems odd, please remove the leading spaces.
> 

Sorry for that. We will fix it.

>>  drivers/soc/Kconfig                 |   1 +
>>  drivers/soc/Makefile                |   1 +
>>  drivers/soc/hisilicon/Kconfig       |  12 +
>>  drivers/soc/hisilicon/Makefile      |   1 +
>>  drivers/soc/hisilicon/djtag.c       | 639 ++++++++++++++++++++++++++++++++++++
>>  include/linux/soc/hisilicon/djtag.h |  38 +++
> 
> Do you expect other drivers to be added that reference this interface?
> If not, or if you are unsure, just put all of it under drivers/perf
> so we don't introduce a global API that has only one user.
> 

OK. For now, this suggestion sounds good.

>> +
>> +#include <linux/bitops.h>
>> +#include <linux/init.h>
>> +#include <linux/list.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/spinlock.h>
>> +
>> +#include <asm-generic/delay.h>
> 
> Never include files from asm-generic directly except from
> an architecture specific asm/*.h header file.
> 
> 

OK. Sorry for that.

>> +DEFINE_IDR(djtag_hosts_idr);
> 
> make this static
> 

OK.

>> +static void djtag_read32_relaxed(void __iomem *regs_base, u32 off, u32 *value)
>> +{
>> +	void __iomem *reg_addr = regs_base + off;
>> +
>> +	*value = readl_relaxed(reg_addr);
>> +}
>> +
>> +static void djtag_write32(void __iomem *regs_base, u32 off, u32 val)
>> +{
>> +	void __iomem *reg_addr = regs_base + off;
>> +
>> +	writel(val, reg_addr);
>> +}
> 
> This looks like an odd combination of interfaces.
> Why can the reads be "relaxed" when the writes can not?
> 
> Generally speaking, I'd advise to always use non-relaxed accessors
> unless there is a strong performance reason, and in that case there
> should be a comment explaining the use at each of the callers
> of a relaxed accessor.
> 

Yes, it is our mistake.

>> +	/* ensure the djtag operation is done */
>> +	do {
>> +		djtag_read32_relaxed(regs_base, SC_DJTAG_MSTR_START_EN_EX, &rd);
>> +
>> +		if (!(rd & DJTAG_MSTR_START_EN_EX))
>> +			break;
>> +
>> +		udelay(1);
>> +	} while (timeout--);
> 
> This one is obviously not performance critical at all, so use a non-relaxed
> accessor. Same for the other two in this function.
> 
> Are these functions ever called from atomic context? If yes, please document
> from what context they can be called, otherwise please consider changing
> the udelay calls into sleeping waits.
> 

Yes, this is not reentrant. 

>> +int hisi_djtag_writel(struct hisi_djtag_client *client, u32 offset, u32 mod_sel,
>> +							u32 mod_mask, u32 val)
>> +{
>> +	void __iomem *reg_map = client->host->sysctl_reg_map;
>> +	unsigned long flags;
>> +	int ret = 0;
>> +
>> +	spin_lock_irqsave(&client->host->lock, flags);
>> +	ret = client->host->djtag_readwrite(reg_map, offset, mod_sel, mod_mask,
>> +					true, val, 0, NULL);
>> +	if (ret)
>> +		pr_err("djtag_writel: error! ret=%d\n", ret);
>> +	spin_unlock_irqrestore(&client->host->lock, flags);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(hisi_djtag_writel);
> 
> That would of course imply changing the spinlock to a mutex here as well.
> 
>> +static const struct of_device_id djtag_of_match[] = {
>> +	/* for hip05(D02) cpu die */
>> +	{ .compatible = "hisilicon,hip05-cpu-djtag-v1",
>> +		.data = (void *)djtag_readwrite_v1 },
>> +	/* for hip05(D02) io die */
>> +	{ .compatible = "hisilicon,hip05-io-djtag-v1",
>> +		.data = (void *)djtag_readwrite_v1 },
>> +	/* for hip06(D03) cpu die */
>> +	{ .compatible = "hisilicon,hip06-cpu-djtag-v1",
>> +		.data = (void *)djtag_readwrite_v1 },
>> +	/* for hip06(D03) io die */
>> +	{ .compatible = "hisilicon,hip06-io-djtag-v2",
>> +		.data = (void *)djtag_readwrite_v2 },
>> +	/* for hip07(D05) cpu die */
>> +	{ .compatible = "hisilicon,hip07-cpu-djtag-v2",
>> +		.data = (void *)djtag_readwrite_v2 },
>> +	/* for hip07(D05) io die */
>> +	{ .compatible = "hisilicon,hip07-io-djtag-v2",
>> +		.data = (void *)djtag_readwrite_v2 },
>> +	{},
>> +};
> 
> If these are backwards compatible, just mark them as compatible in DT,
> e.g. hip06 can use
> 
> 	compatible = "hisilicon,hip06-cpu-djtag-v1", "hisilicon,hip05-cpu-djtag-v1";
> 
> so you can tell the difference if you need to, but the driver only has to
> list the oldest one here.
> 
> What is the difference between the cpu and io djtag interfaces?
> 
> I think you can also drop the '(void *)'.
> 

OK. We will consider it.

Thanks.
Xiaojun.

>> +static void djtag_register_devices(struct hisi_djtag_host *host)
>> +{
>> +	struct device_node *node;
>> +	struct hisi_djtag_client *client;
>> +
>> +	if (!host->of_node)
>> +		return;
>> +
>> +	for_each_available_child_of_node(host->of_node, node) {
>> +		if (of_node_test_and_set_flag(node, OF_POPULATED))
>> +			continue;
>> +		client = hisi_djtag_of_register_device(host, node);
>> +		list_add(&client->next, &host->client_list);
>> +	}
>> +}
> 
> Can you explain your thoughts behind creating a new bus type
> and adding the child devices manually rather than using
> platform_device structures with of_platform_populate()?
> 
> Do you expect to see other implementations of this bus type
> with incompatible bus drivers?
> 
> 	Arnd
> 
> 
> .
> 

[toc] | [prev] | [next] | [standalone]

#1516904 — Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver

On Tuesday 08 November 2016 12:32 PM, Tan Xiaojun wrote:
> On 2016/11/7 21:26, Arnd Bergmann wrote:
>> On Wednesday, November 2, 2016 11:42:46 AM CET Anurup M wrote:
>>> From: Tan Xiaojun <tanxiaojun@huawei.com>
>>>
>>> 	The Hisilicon Djtag is an independent component which connects
>>> 	with some other components in the SoC by Debug Bus. This driver
>>> 	can be configured to access the registers of connecting components
>>> 	(like L3 cache) during real time debugging.
>> The formatting of the text seems odd, please remove the leading spaces.
>>
> Sorry for that. We will fix it.
>
>>>   drivers/soc/Kconfig                 |   1 +
>>>   drivers/soc/Makefile                |   1 +
>>>   drivers/soc/hisilicon/Kconfig       |  12 +
>>>   drivers/soc/hisilicon/Makefile      |   1 +
>>>   drivers/soc/hisilicon/djtag.c       | 639 ++++++++++++++++++++++++++++++++++++
>>>   include/linux/soc/hisilicon/djtag.h |  38 +++
>> Do you expect other drivers to be added that reference this interface?
>> If not, or if you are unsure, just put all of it under drivers/perf
>> so we don't introduce a global API that has only one user.
>>
> OK. For now, this suggestion sounds good.
>
>>> +
>>> +#include <linux/bitops.h>
>>> +#include <linux/init.h>
>>> +#include <linux/list.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/spinlock.h>
>>> +
>>> +#include <asm-generic/delay.h>
>> Never include files from asm-generic directly except from
>> an architecture specific asm/*.h header file.
>>
>>
> OK. Sorry for that.
>
>>> +DEFINE_IDR(djtag_hosts_idr);
>> make this static
>>
> OK.
>
>>> +static void djtag_read32_relaxed(void __iomem *regs_base, u32 off, u32 *value)
>>> +{
>>> +	void __iomem *reg_addr = regs_base + off;
>>> +
>>> +	*value = readl_relaxed(reg_addr);
>>> +}
>>> +
>>> +static void djtag_write32(void __iomem *regs_base, u32 off, u32 val)
>>> +{
>>> +	void __iomem *reg_addr = regs_base + off;
>>> +
>>> +	writel(val, reg_addr);
>>> +}
>> This looks like an odd combination of interfaces.
>> Why can the reads be "relaxed" when the writes can not?
>>
>> Generally speaking, I'd advise to always use non-relaxed accessors
>> unless there is a strong performance reason, and in that case there
>> should be a comment explaining the use at each of the callers
>> of a relaxed accessor.
>>
> Yes, it is our mistake.
Shall use non-relaxed version and make it consistent.
>>> +	/* ensure the djtag operation is done */
>>> +	do {
>>> +		djtag_read32_relaxed(regs_base, SC_DJTAG_MSTR_START_EN_EX, &rd);
>>> +
>>> +		if (!(rd & DJTAG_MSTR_START_EN_EX))
>>> +			break;
>>> +
>>> +		udelay(1);
>>> +	} while (timeout--);
>> This one is obviously not performance critical at all, so use a non-relaxed
>> accessor. Same for the other two in this function.
>>
>> Are these functions ever called from atomic context? If yes, please document
>> from what context they can be called, otherwise please consider changing
>> the udelay calls into sleeping waits.
>>
> Yes, this is not reentrant.
The read/write functions shall also be called from irq handler (for 
handling counter overflow).
So need to use udelay calls. Shall Document it in v2.
>>> +int hisi_djtag_writel(struct hisi_djtag_client *client, u32 offset, u32 mod_sel,
>>> +							u32 mod_mask, u32 val)
>>> +{
>>> +	void __iomem *reg_map = client->host->sysctl_reg_map;
>>> +	unsigned long flags;
>>> +	int ret = 0;
>>> +
>>> +	spin_lock_irqsave(&client->host->lock, flags);
>>> +	ret = client->host->djtag_readwrite(reg_map, offset, mod_sel, mod_mask,
>>> +					true, val, 0, NULL);
>>> +	if (ret)
>>> +		pr_err("djtag_writel: error! ret=%d\n", ret);
>>> +	spin_unlock_irqrestore(&client->host->lock, flags);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(hisi_djtag_writel);
>> That would of course imply changing the spinlock to a mutex here as well.
>>
>>> +static const struct of_device_id djtag_of_match[] = {
>>> +	/* for hip05(D02) cpu die */
>>> +	{ .compatible = "hisilicon,hip05-cpu-djtag-v1",
>>> +		.data = (void *)djtag_readwrite_v1 },
>>> +	/* for hip05(D02) io die */
>>> +	{ .compatible = "hisilicon,hip05-io-djtag-v1",
>>> +		.data = (void *)djtag_readwrite_v1 },
>>> +	/* for hip06(D03) cpu die */
>>> +	{ .compatible = "hisilicon,hip06-cpu-djtag-v1",
>>> +		.data = (void *)djtag_readwrite_v1 },
>>> +	/* for hip06(D03) io die */
>>> +	{ .compatible = "hisilicon,hip06-io-djtag-v2",
>>> +		.data = (void *)djtag_readwrite_v2 },
>>> +	/* for hip07(D05) cpu die */
>>> +	{ .compatible = "hisilicon,hip07-cpu-djtag-v2",
>>> +		.data = (void *)djtag_readwrite_v2 },
>>> +	/* for hip07(D05) io die */
>>> +	{ .compatible = "hisilicon,hip07-io-djtag-v2",
>>> +		.data = (void *)djtag_readwrite_v2 },
>>> +	{},
>>> +};
>> If these are backwards compatible, just mark them as compatible in DT,
>> e.g. hip06 can use
>>
>> 	compatible = "hisilicon,hip06-cpu-djtag-v1", "hisilicon,hip05-cpu-djtag-v1";
>>
>> so you can tell the difference if you need to, but the driver only has to
>> list the oldest one here.
>>
>> What is the difference between the cpu and io djtag interfaces?
On some chips like hip06, the djtag version is different for IO die.

Thanks,
Anurup
>>
>> I think you can also drop the '(void *)'.
>>
> OK. We will consider it.
>
> Thanks.
> Xiaojun.
>
>>> +static void djtag_register_devices(struct hisi_djtag_host *host)
>>> +{
>>> +	struct device_node *node;
>>> +	struct hisi_djtag_client *client;
>>> +
>>> +	if (!host->of_node)
>>> +		return;
>>> +
>>> +	for_each_available_child_of_node(host->of_node, node) {
>>> +		if (of_node_test_and_set_flag(node, OF_POPULATED))
>>> +			continue;
>>> +		client = hisi_djtag_of_register_device(host, node);
>>> +		list_add(&client->next, &host->client_list);
>>> +	}
>>> +}
>> Can you explain your thoughts behind creating a new bus type
>> and adding the child devices manually rather than using
>> platform_device structures with of_platform_populate()?
>>
>> Do you expect to see other implementations of this bus type
>> with incompatible bus drivers?
>>
>> 	Arnd
>>
>>
>> .
>>
>

[toc] | [prev] | [next] | [standalone]

#1517105 — Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver

On Tuesday, November 8, 2016 1:08:31 PM CET Anurup M wrote:
 
> On Tuesday 08 November 2016 12:32 PM, Tan Xiaojun wrote:
> > On 2016/11/7 21:26, Arnd Bergmann wrote:
> >> On Wednesday, November 2, 2016 11:42:46 AM CET Anurup M wrote:
> >>> From: Tan Xiaojun <tanxiaojun@huawei.com>
> >>> +	/* ensure the djtag operation is done */
> >>> +	do {
> >>> +		djtag_read32_relaxed(regs_base, SC_DJTAG_MSTR_START_EN_EX, &rd);
> >>> +
> >>> +		if (!(rd & DJTAG_MSTR_START_EN_EX))
> >>> +			break;
> >>> +
> >>> +		udelay(1);
> >>> +	} while (timeout--);
> >> This one is obviously not performance critical at all, so use a non-relaxed
> >> accessor. Same for the other two in this function.
> >>
> >> Are these functions ever called from atomic context? If yes, please document
> >> from what context they can be called, otherwise please consider changing
> >> the udelay calls into sleeping waits.
> >>
> > Yes, this is not reentrant.
> The read/write functions shall also be called from irq handler (for 
> handling counter overflow).
> So need to use udelay calls. Shall Document it in v2.

Ok.

> >>> +static const struct of_device_id djtag_of_match[] = {
> >>> +	/* for hip05(D02) cpu die */
> >>> +	{ .compatible = "hisilicon,hip05-cpu-djtag-v1",
> >>> +		.data = (void *)djtag_readwrite_v1 },
> >>> +	/* for hip05(D02) io die */
> >>> +	{ .compatible = "hisilicon,hip05-io-djtag-v1",
> >>> +		.data = (void *)djtag_readwrite_v1 },
> >>> +	/* for hip06(D03) cpu die */
> >>> +	{ .compatible = "hisilicon,hip06-cpu-djtag-v1",
> >>> +		.data = (void *)djtag_readwrite_v1 },
> >>> +	/* for hip06(D03) io die */
> >>> +	{ .compatible = "hisilicon,hip06-io-djtag-v2",
> >>> +		.data = (void *)djtag_readwrite_v2 },
> >>> +	/* for hip07(D05) cpu die */
> >>> +	{ .compatible = "hisilicon,hip07-cpu-djtag-v2",
> >>> +		.data = (void *)djtag_readwrite_v2 },
> >>> +	/* for hip07(D05) io die */
> >>> +	{ .compatible = "hisilicon,hip07-io-djtag-v2",
> >>> +		.data = (void *)djtag_readwrite_v2 },
> >>> +	{},
> >>> +};
> >>
> >> If these are backwards compatible, just mark them as compatible in DT,
> >> e.g. hip06 can use
> >>
> >> 	compatible = "hisilicon,hip06-cpu-djtag-v1", "hisilicon,hip05-cpu-djtag-v1";
> >>
> >> so you can tell the difference if you need to, but the driver only has to
> >> list the oldest one here.
> >>
> >> What is the difference between the cpu and io djtag interfaces?
> On some chips like hip06, the djtag version is different for IO die.

In what way? The driver doesn't seem to care about the difference.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1517198 — Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver

On Tuesday 08 November 2016 05:13 PM, Arnd Bergmann wrote:
> On Tuesday, November 8, 2016 1:08:31 PM CET Anurup M wrote:
>   
>> On Tuesday 08 November 2016 12:32 PM, Tan Xiaojun wrote:
>>> On 2016/11/7 21:26, Arnd Bergmann wrote:
>>>> On Wednesday, November 2, 2016 11:42:46 AM CET Anurup M wrote:
>>>>> From: Tan Xiaojun <tanxiaojun@huawei.com>
>>>>> +	/* ensure the djtag operation is done */
>>>>> +	do {
>>>>> +		djtag_read32_relaxed(regs_base, SC_DJTAG_MSTR_START_EN_EX, &rd);
>>>>> +
>>>>> +		if (!(rd & DJTAG_MSTR_START_EN_EX))
>>>>> +			break;
>>>>> +
>>>>> +		udelay(1);
>>>>> +	} while (timeout--);
>>>> This one is obviously not performance critical at all, so use a non-relaxed
>>>> accessor. Same for the other two in this function.
>>>>
>>>> Are these functions ever called from atomic context? If yes, please document
>>>> from what context they can be called, otherwise please consider changing
>>>> the udelay calls into sleeping waits.
>>>>
>>> Yes, this is not reentrant.
>> The read/write functions shall also be called from irq handler (for
>> handling counter overflow).
>> So need to use udelay calls. Shall Document it in v2.
> Ok.
>
>>>>> +static const struct of_device_id djtag_of_match[] = {
>>>>> +	/* for hip05(D02) cpu die */
>>>>> +	{ .compatible = "hisilicon,hip05-cpu-djtag-v1",
>>>>> +		.data = (void *)djtag_readwrite_v1 },
>>>>> +	/* for hip05(D02) io die */
>>>>> +	{ .compatible = "hisilicon,hip05-io-djtag-v1",
>>>>> +		.data = (void *)djtag_readwrite_v1 },
>>>>> +	/* for hip06(D03) cpu die */
>>>>> +	{ .compatible = "hisilicon,hip06-cpu-djtag-v1",
>>>>> +		.data = (void *)djtag_readwrite_v1 },
>>>>> +	/* for hip06(D03) io die */
>>>>> +	{ .compatible = "hisilicon,hip06-io-djtag-v2",
>>>>> +		.data = (void *)djtag_readwrite_v2 },
>>>>> +	/* for hip07(D05) cpu die */
>>>>> +	{ .compatible = "hisilicon,hip07-cpu-djtag-v2",
>>>>> +		.data = (void *)djtag_readwrite_v2 },
>>>>> +	/* for hip07(D05) io die */
>>>>> +	{ .compatible = "hisilicon,hip07-io-djtag-v2",
>>>>> +		.data = (void *)djtag_readwrite_v2 },
>>>>> +	{},
>>>>> +};
>>>> If these are backwards compatible, just mark them as compatible in DT,
>>>> e.g. hip06 can use
>>>>
>>>> 	compatible = "hisilicon,hip06-cpu-djtag-v1", "hisilicon,hip05-cpu-djtag-v1";
>>>>
>>>> so you can tell the difference if you need to, but the driver only has to
>>>> list the oldest one here.
>>>>
>>>> What is the difference between the cpu and io djtag interfaces?
>> On some chips like hip06, the djtag version is different for IO die.
> In what way? The driver doesn't seem to care about the difference.
There is  a difference in djtag version of CPU and IO die (in some chips).
For ex: in hip06 djtag for CPU is v1 and for IO is v2.
so they use different readwrite handlers djtag_readwrite_(v1/2).

+	/* for hip06(D03) cpu die */
+	{ .compatible = "hisilicon,hip06-cpu-djtag-v1",
+		.data = (void *)djtag_readwrite_v1 },
+	/* for hip06(D03) io die */
+	{ .compatible = "hisilicon,hip06-io-djtag-v2",
+		.data = (void *)djtag_readwrite_v2 },


For the same djtag version, there is no difference in handling in the 
driver.

Thanks,
Anurup
> 	Arnd
>

[toc] | [prev] | [next] | [standalone]

#1517287 — Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver

On Tuesday, November 8, 2016 7:16:30 PM CET Anurup M wrote:
> >>>> If these are backwards compatible, just mark them as compatible in DT,
> >>>> e.g. hip06 can use
> >>>>
> >>>>    compatible = "hisilicon,hip06-cpu-djtag-v1", "hisilicon,hip05-cpu-djtag-v1";
> >>>>
> >>>> so you can tell the difference if you need to, but the driver only has to
> >>>> list the oldest one here.
> >>>>
> >>>> What is the difference between the cpu and io djtag interfaces?
> >> On some chips like hip06, the djtag version is different for IO die.
> > In what way? The driver doesn't seem to care about the difference.
> There is  a difference in djtag version of CPU and IO die (in some chips).
> For ex: in hip06 djtag for CPU is v1 and for IO is v2.
> so they use different readwrite handlers djtag_readwrite_(v1/2).
> 
> +       /* for hip06(D03) cpu die */
> +       { .compatible = "hisilicon,hip06-cpu-djtag-v1",
> +               .data = (void *)djtag_readwrite_v1 },
> +       /* for hip06(D03) io die */
> +       { .compatible = "hisilicon,hip06-io-djtag-v2",
> +               .data = (void *)djtag_readwrite_v2 },
> 
> 
> For the same djtag version, there is no difference in handling in the 
> driver.

Right, but my point was about the compatibility with the older chips
using the same IP block, marking the ones as compatible that actually
use the same interface.

I also see that the compatible strings have the version included in
them, and you can probably drop them by requiring them only in the
fallback:

	compatible = "hisilicon,hip05-cpu-djtag", "hisilicon,djtag-v1";
	compatible = "hisilicon,hip05-io-djtag", "hisilicon,djtag-v1";
	compatible = "hisilicon,hip06-cpu-djtag", "hisilicon,djtag-v1";
	compatible = "hisilicon,hip06-io-djtag", "hisilicon,djtag-v2";
	compatible = "hisilicon,hip07-cpu-djtag", "hisilicon,djtag-v2";
	compatible = "hisilicon,hip07-io-djtag", "hisilicon,djtag-v2";

We want to have the first entry be as specific as possible, but
the last (second) entry is the one that can be used by the driver
for matching. When a future hip08/hip09/... chip uses an existing
interface, you then don't have to update the driver.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1517761 — Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver

On Tuesday 08 November 2016 08:38 PM, Arnd Bergmann wrote:
> On Tuesday, November 8, 2016 7:16:30 PM CET Anurup M wrote:
>>>>>> If these are backwards compatible, just mark them as compatible in DT,
>>>>>> e.g. hip06 can use
>>>>>>
>>>>>>     compatible = "hisilicon,hip06-cpu-djtag-v1", "hisilicon,hip05-cpu-djtag-v1";
>>>>>>
>>>>>> so you can tell the difference if you need to, but the driver only has to
>>>>>> list the oldest one here.
>>>>>>
>>>>>> What is the difference between the cpu and io djtag interfaces?
>>>> On some chips like hip06, the djtag version is different for IO die.
>>> In what way? The driver doesn't seem to care about the difference.
>> There is  a difference in djtag version of CPU and IO die (in some chips).
>> For ex: in hip06 djtag for CPU is v1 and for IO is v2.
>> so they use different readwrite handlers djtag_readwrite_(v1/2).
>>
>> +       /* for hip06(D03) cpu die */
>> +       { .compatible = "hisilicon,hip06-cpu-djtag-v1",
>> +               .data = (void *)djtag_readwrite_v1 },
>> +       /* for hip06(D03) io die */
>> +       { .compatible = "hisilicon,hip06-io-djtag-v2",
>> +               .data = (void *)djtag_readwrite_v2 },
>>
>>
>> For the same djtag version, there is no difference in handling in the
>> driver.
> Right, but my point was about the compatibility with the older chips
> using the same IP block, marking the ones as compatible that actually
> use the same interface.
>
> I also see that the compatible strings have the version included in
> them, and you can probably drop them by requiring them only in the
> fallback:
>
> 	compatible = "hisilicon,hip05-cpu-djtag", "hisilicon,djtag-v1";
> 	compatible = "hisilicon,hip05-io-djtag", "hisilicon,djtag-v1";
> 	compatible = "hisilicon,hip06-cpu-djtag", "hisilicon,djtag-v1";
> 	compatible = "hisilicon,hip06-io-djtag", "hisilicon,djtag-v2";
> 	compatible = "hisilicon,hip07-cpu-djtag", "hisilicon,djtag-v2";
> 	compatible = "hisilicon,hip07-io-djtag", "hisilicon,djtag-v2";
>
> We want to have the first entry be as specific as possible, but
> the last (second) entry is the one that can be used by the driver
> for matching. When a future hip08/hip09/... chip uses an existing
> interface, you then don't have to update the driver.
Thanks. I had a similar thought on this. So as I have the version string 
in the
second entry "-v(1/2)".
I can use it in driver for matching. So i think  I will change it as below.
Please correct me if my understanding is wrong.

  static const struct of_device_id djtag_of_match[] = {
-       /* for hip05(D02) cpu die */
-       { .compatible = "hisilicon,hip05-cpu-djtag-v1",
+       /* for hisi djtag-v1 cpu die */
+       { .compatible = "hisilicon,hisi-cpu-djtag-v1",
                 .data = djtag_readwrite_v1 },
-       /* for hip05(D02) io die */
-       { .compatible = "hisilicon,hip05-io-djtag-v1",
+       /* for hisi djtag-v1 io die */
+       { .compatible = "hisilicon,hisi-io-djtag-v1",
                 .data = djtag_readwrite_v1 },
-       /* for hip06(D03) cpu die */
-       { .compatible = "hisilicon,hip06-cpu-djtag-v1",
-               .data = djtag_readwrite_v1 },
-       /* for hip06(D03) io die */
-       { .compatible = "hisilicon,hip06-io-djtag-v2",
-               .data = djtag_readwrite_v2 },
-       /* for hip07(D05) cpu die */
-       { .compatible = "hisilicon,hip07-cpu-djtag-v2",
+       /* for hisi djtag-v2 cpu die */
+       { .compatible = "hisilicon,hisi-cpu-djtag-v2",
                 .data = djtag_readwrite_v2 },
-       /* for hip07(D05) io die */
-       { .compatible = "hisilicon,hip07-io-djtag-v2",
+       /* for hisi djtag-v2 io die */
+       { .compatible = "hisilicon,hisi-io-djtag-v2",
                 .data = (djtag_readwrite_v2 },
         {},
  };

Thanks,
Anurup
> 	Arnd

[toc] | [prev] | [next] | [standalone]

#1518512 — Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver

On Wednesday, November 9, 2016 9:58:38 AM CET Anurup M wrote:
> 
> > I also see that the compatible strings have the version included in
> > them, and you can probably drop them by requiring them only in the
> > fallback:
> >
> >       compatible = "hisilicon,hip05-cpu-djtag", "hisilicon,djtag-v1";
> >       compatible = "hisilicon,hip05-io-djtag", "hisilicon,djtag-v1";
> >       compatible = "hisilicon,hip06-cpu-djtag", "hisilicon,djtag-v1";
> >       compatible = "hisilicon,hip06-io-djtag", "hisilicon,djtag-v2";
> >       compatible = "hisilicon,hip07-cpu-djtag", "hisilicon,djtag-v2";
> >       compatible = "hisilicon,hip07-io-djtag", "hisilicon,djtag-v2";
> >
> > We want to have the first entry be as specific as possible, but
> > the last (second) entry is the one that can be used by the driver
> > for matching. When a future hip08/hip09/... chip uses an existing
> > interface, you then don't have to update the driver.
> Thanks. I had a similar thought on this. So as I have the version string 
> in the
> second entry "-v(1/2)".
> I can use it in driver for matching. So i think  I will change it as below.
> Please correct me if my understanding is wrong.
> 
>   static const struct of_device_id djtag_of_match[] = {
> -       /* for hip05(D02) cpu die */
> -       { .compatible = "hisilicon,hip05-cpu-djtag-v1",
> +       /* for hisi djtag-v1 cpu die */
> +       { .compatible = "hisilicon,hisi-cpu-djtag-v1",
>                  .data = djtag_readwrite_v1 },
> -       /* for hip05(D02) io die */
> -       { .compatible = "hisilicon,hip05-io-djtag-v1",
> +       /* for hisi djtag-v1 io die */
> +       { .compatible = "hisilicon,hisi-io-djtag-v1",

From the code it looks like "hisilicon,hisi-io-djtag-v1" and
"hisilicon,hisi-cpu-djtag-v1" have the same register-level interface,
so we just need one compatible string for them to match the driver.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1519669 — Re: [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver

On Thursday 10 November 2016 03:10 AM, Arnd Bergmann wrote:
> On Wednesday, November 9, 2016 9:58:38 AM CET Anurup M wrote:
>>> I also see that the compatible strings have the version included in
>>> them, and you can probably drop them by requiring them only in the
>>> fallback:
>>>
>>>        compatible = "hisilicon,hip05-cpu-djtag", "hisilicon,djtag-v1";
>>>        compatible = "hisilicon,hip05-io-djtag", "hisilicon,djtag-v1";
>>>        compatible = "hisilicon,hip06-cpu-djtag", "hisilicon,djtag-v1";
>>>        compatible = "hisilicon,hip06-io-djtag", "hisilicon,djtag-v2";
>>>        compatible = "hisilicon,hip07-cpu-djtag", "hisilicon,djtag-v2";
>>>        compatible = "hisilicon,hip07-io-djtag", "hisilicon,djtag-v2";
>>>
>>> We want to have the first entry be as specific as possible, but
>>> the last (second) entry is the one that can be used by the driver
>>> for matching. When a future hip08/hip09/... chip uses an existing
>>> interface, you then don't have to update the driver.
>> Thanks. I had a similar thought on this. So as I have the version string
>> in the
>> second entry "-v(1/2)".
>> I can use it in driver for matching. So i think  I will change it as below.
>> Please correct me if my understanding is wrong.
>>
>>    static const struct of_device_id djtag_of_match[] = {
>> -       /* for hip05(D02) cpu die */
>> -       { .compatible = "hisilicon,hip05-cpu-djtag-v1",
>> +       /* for hisi djtag-v1 cpu die */
>> +       { .compatible = "hisilicon,hisi-cpu-djtag-v1",
>>                   .data = djtag_readwrite_v1 },
>> -       /* for hip05(D02) io die */
>> -       { .compatible = "hisilicon,hip05-io-djtag-v1",
>> +       /* for hisi djtag-v1 io die */
>> +       { .compatible = "hisilicon,hisi-io-djtag-v1",
>  From the code it looks like "hisilicon,hisi-io-djtag-v1" and
> "hisilicon,hisi-cpu-djtag-v1" have the same register-level interface,
> so we just need one compatible string for them to match the driver.
>
> 	Arnd
The djtag versions in CPU die and IO die can be different in the same chip.
For example in hip06, the CPU die has djtag-v1 whereas IO die has djtag-v2.
So I think it need two different compatible string

for hip06 chip CPU DIE "hisilicon,hisi-cpu-djtag-v1"
for hip06 chip IO DIE "hisilicon,hisi-io-djtag-v2"

Thanks,
Anurup

[toc] | [prev] | [next] | [standalone]

#1513934 — [PATCH v1 07/11] perf: hisi: Add support for Hisilicon SoC event counters

	1. Hip05/06/07 uncore PMU to support different hardware
	   event counters.
	2. Hisilicon PMU shall use the DJTAG hardware interface
	   to access hardware event counters and configuration
	   register.
	3. Routines to initialize and setup PMU.
	4. Routines to enable/disable/add/del/start/stop hardware
	   event counting.
	5. Add support to count L3 cache hardware events.

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/perf/Makefile                    |   1 +
 drivers/perf/hisilicon/Makefile          |   1 +
 drivers/perf/hisilicon/hisi_uncore_l3c.c | 571 +++++++++++++++++++++++++++++++
 drivers/perf/hisilicon/hisi_uncore_l3c.h |  67 ++++
 drivers/perf/hisilicon/hisi_uncore_pmu.c | 331 ++++++++++++++++++
 drivers/perf/hisilicon/hisi_uncore_pmu.h | 108 ++++++
 6 files changed, 1079 insertions(+)
 create mode 100644 drivers/perf/hisilicon/Makefile
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_l3c.c
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_l3c.h
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.c
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.h

diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index b116e98..061f229 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_ARM_PMU) += arm_pmu.o
 obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
+obj-$(CONFIG_HISI_PMU) += hisilicon/
diff --git a/drivers/perf/hisilicon/Makefile b/drivers/perf/hisilicon/Makefile
new file mode 100644
index 0000000..e1766cf
--- /dev/null
+++ b/drivers/perf/hisilicon/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_HISI_PMU) += hisi_uncore_pmu.o hisi_uncore_l3c.o
diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c.c b/drivers/perf/hisilicon/hisi_uncore_l3c.c
new file mode 100644
index 0000000..f78f7b2
--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_l3c.c
@@ -0,0 +1,571 @@
+/*
+ * HiSilicon SoC L3C Hardware event counters support
+ *
+ * Copyright (C) 2016 Huawei Technologies Limited
+ * Author: Anurup M <anurup.m@huawei.com>
+ *
+ * This code is based on the uncore PMU's like arm-cci and
+ * arm-ccn.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/>.
+ */
+#include <linux/bitmap.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/perf_event.h>
+#include "hisi_uncore_l3c.h"
+
+static inline int hisi_l3c_counter_valid(int idx)
+{
+	return (idx >= HISI_IDX_L3C_COUNTER0 &&
+			idx <= HISI_IDX_L3C_COUNTER_MAX);
+}
+
+static u32 hisi_read_l3c_counter(struct hisi_l3c_data *l3c_hwmod_data,
+						int cntr_idx, int bank_idx)
+{
+	struct hisi_djtag_client *client = l3c_hwmod_data->client;
+	u32 module_id = l3c_hwmod_data->l3c_hwcfg.module_id[bank_idx];
+	u32 cfg_en = l3c_hwmod_data->l3c_hwcfg.bank_cfgen[bank_idx];
+	u32 reg_offset, value;
+
+	reg_offset = l3c_hwmod_data->l3c_hwcfg.counter_reg0_off +
+							(cntr_idx * 4);
+
+	hisi_djtag_readreg(module_id, cfg_en, reg_offset, client, &value);
+
+	return value;
+}
+
+static u64 hisi_l3c_event_update(struct perf_event *event,
+				struct hw_perf_event *hwc, int idx)
+{
+	struct hisi_pmu *pl3c_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_l3c_data *l3c_hwmod_data = pl3c_pmu->hwmod_data;
+	u64 delta, prev_raw_count, total_raw_count = 0, avg_raw_count = 0;
+	u32 num_banks = l3c_hwmod_data->l3c_hwcfg.num_banks;
+	int i;
+
+	if (!hisi_l3c_counter_valid(idx)) {
+		dev_err(pl3c_pmu->dev, "Unsupported event index:%d!\n", idx);
+		return 0;
+	}
+
+	/* Check if the L3C data is initialized for this SCCL */
+	if (!l3c_hwmod_data->client) {
+		dev_err(pl3c_pmu->dev, "SCL=%d not initialized!\n",
+							pl3c_pmu->scl_id);
+		return 0;
+	}
+
+	do {
+		/* Get count from individual L3C banks and sum them up */
+		for (i = 0; i < num_banks; i++) {
+			total_raw_count += hisi_read_l3c_counter(l3c_hwmod_data,
+									idx, i);
+		}
+		prev_raw_count = local64_read(&hwc->prev_count);
+
+		/*
+		 * As prev_raw_count is updated with average value of
+		 * L3 cache banks, we multiply it by no of banks and
+		 * compute the delta
+		 */
+		delta = (total_raw_count - (prev_raw_count * num_banks)) &
+								HISI_MAX_PERIOD;
+
+		local64_add(delta, &event->count);
+
+		/*
+		 * Divide by num of banks to get average count and
+		 * update prev_count with this value
+		 */
+		avg_raw_count = total_raw_count / num_banks;
+	} while (local64_cmpxchg(
+			 &hwc->prev_count, prev_raw_count, avg_raw_count) !=
+							 prev_raw_count);
+
+	return total_raw_count;
+}
+
+static void hisi_set_l3c_evtype(struct hisi_pmu *pl3c_pmu, int idx, u32 val)
+{
+	struct hisi_djtag_client *client;
+	struct hisi_l3c_data *l3c_hwmod_data = pl3c_pmu->hwmod_data;
+	u32 reg_offset = l3c_hwmod_data->l3c_hwcfg.evtype_reg0_off;
+	u32 event_value, value = 0;
+	u32 cfg_en, module_id;
+	int i;
+
+	event_value = (val -
+			HISI_HWEVENT_L3C_READ_ALLOCATE);
+
+	/* Select the appropriate Event select register */
+	if (idx > 3)
+		reg_offset += 4;
+
+	/* Value to write to event type register */
+	val = event_value << (8 * idx);
+
+	/* Find the djtag Identifier of the Unit */
+	client = l3c_hwmod_data->client;
+
+	/*
+	 * Set the event in L3C_EVENT_TYPEx Register
+	 * for all L3C banks
+	 */
+	for (i = 0; i < l3c_hwmod_data->l3c_hwcfg.num_banks; i++) {
+		module_id = l3c_hwmod_data->l3c_hwcfg.module_id[i];
+		cfg_en = l3c_hwmod_data->l3c_hwcfg.bank_cfgen[i];
+		hisi_djtag_readreg(module_id,
+				cfg_en,
+				reg_offset,
+				client, &value);
+
+		value &= ~(0xff << (8 * idx));
+		value |= val;
+
+		hisi_djtag_writereg(module_id,
+				cfg_en,
+				reg_offset,
+				value,
+				client);
+	}
+}
+
+static u32 hisi_write_l3c_counter(struct hisi_pmu *pl3c_pmu,
+				struct hw_perf_event *hwc, u32 value)
+{
+	struct hisi_djtag_client *client;
+	struct hisi_l3c_data *l3c_hwmod_data = pl3c_pmu->hwmod_data;
+	u32 reg_offset, cfg_en, module_id;
+	int i, ret = 0;
+	int idx = GET_CNTR_IDX(hwc);
+
+	if (!hisi_l3c_counter_valid(idx)) {
+		dev_err(pl3c_pmu->dev,
+				"Unsupported event index:%d!\n", idx);
+		return -EINVAL;
+	}
+
+	reg_offset = l3c_hwmod_data->l3c_hwcfg.counter_reg0_off +
+							(idx * 4);
+
+	client = l3c_hwmod_data->client;
+
+	for (i = 0; i < l3c_hwmod_data->l3c_hwcfg.num_banks; i++) {
+		module_id = l3c_hwmod_data->l3c_hwcfg.module_id[i];
+		cfg_en = l3c_hwmod_data->l3c_hwcfg.bank_cfgen[i];
+		ret = hisi_djtag_writereg(module_id,
+					cfg_en,
+					reg_offset,
+					value,
+					client);
+		if (!ret)
+			ret = value;
+	}
+
+	return ret;
+}
+
+static void hisi_enable_l3c_counter(struct hisi_pmu *pl3c_pmu, int idx)
+{
+	struct hisi_djtag_client *client;
+	struct hisi_l3c_data *l3c_hwmod_data = pl3c_pmu->hwmod_data;
+	u32 reg_offset = l3c_hwmod_data->l3c_hwcfg.event_ctrl_reg_off;
+	u32 eventen = l3c_hwmod_data->l3c_hwcfg.event_enable;
+	u32 value, cfg_en, module_id;
+	int i;
+
+	if (!hisi_l3c_counter_valid(idx)) {
+		dev_err(pl3c_pmu->dev,
+				"Unsupported event index:%d!\n", idx);
+		return;
+	}
+
+	client = l3c_hwmod_data->client;
+
+	/*
+	 * Set the event_bus_en bit in L3C AUCNTRL to enable counting
+	 * for all L3C banks
+	 */
+	for (i = 0; i < l3c_hwmod_data->l3c_hwcfg.num_banks; i++) {
+		module_id = l3c_hwmod_data->l3c_hwcfg.module_id[i];
+		cfg_en = l3c_hwmod_data->l3c_hwcfg.bank_cfgen[i];
+		hisi_djtag_readreg(module_id,
+				cfg_en,
+				reg_offset,
+				client, &value);
+
+		value |= eventen;
+		hisi_djtag_writereg(module_id,
+				cfg_en,
+				reg_offset,
+				value,
+				client);
+	}
+}
+
+static void hisi_disable_l3c_counter(struct hisi_pmu *pl3c_pmu, int idx)
+{
+	struct hisi_djtag_client *client;
+	struct hisi_l3c_data *l3c_hwmod_data = pl3c_pmu->hwmod_data;
+	u32 reg_offset = l3c_hwmod_data->l3c_hwcfg.event_ctrl_reg_off;
+	u32 eventen = l3c_hwmod_data->l3c_hwcfg.event_enable;
+	u32 value, cfg_en, module_id;
+	int i;
+
+	if (!hisi_l3c_counter_valid(idx)) {
+		dev_err(pl3c_pmu->dev,
+				"Unsupported event index:%d!\n", idx);
+		return;
+	}
+
+	/* Find the djtag Identifier of the Unit */
+	client = l3c_hwmod_data->client;
+
+	/*
+	 * Clear the event_bus_en bit in L3C AUCNTRL if no other
+	 * event counting for all L3C banks
+	 */
+	for (i = 0; i < l3c_hwmod_data->l3c_hwcfg.num_banks; i++) {
+		module_id = l3c_hwmod_data->l3c_hwcfg.module_id[i];
+		cfg_en = l3c_hwmod_data->l3c_hwcfg.bank_cfgen[i];
+		hisi_djtag_readreg(module_id,
+				cfg_en,
+				reg_offset,
+				client, &value);
+
+		value &= ~(eventen);
+		hisi_djtag_writereg(module_id,
+				cfg_en,
+				reg_offset,
+				value,
+				client);
+	}
+}
+
+static void hisi_clear_l3c_event_idx(struct hisi_pmu *pl3c_pmu,
+							int idx)
+{
+	struct hisi_djtag_client *client;
+	struct hisi_l3c_data *l3c_hwmod_data = pl3c_pmu->hwmod_data;
+	u32 reg_offset = l3c_hwmod_data->l3c_hwcfg.evtype_reg0_off;
+	void *bitmap_addr;
+	u32 cfg_en, value, module_id;
+	int i;
+
+	if (!hisi_l3c_counter_valid(idx)) {
+		dev_err(pl3c_pmu->dev,
+				"Unsupported event index:%d!\n", idx);
+		return;
+	}
+
+	bitmap_addr = l3c_hwmod_data->hisi_l3c_event_used_mask;
+
+	__clear_bit(idx, bitmap_addr);
+
+	/* Clear Counting in L3C event config register */
+	if (idx > 3)
+		reg_offset += 4;
+
+	client = l3c_hwmod_data->client;
+
+	/*
+	 * Clear the event in L3C_EVENT_TYPEx Register
+	 * for all L3C banks
+	 */
+	for (i = 0; i < l3c_hwmod_data->l3c_hwcfg.num_banks; i++) {
+		module_id = l3c_hwmod_data->l3c_hwcfg.module_id[i];
+		cfg_en = l3c_hwmod_data->l3c_hwcfg.bank_cfgen[i];
+		hisi_djtag_readreg(module_id,
+				cfg_en,
+				reg_offset,
+				client, &value);
+
+		value &= ~(0xff << (8 * idx));
+		value |= (0xff << (8 * idx));
+		hisi_djtag_writereg(module_id,
+				cfg_en,
+				reg_offset,
+				value,
+				client);
+	}
+}
+
+static int hisi_l3c_get_event_idx(struct hisi_pmu *pl3c_pmu)
+{
+	struct hisi_l3c_data *l3c_hwmod_data = pl3c_pmu->hwmod_data;
+	int event_idx;
+
+	event_idx =
+		find_first_zero_bit(
+			l3c_hwmod_data->hisi_l3c_event_used_mask,
+					 pl3c_pmu->num_counters);
+
+	if (event_idx == HISI_MAX_CFG_L3C_CNTR)
+		return -EAGAIN;
+
+	__set_bit(event_idx,
+		l3c_hwmod_data->hisi_l3c_event_used_mask);
+
+	return event_idx;
+}
+
+static void hisi_free_l3c_data(struct hisi_pmu *pl3c_pmu)
+{
+	kfree(pl3c_pmu->hwmod_data);
+	pl3c_pmu->hwmod_data = NULL;
+}
+
+static int init_hisi_l3c_hwcfg(struct device *dev,
+				struct hisi_l3c_data *pl3c_data)
+{
+	struct hisi_l3c_hwcfg *pl3c_hwcfg = &pl3c_data->l3c_hwcfg;
+	struct device_node *node = dev->of_node;
+	u32 prop_len;
+	int ret;
+
+	if (of_property_read_u32(node, "counter-reg",
+				     &pl3c_hwcfg->counter_reg0_off)) {
+		dev_err(dev, "DT:Couldnot read counter-reg!\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "evctrl-reg",
+				     &pl3c_hwcfg->event_ctrl_reg_off)) {
+		dev_err(dev, "DT:Couldnot read evctrl-reg!\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "event-en",
+				     &pl3c_hwcfg->event_enable)) {
+		dev_err(dev, "DT:Couldnot read event-en!\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "evtype-reg",
+				     &pl3c_hwcfg->evtype_reg0_off)) {
+		dev_err(dev, "DT:Couldnot read evtype-reg!\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "num-banks",
+				     &pl3c_hwcfg->num_banks)) {
+		dev_err(dev, "DT:Couldnot read num-banks!\n");
+		return -EINVAL;
+	}
+
+	prop_len = of_property_count_u32_elems(node, "module-id");
+	if (prop_len != pl3c_hwcfg->num_banks) {
+		dev_err(dev, "DT:module-id entry not valid!\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32_array(node, "module-id",
+					  &pl3c_hwcfg->module_id[0],
+						  pl3c_hwcfg->num_banks);
+	if (ret < 0) {
+		dev_err(dev, "DT:Couldnot read module-id!\n");
+		return -EINVAL;
+	}
+
+	prop_len = of_property_count_u32_elems(node, "cfgen-map");
+	if (prop_len != pl3c_hwcfg->num_banks) {
+		dev_err(dev, "DT:cfgen-map entrynot valid!\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32_array(node, "cfgen-map",
+					&pl3c_hwcfg->bank_cfgen[0],
+						pl3c_hwcfg->num_banks);
+	if (ret < 0) {
+		dev_err(dev, "DT:Couldnot read cfgen-map!\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int init_hisi_l3c_data(struct device *dev,
+					struct hisi_pmu *pl3c_pmu,
+					struct hisi_djtag_client *client)
+{
+	struct hisi_l3c_data *l3c_hwmod_data = NULL;
+	int ret;
+
+	l3c_hwmod_data = kzalloc(sizeof(struct hisi_l3c_data),
+							GFP_KERNEL);
+	if (!l3c_hwmod_data)
+		return -ENOMEM;
+
+	/* Set the djtag Identifier */
+	l3c_hwmod_data->client = client;
+
+	pl3c_pmu->hw_events.events = devm_kcalloc(dev,
+					pl3c_pmu->num_counters,
+					sizeof(*pl3c_pmu->hw_events.events),
+					GFP_KERNEL);
+	if (!pl3c_pmu->hw_events.events) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	raw_spin_lock_init(&pl3c_pmu->hw_events.pmu_lock);
+
+	pl3c_pmu->hwmod_data = l3c_hwmod_data;
+
+	ret = init_hisi_l3c_hwcfg(dev, l3c_hwmod_data);
+	if (ret)
+		goto fail;
+
+	return 0;
+
+fail:
+	hisi_free_l3c_data(pl3c_pmu);
+	return ret;
+}
+
+static struct hisi_uncore_ops hisi_uncore_l3c_ops = {
+	.set_evtype = hisi_set_l3c_evtype,
+	.set_event_period = hisi_pmu_set_event_period,
+	.get_event_idx = hisi_l3c_get_event_idx,
+	.clear_event_idx = hisi_clear_l3c_event_idx,
+	.event_update = hisi_l3c_event_update,
+	.enable_counter = hisi_enable_l3c_counter,
+	.disable_counter = hisi_disable_l3c_counter,
+	.write_counter = hisi_write_l3c_counter,
+};
+
+static int hisi_l3c_pmu_init(struct device *dev,
+					struct hisi_pmu *pl3c_pmu)
+{
+	int ret;
+
+	/* Read common PMU properties */
+	ret = hisi_uncore_common_fwprop_read(dev, pl3c_pmu);
+	if (ret)
+		return ret;
+
+	pl3c_pmu->name = kasprintf(GFP_KERNEL, "hisi_l3c%d",
+						pl3c_pmu->scl_id);
+	pl3c_pmu->ops = &hisi_uncore_l3c_ops;
+	pl3c_pmu->dev = dev;
+
+	/* Pick one core to use for cpumask attributes */
+	cpumask_set_cpu(smp_processor_id(), &pl3c_pmu->cpu);
+
+	return 0;
+}
+
+static int hisi_pmu_l3c_dev_probe(struct hisi_djtag_client *client)
+{
+	struct hisi_pmu *pl3c_pmu = NULL;
+	struct device *dev = &client->dev;
+	int ret;
+
+	pl3c_pmu = hisi_pmu_alloc(dev);
+	if (IS_ERR(pl3c_pmu))
+		return PTR_ERR(pl3c_pmu);
+
+	ret = hisi_l3c_pmu_init(dev, pl3c_pmu);
+	if (ret)
+		return ret;
+
+	ret = init_hisi_l3c_data(dev, pl3c_pmu, client);
+	if (ret)
+		goto fail_init;
+
+	/* Register with perf PMU */
+	pl3c_pmu->pmu = (struct pmu) {
+		.name = pl3c_pmu->name,
+		.task_ctx_nr = perf_invalid_context,
+		.event_init = hisi_uncore_pmu_event_init,
+		.add = hisi_uncore_pmu_add,
+		.del = hisi_uncore_pmu_del,
+		.start = hisi_uncore_pmu_start,
+		.stop = hisi_uncore_pmu_stop,
+		.read = hisi_uncore_pmu_read,
+	};
+
+	ret = hisi_uncore_pmu_setup(pl3c_pmu, pl3c_pmu->name);
+	if (ret) {
+		dev_err(dev, "hisi_uncore_pmu_init FAILED!!\n");
+		goto fail;
+	}
+
+	/* Set the drv data to l3c_pmu */
+	dev_set_drvdata(dev, pl3c_pmu);
+
+	return 0;
+
+fail:
+	hisi_free_l3c_data(pl3c_pmu);
+
+fail_init:
+	dev_err(dev, "%s failed\n", __func__);
+	return ret;
+}
+
+static int hisi_pmu_l3c_dev_remove(struct hisi_djtag_client *client)
+{
+	struct hisi_pmu *pl3c_pmu = NULL;
+	struct device *dev = &client->dev;
+
+	pl3c_pmu = dev_get_drvdata(dev);
+
+	perf_pmu_unregister(&pl3c_pmu->pmu);
+	hisi_free_l3c_data(pl3c_pmu);
+
+	return 0;
+}
+
+static const struct of_device_id l3c_of_match[] = {
+	{ .compatible = "hisilicon,hisi-pmu-l3c-v1", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, l3c_of_match);
+
+static struct hisi_djtag_driver hisi_pmu_l3c_driver = {
+	.driver = {
+		.name = "hisi-pmu-l3c",
+		.of_match_table = l3c_of_match,
+	},
+	.probe = hisi_pmu_l3c_dev_probe,
+	.remove = hisi_pmu_l3c_dev_remove,
+};
+
+static int __init hisi_pmu_l3c_init(void)
+{
+	int rc;
+
+	rc = hisi_djtag_register_driver(THIS_MODULE, &hisi_pmu_l3c_driver);
+	if (rc < 0) {
+		pr_err("hisi pmu l3c init failed, rc=%d\n", rc);
+		return rc;
+	}
+
+	return 0;
+}
+module_init(hisi_pmu_l3c_init);
+
+static void __exit hisi_pmu_l3c_exit(void)
+{
+	hisi_djtag_unregister_driver(&hisi_pmu_l3c_driver);
+
+}
+module_exit(hisi_pmu_l3c_exit);
+
+MODULE_DESCRIPTION("HiSilicon SoC HIP0x L3C PMU driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Anurup M");
diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c.h b/drivers/perf/hisilicon/hisi_uncore_l3c.h
new file mode 100644
index 0000000..a4a1777
--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_l3c.h
@@ -0,0 +1,67 @@
+/*
+ * HiSilicon SoC L3C Hardware event counters support
+ *
+ * Copyright (C) 2016 Huawei Technologies Limited
+ * Author: Anurup M <anurup.m@huawei.com>
+ *
+ * This code is based on the uncore PMU's like arm-cci and
+ * arm-ccn.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/>.
+ */
+#ifndef __HISI_UNCORE_L3C_H__
+#define __HISI_UNCORE_L3C_H__
+
+#include "hisi_uncore_pmu.h"
+
+/*
+ * ARMv8 HiSilicon L3C RAW event types.
+ */
+enum armv8_hisi_l3c_event_types {
+	HISI_HWEVENT_L3C_READ_ALLOCATE		= 0x0,
+	HISI_HWEVENT_L3C_WRITE_ALLOCATE		= 0x01,
+	HISI_HWEVENT_L3C_READ_NOALLOCATE	= 0x02,
+	HISI_HWEVENT_L3C_WRITE_NOALLOCATE	= 0x03,
+	HISI_HWEVENT_L3C_READ_HIT		= 0x04,
+	HISI_HWEVENT_L3C_WRITE_HIT		= 0x05,
+	HISI_HWEVENT_L3C_EVENT_MAX		= 0x15,
+};
+
+/*
+ * ARMv8 HiSilicon Hardware counter Index.
+ */
+enum armv8_hisi_l3c_counters {
+	HISI_IDX_L3C_COUNTER0		= 0x0,
+	HISI_IDX_L3C_COUNTER_MAX	= 0x7,
+};
+
+#define HISI_MAX_CFG_L3C_CNTR	0x08
+
+struct hisi_l3c_hwcfg {
+	u32 evtype_reg0_off;
+	u32 counter_reg0_off;
+	u32 event_ctrl_reg_off;
+	u32 event_enable;
+	u32 module_id[MAX_BANKS];
+	u32 num_banks;
+	u32 bank_cfgen[MAX_BANKS];
+};
+
+struct hisi_l3c_data {
+	struct hisi_djtag_client *client;
+	DECLARE_BITMAP(hisi_l3c_event_used_mask,
+				HISI_MAX_CFG_L3C_CNTR);
+	struct hisi_l3c_hwcfg l3c_hwcfg;
+};
+
+#endif /* __HISI_UNCORE_L3C_H__ */
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
new file mode 100644
index 0000000..8d29fcc
--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -0,0 +1,331 @@
+/*
+ * HiSilicon SoC Hardware event counters support
+ *
+ * Copyright (C) 2016 Huawei Technologies Limited
+ * Author: Anurup M <anurup.m@huawei.com>
+ *
+ * This code is based on the uncore PMU's like arm-cci and
+ * arm-ccn.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/>.
+ */
+#include <linux/bitmap.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/perf_event.h>
+#include "hisi_uncore_pmu.h"
+
+/* djtag read interface - Call djtag driver to access SoC registers */
+int hisi_djtag_readreg(int module_id, int bank, u32 offset,
+				struct hisi_djtag_client *client, u32 *pvalue)
+{
+	int ret;
+	u32 chain_id = 0;
+
+	while (bank != 1) {
+		bank = (bank >> 0x1);
+		chain_id++;
+	}
+
+	ret = hisi_djtag_readl(client, offset, module_id,
+						chain_id, pvalue);
+	if (ret)
+		dev_err(&client->dev, "read failed, ret=%d!\n", ret);
+
+	return ret;
+}
+
+/* djtag write interface - Call djtag driver  to access SoC registers */
+int hisi_djtag_writereg(int module_id, int bank,
+				u32 offset, u32 value,
+				struct hisi_djtag_client *client)
+{
+	int ret;
+
+	ret = hisi_djtag_writel(client, offset, module_id,
+						HISI_DJTAG_MOD_MASK, value);
+	if (ret)
+		dev_err(&client->dev, "write failed, ret=%d!\n", ret);
+
+	return ret;
+}
+
+static int pmu_map_event(struct perf_event *event)
+{
+	return (int)(event->attr.config & HISI_EVTYPE_EVENT);
+}
+
+static int
+__hw_perf_event_init(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct device *dev = phisi_pmu->dev;
+	int mapping;
+
+	mapping = pmu_map_event(event);
+	if (mapping < 0) {
+		dev_err(dev, "event %x:%llx not supported\n", event->attr.type,
+							 event->attr.config);
+		return mapping;
+	}
+
+	/*
+	 * We don't assign an index until we actually place the event onto
+	 * hardware. Use -1 to signify that we haven't decided where to put it
+	 * yet.
+	 */
+	hwc->idx		= -1;
+	hwc->config		= 0;
+	hwc->event_base		= 0;
+
+	/* For HiSilicon SoC L3C update config_base based on event encoding */
+	hwc->config_base = event->attr.config;
+
+	return 0;
+}
+
+int hisi_uncore_pmu_event_init(struct perf_event *event)
+{
+	int err;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	/* we do not support sampling as the counters are all
+	 * shared by all CPU cores in a CPU die(SCCL). Also we
+	 * donot support attach to a task(per-process mode)
+	 */
+	if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
+		return -EOPNOTSUPP;
+
+	/* counters do not have these bits */
+	if (event->attr.exclude_user	||
+	    event->attr.exclude_kernel	||
+	    event->attr.exclude_host	||
+	    event->attr.exclude_guest	||
+	    event->attr.exclude_hv	||
+	    event->attr.exclude_idle)
+		return -EINVAL;
+
+	if (event->cpu < 0)
+		return -EINVAL;
+
+	event->cpu = cpumask_first(&phisi_pmu->cpu);
+
+	err = __hw_perf_event_init(event);
+
+	return err;
+}
+
+/*
+ * Enable counter and set the counter to count
+ * the event that we're interested in.
+ */
+void hisi_uncore_pmu_enable_event(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+
+	/* Disable the hardware event counting */
+	if (phisi_pmu->ops->disable_counter)
+		phisi_pmu->ops->disable_counter(phisi_pmu, GET_CNTR_IDX(hwc));
+
+	/*
+	 * Set event (if destined for Hisilicon SoC counters).
+	 */
+	if (phisi_pmu->ops->set_evtype)
+		phisi_pmu->ops->set_evtype(phisi_pmu, GET_CNTR_IDX(hwc),
+							hwc->config_base);
+
+	/* Enable the hardware event counting */
+	if (phisi_pmu->ops->enable_counter)
+		phisi_pmu->ops->enable_counter(phisi_pmu, GET_CNTR_IDX(hwc));
+}
+
+void hisi_pmu_set_event_period(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+
+	/*
+	 * The Hisilicon PMU counters have a period of 2^32. 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.
+	 */
+	u64 val = 1ULL << 31;
+
+	local64_set(&hwc->prev_count, val);
+
+	/* Write to the hardware event counter */
+	phisi_pmu->ops->write_counter(phisi_pmu, hwc, val);
+}
+
+void hisi_uncore_pmu_start(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_pmu_hw_events *hw_events;
+
+	hw_events = &phisi_pmu->hw_events;
+
+	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
+		return;
+
+	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
+	hwc->state = 0;
+
+	if (phisi_pmu->ops->set_event_period)
+		phisi_pmu->ops->set_event_period(event);
+
+	if (flags & PERF_EF_RELOAD) {
+		u64 prev_raw_count =  local64_read(&hwc->prev_count);
+
+		phisi_pmu->ops->write_counter(phisi_pmu, hwc,
+						(u32)prev_raw_count);
+	}
+
+	hisi_uncore_pmu_enable_event(event);
+	perf_event_update_userpage(event);
+}
+
+void hisi_uncore_pmu_stop(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+
+	if (hwc->state & PERF_HES_UPTODATE)
+		return;
+
+	/*
+	 * We always reprogram the counter, so ignore PERF_EF_UPDATE.
+	 * See hisi_uncore_pmu_start()
+	 */
+	if (phisi_pmu->ops->disable_counter)
+		phisi_pmu->ops->disable_counter(phisi_pmu,
+						GET_CNTR_IDX(hwc));
+
+	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
+	hwc->state |= PERF_HES_STOPPED;
+	if (hwc->state & PERF_HES_UPTODATE)
+		return;
+
+	/* Read hardware counter and update the Perf counter statistics */
+	phisi_pmu->ops->event_update(event, hwc, GET_CNTR_IDX(hwc));
+	hwc->state |= PERF_HES_UPTODATE;
+}
+
+int hisi_uncore_pmu_add(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_pmu_hw_events *hw_events;
+	int idx;
+
+	hw_events = &phisi_pmu->hw_events;
+
+	hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
+
+	/* If we don't have a free counter then return early. */
+	idx = phisi_pmu->ops->get_event_idx(phisi_pmu);
+	if (idx < 0)
+		return -EAGAIN;
+
+	event->hw.idx = idx;
+	hw_events->events[idx] = event;
+
+	if (flags & PERF_EF_START)
+		hisi_uncore_pmu_start(event, PERF_EF_RELOAD);
+
+	/* Propagate our changes to the userspace mapping. */
+	perf_event_update_userpage(event);
+
+	return 0;
+}
+
+void hisi_uncore_pmu_del(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_pmu_hw_events *hw_events;
+
+	hw_events = &phisi_pmu->hw_events;
+
+	hisi_uncore_pmu_stop(event, PERF_EF_UPDATE);
+
+	phisi_pmu->ops->clear_event_idx(phisi_pmu, GET_CNTR_IDX(hwc));
+	perf_event_update_userpage(event);
+	hw_events->events[GET_CNTR_IDX(hwc)] = NULL;
+}
+
+struct hisi_pmu *hisi_pmu_alloc(struct device *dev)
+{
+	struct hisi_pmu *phisi_pmu;
+
+	phisi_pmu = devm_kzalloc(dev, sizeof(*phisi_pmu), GFP_KERNEL);
+	if (!phisi_pmu)
+		return ERR_PTR(-ENOMEM);
+
+	return phisi_pmu;
+}
+
+void hisi_uncore_pmu_read(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+
+	/* Read hardware counter and update the Perf counter statistics */
+	phisi_pmu->ops->event_update(event, hwc, GET_CNTR_IDX(hwc));
+}
+
+int hisi_uncore_common_fwprop_read(struct device *dev,
+					struct hisi_pmu *phisi_pmu)
+{
+	if (device_property_read_u32(dev, "num-events",
+					&phisi_pmu->num_events)) {
+		dev_err(dev, "Cant read num-events from DT!\n");
+		return -EINVAL;
+	}
+
+	if (device_property_read_u32(dev, "num-counters",
+				     &phisi_pmu->num_counters)) {
+		dev_err(dev, "Cant read num-counters from DT!\n");
+		return -EINVAL;
+	}
+
+	/* Find the SCL ID */
+	if (device_property_read_u32(dev, "scl-id",
+					&phisi_pmu->scl_id)) {
+		dev_err(dev, "Cant read scl-id!\n");
+		return -EINVAL;
+	}
+
+	if (phisi_pmu->scl_id == 0 ||
+		phisi_pmu->scl_id >= MAX_UNITS) {
+		dev_err(dev, "Invalid SCL=%d!\n",
+					phisi_pmu->scl_id);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+int hisi_uncore_pmu_setup(struct hisi_pmu *phisi_pmu,
+					const char *pmu_name)
+{
+	/* Register the events with perf */
+	return perf_pmu_register(&phisi_pmu->pmu, pmu_name, -1);
+}
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
new file mode 100644
index 0000000..b6b16df
--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -0,0 +1,108 @@
+/*
+ * HiSilicon SoC Hardware event counters support
+ *
+ * Copyright (C) 2016 Huawei Technologies Limited
+ * Author: Anurup M <anurup.m@huawei.com>
+ *
+ * This code is based on the uncore PMU's like arm-cci and
+ * arm-ccn.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/>.
+ */
+#ifndef __HISI_UNCORE_PMU_H__
+#define __HISI_UNCORE_PMU_H__
+
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/soc/hisilicon/djtag.h>
+#include <linux/types.h>
+#include <asm/local64.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt)     "hisi_pmu: " fmt
+
+#define HISI_DJTAG_MOD_MASK (0xFFFF)
+#define HISI_CNTR_SCCL_MASK    (0xF00)
+
+#define HISI_EVTYPE_EVENT	0xfff
+#define HISI_MAX_PERIOD ((1LLU << 32) - 1)
+
+#define MAX_BANKS 8
+#define MAX_COUNTERS 30
+#define MAX_UNITS 8
+
+#define GET_CNTR_IDX(hwc) (hwc->idx)
+#define to_hisi_pmu(c)	(container_of(c, struct hisi_pmu, pmu))
+
+#define GET_UNIT_IDX(event_code)		\
+	(((event_code & HISI_SCCL_MASK) >>	\
+			   HISI_SCCL_SHIFT) - 1)
+
+struct hisi_pmu;
+
+struct hisi_uncore_ops {
+	void (*set_evtype)(struct hisi_pmu *, int, u32);
+	void (*set_event_period)(struct perf_event *);
+	int (*get_event_idx)(struct hisi_pmu *);
+	void (*clear_event_idx)(struct hisi_pmu *, int);
+	u64 (*event_update)(struct perf_event *,
+			     struct hw_perf_event *, int);
+	u32 (*read_counter)(struct hisi_pmu *, int, int);
+	u32 (*write_counter)(struct hisi_pmu *,
+				struct hw_perf_event *, u32);
+	void (*enable_counter)(struct hisi_pmu *, int);
+	void (*disable_counter)(struct hisi_pmu *, int);
+};
+
+struct hisi_pmu_hw_events {
+	struct perf_event **events;
+	raw_spinlock_t pmu_lock;
+};
+
+/* Generic pmu struct for different pmu types */
+struct hisi_pmu {
+	const char *name;
+	struct hisi_pmu_hw_events hw_events;
+	struct hisi_uncore_ops *ops;
+	struct device *dev;
+	void *hwmod_data; /* Hardware module specific data */
+	cpumask_t cpu;
+	struct pmu pmu;
+	u32 scl_id;
+	int num_counters;
+	int num_events;
+	int num_units;
+};
+
+void hisi_uncore_pmu_read(struct perf_event *event);
+void hisi_uncore_pmu_del(struct perf_event *event, int flags);
+int hisi_uncore_pmu_add(struct perf_event *event, int flags);
+void hisi_uncore_pmu_start(struct perf_event *event, int flags);
+void hisi_uncore_pmu_stop(struct perf_event *event, int flags);
+void hisi_pmu_set_event_period(struct perf_event *event);
+void hisi_uncore_pmu_enable_event(struct perf_event *event);
+int hisi_uncore_pmu_setup(struct hisi_pmu *phisi_pmu, const char *pmu_name);
+int hisi_uncore_pmu_event_init(struct perf_event *event);
+int hisi_djtag_readreg(int module_id, int bank, u32 offset,
+				struct hisi_djtag_client *client,
+							u32 *pvalue);
+int hisi_djtag_writereg(int module_id, int bank,
+				u32 offset, u32 value,
+				struct hisi_djtag_client *client);
+struct hisi_pmu *hisi_pmu_alloc(struct device *dev);
+int hisi_uncore_common_fwprop_read(struct device *dev,
+					struct hisi_pmu *phisi_pmu);
+#endif /* __HISI_UNCORE_PMU_H__ */
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1513935 — [PATCH v1 05/11] dt-bindings: perf: hisi: Add Devicetree bindings for Hisilicon SoC PMU

	1) Device tree bindings for Hisilicon SoC PMU.
	2) Add example for Hisilicon L3 cache, MN and DDRC PMU.

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 .../devicetree/bindings/arm/hisilicon/pmu.txt      | 127 +++++++++++++++++++++
 1 file changed, 127 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/pmu.txt

diff --git a/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
new file mode 100644
index 0000000..e7b35e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
@@ -0,0 +1,127 @@
+Hisilicon SoC hip05/06/07 ARMv8 PMU
+===================================
+
+The Hisilicon SoC chips like hip05/06/07 etc. consist of varous independent
+system device PMU's such as L3 cache (L3C), Miscellaneous Nodes(MN) and DDR
+comtroller. These PMU devices are independent and have hardware logic to
+gather statistics and performance information.
+
+HiSilicon SoC chip is encapsulated by multiple CPU and IO die's. The CPU die
+is called as Super CPU cluster (SCCL) which includes 16 cpu-cores. Every SCCL
+is further grouped as CPU clusters (CCL) which includes 4 cpu-cores each.
+e.g. In the case of hip05/06/07, each SCCL has 1 L3 cache and 1 MN PMU device.
+
+The Hisilicon SoC PMU DT node bindigs for uncore PMU devices are as below.
+For PMU devices like L3 cache. MN etc. which are accessed using the djtag,
+the parent node will be the djtag node of the corresponding CPU die(SCCL).
+
+For uncore PMU devices there are some common required properties as detailed
+below.
+
+Required properties:
+	- compatible : This field contain two values. The first value is
+		always "hisilicon" and second value is the Module type as shown
+		in below examples:
+		(a) "hisilicon,hisi-pmu-l3c-v1" for Hisilicon SoC L3C PMU
+			device (Version 1)
+		(b) "hisilicon,hisi-pmu-mn-v1" for Hisilicon SoC MN PMU
+			device (Version 1)
+		(c) "hisilicon,hisi-pmu-ddrc-v1" for Hisilicon SoC DDRC PMU
+			device (Version 1)
+		The hip05/06/07 chips have v1 hardware for L3C, MN and DDRC.
+
+	- scl-id : The Super Cluster ID. This can be the ID of the CPU die
+		   or IO die in the chip.
+
+	- num-events : No of events supported by this PMU device.
+
+	- num-counters : No of hardware counters available for counting.
+
+L3 cache
+--------
+The L3 cache is dedicated for each SCCL and hence there are separate DT nodes
+for L3 cache for each SCCL. For L3 cache PMU the additional required properties
+are
+	- counter-reg : Counter register offset.
+
+	- evtype-reg : Event select register offset.
+
+	- evctrl-reg : Event counting control(LAUCTRL) register offset.
+
+	- event-en : Event enable value.
+
+	- module-id : Module ID to input for djtag. This property is an array of
+		      module_id for each L3 cache banks.
+
+	- num-banks : Number of banks or instances of the device.
+
+	- cfgen-map : Config enable array to select the bank.
+
+Miscellaneous Node
+-------------------
+The MN is dedicated for each SCCL and hence there are separate DT nodes for MN
+for each SCCL. For MN PMU the additional required properties are
+	- counter-reg : Counter register offset.
+
+	- evtype-reg : Event select register offset.
+
+	- evctrl-reg : Event counting control register offset.
+
+	- module-id : Module ID to input for djtag. As MN doesnot have multiple banks
+		      this property is a single value.
+
+	- cfgen-map : Config enable to select the bank. For MN it is a single value
+
+	- event-en : Event enable value.
+
+Example:
+
+	djtag0: djtag@0 {
+		compatible = "hisilicon,hip05-cpu-djtag-v1";
+		pmul3c0 {
+			compatible = "hisilicon,hisi-pmu-l3c-v1";
+			scl-id = <0x02>;
+			num-events = <0x16>;
+			num-counters = <0x08>;
+			module-id = <0x04 0x04 0x04 0x04>;
+			num-banks = <0x04>;
+			cfgen-map = <0x02 0x04 0x01 0x08>;
+			counter-reg = <0x170>;
+			evctrl-reg = <0x04>;
+			event-en = <0x1000000>;
+			evtype-reg = <0x140>;
+		};
+
+		pmumn0 {
+			compatible = "hisilicon,hisi-pmu-mn-v1";
+			scl-id = <0x02>;
+			num-events = <0x09>;
+			num-counters = <0x04>;
+			module-id = <0x0b>;
+			cfgen-map = <0x01>;
+			counter-reg = <0x30>;
+			evctrl-reg = <0x40>;
+			event-en = <0x01>;
+			evtype-reg = <0x48>;
+		};
+	};
+
+DDR controller
+--------------
+Each SCCL in Hip05/06/07 chips have 2 DDR channels and hence 2 DDR controllers.
+There are separate DT nodes for each DDR channel.
+For DDRC PMU the additional required properties are
+
+	- ch-id : DDRC Channel ID.
+	- reg : Register base address and range for the DDRC channel.
+
+Example:
+	/* DDRC for CPU die scl #2 Channel #1 for hip05 */
+	pmu_sccl0_ddrc1: pmu_ddrc1@80358000 {
+		 compatible = "hisilicon,hisi-pmu-ddrc-v1";
+		 scl-id = <0x02>;
+		 ch-id = <0x1>;
+		 num-events = <0x0D>;
+		 num-counters = <0x04>;
+		 reg = <0x80358000 0x10000>; /* TOTEMC DDRC1 */
+	 };
-- 
2.1.4

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web