Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6fe2be1b-faaf-86cd-8fcc-a919d30d65c6@linaro.org>
Date:   Tue, 27 Sep 2022 11:28:32 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Reply-To: neil.armstrong@linaro.org
Subject: Re: [PATCH v8 1/4] perf/amlogic: Add support for Amlogic meson G12
 SoC DDR PMU driver
Content-Language: en-US
To:     Jiucheng Xu <jiucheng.xu@amlogic.com>,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-amlogic@lists.infradead.org, devicetree@vger.kernel.org
Cc:     Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Will Deacon <will@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Kevin Hilman <khilman@baylibre.com>,
        Jerome Brunet <jbrunet@baylibre.com>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        Chris Healy <cphealy@gmail.com>,
        Jianxin Pan <jianxin.pan@amlogic.com>,
        Kelvin Zhang <kelvin.zhang@amlogic.com>
References: <20220916020320.2455580-1-jiucheng.xu@amlogic.com>
From:   Neil Armstrong <neil.armstrong@linaro.org>
Organization: Linaro Developer Services
In-Reply-To: <20220916020320.2455580-1-jiucheng.xu@amlogic.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Will, Mark,

On 16/09/2022 04:03, Jiucheng Xu wrote:
> This patch adds support Amlogic meson G12 series SoC
> DDR bandwidth PMU driver framework and interfaces.
> 
> The PMU not only can monitor the total DDR bandwidth,
> but also the bandwidth which is from individual IP module.
> 
> Example usage:
> 
>   $ perf stat -a -e meson_ddr_bw/total_rw_bytes/ -I 1000 sleep 10
> 
> - or -
> 
>   $ perf stat -a -e \
>     meson_ddr_bw/total_rw_bytes/,\
>     meson_ddr_bw/chan_1_rw_bytes,arm=1/ -I 1000 \
>     sleep 10
> 
> g12 SoC support 4 channels to monitor DDR bandwidth
> simultaneously. Each channel can monitor up to 4 IP modules
> simultaneously.
> 
> For Instance, If you want to get the sum of DDR bandwidth
> from CPU, GPU, USB3.0 and VDEC. You can use the following
> command parameters to display.
> 
>   $ perf stat -a -e \
>     meson_ddr_bw/chan_2_rw_bytes,arm=1,gpu=1,usb3_0=1,nna=1/ -I 1000 \
>     sleep 10
> 
> Other events are supported, and advertised via perf list.
> 
> Signed-off-by: Jiucheng Xu <jiucheng.xu@amlogic.com>
> ---
> Changes v7 -> v8:
>    - Add linux-amlogic@lists.infradead.org maillist for MAINTAINERS
>    - Change driver name from "amlogic,g12-ddr-pmu" to "meson-g12-ddr-pmu"
> 
> Changes v6 -> v7:
>    - Drop the Reported-by tag
> 
> Changes v5 -> v6:
>    - Add const for driver data
> 
> Changes v4 -> v5:
>    - Remove error message
>    - Use smp_processor_id() instead of raw_smp_processor_id()
>    - Remove EXPORT_SYMBOL()
>    - Use variant specific driver data for compatible
>    - Use module_platform_driver for modules_init/exit
>    - Change location of driver structures from .h  to .c
> 
> Changes v3 -> v4:
>    - No change
> 
> Changes v2 -> v3:
>    - Fix sh GCC 12.1.0 compiling warning
>    - Rename prefix aml to meson for files and code
> 
> Changes v1 -> v2:
>    - Remove inline to let GCC make the decisions
>    - Remove spinlock
>    - Remove ddr_cnt_accumulate()
>    - Remove the message which only indicate a bug
>    - Remove all dev_warn() message
>    - Use hweight64() helper instead of whole loop
>    - Remove setting of hwc
>    - Use for_each_set_bit() helper for bit loop
>    - Use sysfs_emit() in sysfs show
>    - Remove checking for bugs
>    - Replace irq_set_affinity_hint() to irq_set_affinity()
>    - Remove #ifdef CONFIG_OF
>    - Use devm_platform_ioremap_resource() instead of
>      platform_get_resource()&ioremap()
>    - Use platform_get_irq() instead of platform_get_resource()&ioremap()
>    - Replace IRQF_SHARED to IRQF_NOBALANCING
>    - Remove meaningless log like "init ok"
>    - Use compatible instead of creating new property to distinguish
>      different platform.
>    - Use the is_visible callback to avoid exposing unsupported fmt_attr
>    - Use module_platform_driver_probe() instead of module_init/exit

<snip>

Gentle ping, it seems all previous review comments were addressed, could you do another review round on this v8 ?

Thanks,
Neil