* [PATCH v2 2/2] perf cs-etm: Remove redundant space
From: Leo Yan @ 2018-05-09 4:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525839312-20544-1-git-send-email-leo.yan@linaro.org>
There have two spaces ahead function name cs_etm__set_pid_tid_cpu(), so
remove one space and correct indentation.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
tools/perf/util/cs-etm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 69f21de..822ba91 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -611,8 +611,8 @@ cs_etm__get_trace(struct cs_etm_buffer *buff, struct cs_etm_queue *etmq)
return buff->len;
}
-static void cs_etm__set_pid_tid_cpu(struct cs_etm_auxtrace *etm,
- struct auxtrace_queue *queue)
+static void cs_etm__set_pid_tid_cpu(struct cs_etm_auxtrace *etm,
+ struct auxtrace_queue *queue)
{
struct cs_etm_queue *etmq = queue->priv;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/2] perf cs-etm: Support unknown_thread in cs_etm_auxtrace
From: Leo Yan @ 2018-05-09 4:15 UTC (permalink / raw)
To: linux-arm-kernel
CoreSight doesn't allocate thread structure for unknown_thread in etm
auxtrace, so unknown_thread is NULL pointer. If the perf data doesn't
contain valid tid and then cs_etm__mem_access() uses unknown_thread
instead as thread handler, this results in segmentation fault when
thread__find_addr_map() accesses thread handler.
This commit creates new thread data which is used by unknown_thread, so
CoreSight tracing can roll back to use unknown_thread if perf data
doesn't include valid thread info. This commit also releases thread
data for initialization failure case and for normal auxtrace free flow.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
tools/perf/util/cs-etm.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 6533b1a..69f21de 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -239,6 +239,7 @@ static void cs_etm__free(struct perf_session *session)
for (i = 0; i < aux->num_cpu; i++)
zfree(&aux->metadata[i]);
+ thread__zput(aux->unknown_thread);
zfree(&aux->metadata);
zfree(&aux);
}
@@ -1355,6 +1356,23 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
etm->auxtrace.free = cs_etm__free;
session->auxtrace = &etm->auxtrace;
+ etm->unknown_thread = thread__new(999999999, 999999999);
+ if (!etm->unknown_thread)
+ goto err_free_queues;
+
+ /*
+ * Initialize list node so that at thread__zput() we can avoid
+ * segmentation fault at list_del_init().
+ */
+ INIT_LIST_HEAD(&etm->unknown_thread->node);
+
+ err = thread__set_comm(etm->unknown_thread, "unknown", 0);
+ if (err)
+ goto err_delete_thread;
+
+ if (thread__init_map_groups(etm->unknown_thread, etm->machine))
+ goto err_delete_thread;
+
if (dump_trace) {
cs_etm__print_auxtrace_info(auxtrace_info->priv, num_cpu);
return 0;
@@ -1369,16 +1387,18 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
err = cs_etm__synth_events(etm, session);
if (err)
- goto err_free_queues;
+ goto err_delete_thread;
err = auxtrace_queues__process_index(&etm->queues, session);
if (err)
- goto err_free_queues;
+ goto err_delete_thread;
etm->data_queued = etm->queues.populated;
return 0;
+err_delete_thread:
+ thread__zput(etm->unknown_thread);
err_free_queues:
auxtrace_queues__free(&etm->queues);
session->auxtrace = NULL;
--
2.7.4
^ permalink raw reply related
* [PATCH] hwmon: (aspeed-pwm-tacho) Use 24MHz clock
From: Benjamin Herrenschmidt @ 2018-05-09 4:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAARXrt=ZA7sz_CvNUxHRvie3YfErU-5d5ewHry=mgxb6Uz-+TA@mail.gmail.com>
On Wed, 2018-05-09 at 11:50 +0800, Lei YU wrote:
> On Wed, May 9, 2018 at 11:43 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > I am not going to accept this change, period. This is not one, it is five
> > steps
> > backward. If "aspeed_set_clock_source(priv->regmap, 0)" changes the clock
> > speed,
> > or the clock source, read it later, and attach to the correct clock. If that
> > doesn't work, fix the problem in the clock subsystem. Hacking the driver is
> > just
> > plain wrong.
> >
> > Also, if the idea in DT is to provide a different clock to the watchdog on
> > purpose,
> > maybe the call to "aspeed_set_clock_source(priv->regmap, 0)" is wrong.
>
> Exactly!
> My first change (not sent to mailing list) is to remove the call of
> "aspeed_set_clock_source(priv->regmap, 0)", instead, checking the clock
> source, and use either 24M or the memory controller clock.
> But that make things a bit more complicated and Aspeed suggests to use 24M
> clock. So I sent this simplified change.
>
> It's OK to not accept this change, the fix in DT will fix the issue as well.
Another option is to give *both* clock sources in the DT, since in HW
they both eventually reach the IP block, and have the driver select
which one it wants to use (that itself can also be in the DT).
That way you can select 0 if you want and still use clk_get_rate() to
know how fast it's ticking.
Cheers,
Ben.
^ permalink raw reply
* [PATCH 5/5] hisi: Consolidate the Kconfigs for the CLOCK_STUB and the MAILBOX
From: Leo Yan @ 2018-05-09 4:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CABb+yY2ke__W2ZW4c_Z9CejZzup52ykRDyNecTs0+nkvf5cbNA@mail.gmail.com>
On Wed, May 09, 2018 at 09:19:13AM +0530, Jassi Brar wrote:
> On Wed, Apr 4, 2018 at 8:44 AM, Leo Yan <leo.yan@linaro.org> wrote:
> > From: Daniel Lezcano <daniel.lezcano@linaro.org>
> >
> > The current defconfig is inconsistent as it selects the mailbox and
> > the clock for the hi6220 and the hi3660 without having their Kconfigs
> > making sure the dependencies are correct. It ends up when selecting
> > different versions for the kernel (for example when git bisecting)
> > those options disappear and they don't get back, leading to unexpected
> > behaviors. In our case, the cpufreq driver does no longer work because
> > the clock fails to initialize due to the clock stub and the mailbox
> > missing.
> >
> > In order to have the dependencies correctly set when defaulting, let's
> > do the same as commit 3a49afb84ca074e ("clk: enable hi655x common clk
> > automatically") where we select automatically the driver when the
> > parent driver is selected. With sensible defaults in place, we can leave
> > other choices for EXPERT.
> >
> Acked-by: Jassi Brar <jaswinder.singh@linaro.org>
Thanks Jassi & Stephen.
Wei, could you pick this patch?
Thanks,
Leo Yan
^ permalink raw reply
* [PATCH] hwmon: (aspeed-pwm-tacho) Use 24MHz clock
From: Lei YU @ 2018-05-09 3:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c5770e4a-9445-d4ef-cc1f-a7a84599c4a1@roeck-us.net>
On Wed, May 9, 2018 at 11:43 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>
> I am not going to accept this change, period. This is not one, it is five
> steps
> backward. If "aspeed_set_clock_source(priv->regmap, 0)" changes the clock
> speed,
> or the clock source, read it later, and attach to the correct clock. If that
> doesn't work, fix the problem in the clock subsystem. Hacking the driver is
> just
> plain wrong.
>
> Also, if the idea in DT is to provide a different clock to the watchdog on
> purpose,
> maybe the call to "aspeed_set_clock_source(priv->regmap, 0)" is wrong.
Exactly!
My first change (not sent to mailing list) is to remove the call of
"aspeed_set_clock_source(priv->regmap, 0)", instead, checking the clock
source, and use either 24M or the memory controller clock.
But that make things a bit more complicated and Aspeed suggests to use 24M
clock. So I sent this simplified change.
It's OK to not accept this change, the fix in DT will fix the issue as well.
Please drop this.
Thanks!
^ permalink raw reply
* [PATCH 5/5] hisi: Consolidate the Kconfigs for the CLOCK_STUB and the MAILBOX
From: Jassi Brar @ 2018-05-09 3:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1522811675-12741-6-git-send-email-leo.yan@linaro.org>
On Wed, Apr 4, 2018 at 8:44 AM, Leo Yan <leo.yan@linaro.org> wrote:
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> The current defconfig is inconsistent as it selects the mailbox and
> the clock for the hi6220 and the hi3660 without having their Kconfigs
> making sure the dependencies are correct. It ends up when selecting
> different versions for the kernel (for example when git bisecting)
> those options disappear and they don't get back, leading to unexpected
> behaviors. In our case, the cpufreq driver does no longer work because
> the clock fails to initialize due to the clock stub and the mailbox
> missing.
>
> In order to have the dependencies correctly set when defaulting, let's
> do the same as commit 3a49afb84ca074e ("clk: enable hi655x common clk
> automatically") where we select automatically the driver when the
> parent driver is selected. With sensible defaults in place, we can leave
> other choices for EXPERT.
>
Acked-by: Jassi Brar <jaswinder.singh@linaro.org>
^ permalink raw reply
* [PATCH] hwmon: (aspeed-pwm-tacho) Use 24MHz clock
From: Guenter Roeck @ 2018-05-09 3:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAARXrtmO=wsPK6bfO0E7fvAkqEJUV1kQSYK1gWXkUhmKytq42A@mail.gmail.com>
On 05/08/2018 08:18 PM, Lei YU wrote:
> On Tue, May 8, 2018 at 9:51 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> No mixed C/C++ comments in hwmon drivers.
>>
>>> aspeed_set_clock_source(priv->regmap, 0);
>>> + priv->clk_freq = 24000000;
>>>
>>
>>
>> Are you saying that clk_get_rate() is wrong ? Anyway, if the DT is bad, it
>> should be fixed.
> Nope, clk_get_rate() is OK.
> The reason I make this change is because the PWM supports two types of clock
> source, the 24MHz or the clock from memory controller.
> If the DT uses 24MHz clock, this code is OK.
> But if the DT configs this pwm to use mclk (memory controller clk), this piece
> of code becomes wrong, because the code
> `aspeed_set_clock_source(priv->regmap, 0)` configs the device to use the 24MHz
> clock.
> So no matter what DT configs the clk, this driver *always* uses 24MHz clock.
>
> That's why I want to make this change.
>
>> I am not a friend of hacking drivers to fix up bad DTs, and much less so
>> without explanation.
>> Plus, how do we know that the next chip supported by the driver doesn't have
>> a 32MHz clock ?
> This driver currently supports ast2400 and ast2500, and they both use 24MHz
> clock.
> In case future device uses a different clock, we can update this code, right?
>
I am not going to accept this change, period. This is not one, it is five steps
backward. If "aspeed_set_clock_source(priv->regmap, 0)" changes the clock speed,
or the clock source, read it later, and attach to the correct clock. If that
doesn't work, fix the problem in the clock subsystem. Hacking the driver is just
plain wrong.
Also, if the idea in DT is to provide a different clock to the watchdog on purpose,
maybe the call to "aspeed_set_clock_source(priv->regmap, 0)" is wrong.
Guenter
>> Really, please fix the DT.
> Sure, I will send patch to config the clock to use fixed 24MHz clock as well.
>
>>
>> Guenter
>>
>>> aspeed_create_type(priv);
>>>
>>
>>
>
^ permalink raw reply
* [PATCH] Kirin-PCIe: Add kirin pcie msi feature.
From: Yao Chen @ 2018-05-09 3:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAH7ZN-zOwCqzzmKxkCeLt+z2FCpMrxnu5a8s8Os-fSsuS2R01w@mail.gmail.com>
On 2018/5/9 5:31, Dmitry Shmidt wrote:
> On Tue, May 8, 2018 at 12:03 AM, Yao Chen <chenyao11@huawei.com> wrote:
>> This patch adds kirin pcie msi feature.
>>
>> Signed-off-by: Yao Chen <chenyao11@huawei.com>
>> ---
>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 ++
>> drivers/pci/dwc/pcie-kirin.c | 38 +++++++++++++++++++++++++++++++
>> 2 files changed, 40 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> index ec3eb8e..4ef684f 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> @@ -872,6 +872,8 @@
>> 0x0 0x02000000>;
>> num-lanes = <1>;
>> #interrupt-cells = <1>;
>> + interrupts = <0 283 4>;
>> + interrupts-names = "msi";
>
> Possible typo? Should be interrupt-names = "msi";
Yes, you're right. It should be interrupt-names = "msi". I will fix it. Thanks.
>
>> interrupt-map-mask = <0xf800 0 0 7>;
>> interrupt-map = <0x0 0 0 1
>> &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
>> diff --git a/drivers/pci/dwc/pcie-kirin.c b/drivers/pci/dwc/pcie-kirin.c
>> index d2970a0..2319c9c 100644
>> --- a/drivers/pci/dwc/pcie-kirin.c
>> +++ b/drivers/pci/dwc/pcie-kirin.c
>> @@ -426,9 +426,28 @@ static int kirin_pcie_establish_link(struct pcie_port *pp)
>> return 0;
>> }
>>
>> +static irqreturn_t kirin_pcie_msi_irq_handler(int irq, void *arg)
>> +{
>> + struct pcie_port *pp = arg;
>> +
>> + return dw_handle_msi_irq(pp);
>> +}
>> +
>> +static void kirin_pcie_msi_init(struct pcie_port *pp)
>> +{
>> + dw_pcie_msi_init(pp);
>> +}
>> +
>> +static void kirin_pcie_enable_interrupts(struct pcie_port *pp)
>> +{
>> + if (IS_ENABLED(CONFIG_PCI_MSI))
>> + kirin_pcie_msi_init(pp);
>> +}
>> +
>> static int kirin_pcie_host_init(struct pcie_port *pp)
>> {
>> kirin_pcie_establish_link(pp);
>> + kirin_pcie_enable_interrupts(pp);
>>
>> return 0;
>> }
>> @@ -448,6 +467,25 @@ static int kirin_pcie_host_init(struct pcie_port *pp)
>> static int __init kirin_add_pcie_port(struct dw_pcie *pci,
>> struct platform_device *pdev)
>> {
>> + int ret;
>> +
>> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> + pci->pp.msi_irq = platform_get_irq(pdev, 0);
>> + if (!pci->pp.msi_irq) {
>> + dev_err(&pdev->dev, "failed to get msi irq\n");
>> + return -ENODEV;
>> + }
>> + ret = devm_request_irq(&pdev->dev, pci->pp.msi_irq,
>> + kirin_pcie_msi_irq_handler,
>> + IRQF_SHARED | IRQF_NO_THREAD,
>> + "kirin_pcie_msi", &pci->pp);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to request msi irq\n");
>> + return ret;
>> + }
>> + }
>> +
>> + pci->pp.root_bus_nr = -1;
>> pci->pp.ops = &kirin_pcie_host_ops;
>>
>> return dw_pcie_host_init(&pci->pp);
>> --
>> 1.9.1
>>
>
> .
>
^ permalink raw reply
* [PATCH 2/4] phy: rockchip-typec: support variable phy config value
From: hl @ 2018-05-09 3:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFqH_511VgKG+_rqPNGf5Ez2+tc_ijqNQA_rYW1xr6903BSSoA@mail.gmail.com>
On Monday, May 07, 2018 09:59 PM, Enric Balletbo Serra wrote:
> Hi Lin,
>
> Thanks for the patch, apart from the new build warnings introduced
> some more comments below.
>
> 2018-05-04 10:08 GMT+02:00 Lin Huang <hl@rock-chips.com>:
>> the phy config values used to fix in dp firmware, but some boards
>> need change these values to do training and get the better eye diagram
>> result. So support that in phy driver.
>>
>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>> Signed-off-by: Lin Huang <hl@rock-chips.com>
>> ---
>> drivers/phy/rockchip/phy-rockchip-typec.c | 286 +++++++++++++++++++-----------
>> include/soc/rockchip/rockchip_phy_typec.h | 72 ++++++++
>> 2 files changed, 259 insertions(+), 99 deletions(-)
>> create mode 100644 include/soc/rockchip/rockchip_phy_typec.h
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
>> index 76a4b58..831a93b 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
>> @@ -63,6 +63,7 @@
>>
>> #include <linux/mfd/syscon.h>
>> #include <linux/phy/phy.h>
>> +#include <soc/rockchip/rockchip_phy_typec.h>
>>
>> #define CMN_SSM_BANDGAP (0x21 << 2)
>> #define CMN_SSM_BIAS (0x22 << 2)
>> @@ -323,23 +324,31 @@
>> * clock 0: PLL 0 div 1
>> * clock 1: PLL 1 div 2
>> */
>> -#define CLK_PLL_CONFIG 0X30
>> +#define CLK_PLL1_DIV1 0x20
>> +#define CLK_PLL1_DIV2 0x30
>> #define CLK_PLL_MASK 0x33
>>
>> #define CMN_READY BIT(0)
>>
>> +#define DP_PLL_CLOCK_ENABLE_ACK BIT(3)
>> #define DP_PLL_CLOCK_ENABLE BIT(2)
>> +#define DP_PLL_ENABLE_ACK BIT(1)
>> #define DP_PLL_ENABLE BIT(0)
>> #define DP_PLL_DATA_RATE_RBR ((2 << 12) | (4 << 8))
>> #define DP_PLL_DATA_RATE_HBR ((2 << 12) | (4 << 8))
>> #define DP_PLL_DATA_RATE_HBR2 ((1 << 12) | (2 << 8))
>> +#define DP_PLL_DATA_RATE_MASK 0xff00
>>
>> -#define DP_MODE_A0 BIT(4)
>> -#define DP_MODE_A2 BIT(6)
>> -#define DP_MODE_ENTER_A0 0xc101
>> -#define DP_MODE_ENTER_A2 0xc104
>> +#define DP_MODE_MASK 0xf
>> +#define DP_MODE_ENTER_A0 BIT(0)
>> +#define DP_MODE_ENTER_A2 BIT(2)
>> +#define DP_MODE_ENTER_A3 BIT(3)
>> +#define DP_MODE_A0_ACK BIT(4)
>> +#define DP_MODE_A2_ACK BIT(6)
>> +#define DP_MODE_A3_ACK BIT(7)
>> +#define DP_LINK_RESET_DEASSERTED BIT(8)
>>
>> -#define PHY_MODE_SET_TIMEOUT 100000
>> +#define PHY_MODE_SET_TIMEOUT 1000000
>>
> Why do you need to increase this timeout? Is because the software link
> training timed out using the old value?
That for debug, will fix it in next version.
>
>
>> #define PIN_ASSIGN_C_E 0x51d9
>> #define PIN_ASSIGN_D_F 0x5100
>> @@ -349,51 +358,7 @@
>> #define MODE_DFP_USB BIT(1)
>> #define MODE_DFP_DP BIT(2)
>>
>> -struct usb3phy_reg {
>> - u32 offset;
>> - u32 enable_bit;
>> - u32 write_enable;
>> -};
>> -
>> -/**
>> - * struct rockchip_usb3phy_port_cfg: usb3-phy port configuration.
>> - * @reg: the base address for usb3-phy config.
>> - * @typec_conn_dir: the register of type-c connector direction.
>> - * @usb3tousb2_en: the register of type-c force usb2 to usb2 enable.
>> - * @external_psm: the register of type-c phy external psm clock.
>> - * @pipe_status: the register of type-c phy pipe status.
>> - * @usb3_host_disable: the register of type-c usb3 host disable.
>> - * @usb3_host_port: the register of type-c usb3 host port.
>> - * @uphy_dp_sel: the register of type-c phy DP select control.
>> - */
>> -struct rockchip_usb3phy_port_cfg {
>> - unsigned int reg;
>> - struct usb3phy_reg typec_conn_dir;
>> - struct usb3phy_reg usb3tousb2_en;
>> - struct usb3phy_reg external_psm;
>> - struct usb3phy_reg pipe_status;
>> - struct usb3phy_reg usb3_host_disable;
>> - struct usb3phy_reg usb3_host_port;
>> - struct usb3phy_reg uphy_dp_sel;
>> -};
>> -
>> -struct rockchip_typec_phy {
>> - struct device *dev;
>> - void __iomem *base;
>> - struct extcon_dev *extcon;
>> - struct regmap *grf_regs;
>> - struct clk *clk_core;
>> - struct clk *clk_ref;
>> - struct reset_control *uphy_rst;
>> - struct reset_control *pipe_rst;
>> - struct reset_control *tcphy_rst;
>> - const struct rockchip_usb3phy_port_cfg *port_cfgs;
>> - /* mutex to protect access to individual PHYs */
>> - struct mutex lock;
>> -
>> - bool flip;
>> - u8 mode;
>> -};
>> +#define DEFAULT_RATE 162000
>>
> DEFAULT_RATE seems a very common name for me, maybe add a prefix?
Okay, will fix it.
>
>
>> struct phy_reg {
>> u16 value;
>> @@ -417,15 +382,15 @@ struct phy_reg usb3_pll_cfg[] = {
>> { 0x8, CMN_DIAG_PLL0_LF_PROG },
>> };
>>
>> -struct phy_reg dp_pll_cfg[] = {
>> +struct phy_reg dp_pll_rbr_cfg[] = {
>> { 0xf0, CMN_PLL1_VCOCAL_INIT },
>> { 0x18, CMN_PLL1_VCOCAL_ITER },
>> { 0x30b9, CMN_PLL1_VCOCAL_START },
>> - { 0x21c, CMN_PLL1_INTDIV },
>> + { 0x87, CMN_PLL1_INTDIV },
>> { 0, CMN_PLL1_FRACDIV },
>> - { 0x5, CMN_PLL1_HIGH_THR },
>> - { 0x35, CMN_PLL1_SS_CTRL1 },
>> - { 0x7f1e, CMN_PLL1_SS_CTRL2 },
>> + { 0x22, CMN_PLL1_HIGH_THR },
>> + { 0x8000, CMN_PLL1_SS_CTRL1 },
>> + { 0, CMN_PLL1_SS_CTRL2 },
>> { 0x20, CMN_PLL1_DSM_DIAG },
>> { 0, CMN_PLLSM1_USER_DEF_CTRL },
>> { 0, CMN_DIAG_PLL1_OVRD },
>> @@ -436,9 +401,52 @@ struct phy_reg dp_pll_cfg[] = {
>> { 0x8, CMN_DIAG_PLL1_LF_PROG },
>> { 0x100, CMN_DIAG_PLL1_PTATIS_TUNE1 },
>> { 0x7, CMN_DIAG_PLL1_PTATIS_TUNE2 },
>> - { 0x4, CMN_DIAG_PLL1_INCLK_CTRL },
>> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
>> +};
>> +
>> +struct phy_reg dp_pll_hbr_cfg[] = {
>> + { 0xf0, CMN_PLL1_VCOCAL_INIT },
>> + { 0x18, CMN_PLL1_VCOCAL_ITER },
>> + { 0x30b4, CMN_PLL1_VCOCAL_START },
>> + { 0xe1, CMN_PLL1_INTDIV },
>> + { 0, CMN_PLL1_FRACDIV },
>> + { 0x5, CMN_PLL1_HIGH_THR },
>> + { 0x8000, CMN_PLL1_SS_CTRL1 },
>> + { 0, CMN_PLL1_SS_CTRL2 },
>> + { 0x20, CMN_PLL1_DSM_DIAG },
>> + { 0x1000, CMN_PLLSM1_USER_DEF_CTRL },
>> + { 0, CMN_DIAG_PLL1_OVRD },
>> + { 0, CMN_DIAG_PLL1_FBH_OVRD },
>> + { 0, CMN_DIAG_PLL1_FBL_OVRD },
>> + { 0x7, CMN_DIAG_PLL1_V2I_TUNE },
>> + { 0x45, CMN_DIAG_PLL1_CP_TUNE },
>> + { 0x8, CMN_DIAG_PLL1_LF_PROG },
>> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE1 },
>> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE2 },
>> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
>> };
>>
>> +struct phy_reg dp_pll_hbr2_cfg[] = {
>> + { 0xf0, CMN_PLL1_VCOCAL_INIT },
>> + { 0x18, CMN_PLL1_VCOCAL_ITER },
>> + { 0x30b4, CMN_PLL1_VCOCAL_START },
>> + { 0xe1, CMN_PLL1_INTDIV },
>> + { 0, CMN_PLL1_FRACDIV },
>> + { 0x5, CMN_PLL1_HIGH_THR },
>> + { 0x8000, CMN_PLL1_SS_CTRL1 },
>> + { 0, CMN_PLL1_SS_CTRL2 },
>> + { 0x20, CMN_PLL1_DSM_DIAG },
>> + { 0x1000, CMN_PLLSM1_USER_DEF_CTRL },
>> + { 0, CMN_DIAG_PLL1_OVRD },
>> + { 0, CMN_DIAG_PLL1_FBH_OVRD },
>> + { 0, CMN_DIAG_PLL1_FBL_OVRD },
>> + { 0x7, CMN_DIAG_PLL1_V2I_TUNE },
>> + { 0x45, CMN_DIAG_PLL1_CP_TUNE },
>> + { 0x8, CMN_DIAG_PLL1_LF_PROG },
>> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE1 },
>> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE2 },
>> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
>> +};
>> static const struct rockchip_usb3phy_port_cfg rk3399_usb3phy_port_cfgs[] = {
>> {
>> .reg = 0xff7c0000,
>> @@ -484,7 +492,7 @@ static void tcphy_cfg_24m(struct rockchip_typec_phy *tcphy)
>>
>> rdata = readl(tcphy->base + CMN_DIAG_HSCLK_SEL);
>> rdata &= ~CLK_PLL_MASK;
>> - rdata |= CLK_PLL_CONFIG;
>> + rdata |= CLK_PLL1_DIV2;
>> writel(rdata, tcphy->base + CMN_DIAG_HSCLK_SEL);
>> }
>>
>> @@ -498,17 +506,44 @@ static void tcphy_cfg_usb3_pll(struct rockchip_typec_phy *tcphy)
>> tcphy->base + usb3_pll_cfg[i].addr);
>> }
>>
>> -static void tcphy_cfg_dp_pll(struct rockchip_typec_phy *tcphy)
>> +static void tcphy_cfg_dp_pll(struct rockchip_typec_phy *tcphy, int link_rate)
>> {
>> - u32 i;
>> + struct phy_reg *phy_cfg;
>> + u32 clk_ctrl;
>> + u32 i, cfg_size, hsclk_sel;
>> +
>> + hsclk_sel = readl(tcphy->base + CMN_DIAG_HSCLK_SEL);
>> + hsclk_sel &= ~CLK_PLL_MASK;
>> +
>> + switch (link_rate) {
>> + case 162000:
>> + clk_ctrl = DP_PLL_DATA_RATE_RBR;
>> + hsclk_sel |= CLK_PLL1_DIV2;
>> + phy_cfg = dp_pll_rbr_cfg;
>> + cfg_size = ARRAY_SIZE(dp_pll_rbr_cfg);
>> + break;
>> + case 270000:
>> + clk_ctrl = DP_PLL_DATA_RATE_HBR;
>> + hsclk_sel |= CLK_PLL1_DIV2;
>> + phy_cfg = dp_pll_hbr_cfg;
>> + cfg_size = ARRAY_SIZE(dp_pll_hbr_cfg);
>> + break;
>> + case 540000:
>> + clk_ctrl = DP_PLL_DATA_RATE_HBR2;
>> + hsclk_sel |= CLK_PLL1_DIV1;
>> + phy_cfg = dp_pll_hbr2_cfg;
>> + cfg_size = ARRAY_SIZE(dp_pll_hbr2_cfg);
>> + break;
>> + }
>> +
>> + clk_ctrl |= DP_PLL_CLOCK_ENABLE | DP_PLL_ENABLE;
>> + writel(clk_ctrl, tcphy->base + DP_CLK_CTL);
>>
>> - /* set the default mode to RBR */
>> - writel(DP_PLL_CLOCK_ENABLE | DP_PLL_ENABLE | DP_PLL_DATA_RATE_RBR,
>> - tcphy->base + DP_CLK_CTL);
>> + writel(hsclk_sel, tcphy->base + CMN_DIAG_HSCLK_SEL);
>>
>> /* load the configuration of PLL1 */
>> - for (i = 0; i < ARRAY_SIZE(dp_pll_cfg); i++)
>> - writel(dp_pll_cfg[i].value, tcphy->base + dp_pll_cfg[i].addr);
>> + for (i = 0; i < cfg_size; i++)
>> + writel(phy_cfg[i].value, tcphy->base + phy_cfg[i].addr);
>> }
>>
>> static void tcphy_tx_usb3_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
>> @@ -535,9 +570,10 @@ static void tcphy_rx_usb3_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
>> writel(0xfb, tcphy->base + XCVR_DIAG_BIDI_CTRL(lane));
>> }
>>
>> -static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
>> +static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, int link_rate,
>> + u8 swing, u8 pre_emp, u32 lane)
>> {
>> - u16 rdata;
>> + u16 val;
> >From what I see you are only renaming rdata to val, there is any reason?
not reason, just rename a new variable :)
>
>> writel(0xbefc, tcphy->base + XCVR_PSM_RCTRL(lane));
>> writel(0x6799, tcphy->base + TX_PSC_A0(lane));
>> @@ -545,25 +581,31 @@ static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
>> writel(0x98, tcphy->base + TX_PSC_A2(lane));
>> writel(0x98, tcphy->base + TX_PSC_A3(lane));
>>
>> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_000(lane));
>> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_001(lane));
>> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_010(lane));
>> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_011(lane));
>> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_100(lane));
>> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_101(lane));
>> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_110(lane));
>> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_111(lane));
>> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_10(lane));
>> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_01(lane));
>> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_00(lane));
>> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_11(lane));
>> -
>> - writel(0x128, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
>> - writel(0x400, tcphy->base + TX_DIAG_TX_DRV(lane));
>> -
>> - rdata = readl(tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
>> - rdata = (rdata & 0x8fff) | 0x6000;
>> - writel(rdata, tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
>> + writel(tcphy->config[swing][pre_emp].swing,
>> + tcphy->base + TX_TXCC_MGNFS_MULT_000(lane));
>> + writel(tcphy->config[swing][pre_emp].pe,
>> + tcphy->base + TX_TXCC_CPOST_MULT_00(lane));
>> +
>> + if (swing == 2 && pre_emp == 0 && link_rate != 540000) {
>> + writel(0x700, tcphy->base + TX_DIAG_TX_DRV(lane));
>> + writel(0x13c, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
>> + } else {
>> + writel(0x128, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
>> + writel(0x0400, tcphy->base + TX_DIAG_TX_DRV(lane));
>> + }
>> +
>> + val = readl(tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
>> + val = val & 0x8fff;
>> + switch (link_rate) {
>> + case 162000:
>> + case 270000:
>> + val |= (6 << 12);
>> + break;
>> + case 540000:
>> + val |= (4 << 12);
>> + break;
>> + }
>> + writel(val, tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
>> }
>>
>> static inline int property_enable(struct rockchip_typec_phy *tcphy,
>> @@ -754,30 +796,33 @@ static int tcphy_phy_init(struct rockchip_typec_phy *tcphy, u8 mode)
>> tcphy_cfg_24m(tcphy);
>>
>> if (mode == MODE_DFP_DP) {
>> - tcphy_cfg_dp_pll(tcphy);
>> + tcphy_cfg_dp_pll(tcphy, DEFAULT_RATE);
>> for (i = 0; i < 4; i++)
>> - tcphy_dp_cfg_lane(tcphy, i);
>> + tcphy_dp_cfg_lane(tcphy, DEFAULT_RATE, 0, 0, i);
>>
>> writel(PIN_ASSIGN_C_E, tcphy->base + PMA_LANE_CFG);
>> } else {
>> tcphy_cfg_usb3_pll(tcphy);
>> - tcphy_cfg_dp_pll(tcphy);
>> + tcphy_cfg_dp_pll(tcphy, DEFAULT_RATE);
>> if (tcphy->flip) {
>> tcphy_tx_usb3_cfg_lane(tcphy, 3);
>> tcphy_rx_usb3_cfg_lane(tcphy, 2);
>> - tcphy_dp_cfg_lane(tcphy, 0);
>> - tcphy_dp_cfg_lane(tcphy, 1);
>> + tcphy_dp_cfg_lane(tcphy, DEFAULT_RATE, 0, 0, 0);
>> + tcphy_dp_cfg_lane(tcphy, DEFAULT_RATE, 0, 0, 1);
>> } else {
>> tcphy_tx_usb3_cfg_lane(tcphy, 0);
>> tcphy_rx_usb3_cfg_lane(tcphy, 1);
>> - tcphy_dp_cfg_lane(tcphy, 2);
>> - tcphy_dp_cfg_lane(tcphy, 3);
>> + tcphy_dp_cfg_lane(tcphy, DEFAULT_RATE, 0, 0, 2);
>> + tcphy_dp_cfg_lane(tcphy, DEFAULT_RATE, 0, 0, 3);
>> }
>>
>> writel(PIN_ASSIGN_D_F, tcphy->base + PMA_LANE_CFG);
>> }
>>
>> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
>> + val = readl(tcphy->base + DP_MODE_CTL);
>> + val &= ~DP_MODE_MASK;
>> + val |= DP_MODE_ENTER_A2 | DP_LINK_RESET_DEASSERTED;
>> + writel(val, tcphy->base + DP_MODE_CTL);
>>
>> reset_control_deassert(tcphy->uphy_rst);
>>
>> @@ -990,7 +1035,7 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
>> property_enable(tcphy, &cfg->uphy_dp_sel, 1);
>>
>> ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,
>> - val, val & DP_MODE_A2, 1000,
>> + val, val & DP_MODE_A2_ACK, 1000,
>> PHY_MODE_SET_TIMEOUT);
>> if (ret < 0) {
>> dev_err(tcphy->dev, "failed to wait TCPHY enter A2\n");
>> @@ -999,13 +1044,19 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
>>
>> tcphy_dp_aux_calibration(tcphy);
>>
>> - writel(DP_MODE_ENTER_A0, tcphy->base + DP_MODE_CTL);
>> + /* enter A0 mode */
>> + val = readl(tcphy->base + DP_MODE_CTL);
>> + val &= ~DP_MODE_MASK;
>> + val |= DP_MODE_ENTER_A0;
>> + writel(val, tcphy->base + DP_MODE_CTL);
>>
>> ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,
>> - val, val & DP_MODE_A0, 1000,
>> + val, val & DP_MODE_A0_ACK, 1000,
>> PHY_MODE_SET_TIMEOUT);
>> if (ret < 0) {
>> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
>> + val &= ~DP_MODE_MASK;
>> + val |= DP_MODE_ENTER_A2;
>> + writel(val, tcphy->base + DP_MODE_CTL);
>> dev_err(tcphy->dev, "failed to wait TCPHY enter A0\n");
>> goto power_on_finish;
>> }
>> @@ -1023,6 +1074,7 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
>> static int rockchip_dp_phy_power_off(struct phy *phy)
>> {
>> struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
>> + u32 val;
>>
>> mutex_lock(&tcphy->lock);
>>
>> @@ -1031,7 +1083,10 @@ static int rockchip_dp_phy_power_off(struct phy *phy)
>>
>> tcphy->mode &= ~MODE_DFP_DP;
>>
>> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
>> + val = readl(tcphy->base + DP_MODE_CTL);
>> + val &= ~DP_MODE_MASK;
>> + val |= DP_MODE_ENTER_A2;
>> + writel(val, tcphy->base + DP_MODE_CTL);
>>
>> if (tcphy->mode == MODE_DISCONNECT)
>> tcphy_phy_deinit(tcphy);
>> @@ -1047,6 +1102,30 @@ static const struct phy_ops rockchip_dp_phy_ops = {
>> .owner = THIS_MODULE,
>> };
>>
>> +static int type_c_dp_phy_config(struct phy *phy, int link_rate,
> s/type_c/typec/ to be coherent with the rest of the code.
>
>> + int lanes, u8 swing, u8 pre_emp)
>> +{
>> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
>> + u8 i;
>> +
>> + tcphy_cfg_dp_pll(tcphy, link_rate);
>> +
>> + if (tcphy->mode == MODE_DFP_DP) {
>> + for (i = 0; i < 4; i++)
>> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, i);
>> + } else {
>> + if (tcphy->flip) {
>> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 0);
>> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 1);
>> + } else {
>> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 2);
>> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 3);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
>> struct device *dev)
>> {
>> @@ -1087,6 +1166,14 @@ static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
>> return PTR_ERR(tcphy->tcphy_rst);
>> }
>>
>> + /*
>> + * check if phy_config pass from dts, if yes,
>> + * need to use this phy config to do software training later
>> + */
>> + if (!of_property_read_u32_array(dev->of_node, "rockchip,phy_config",
>> + (u32 *)tcphy->config, sizeof(tcphy->config) / sizeof(u32)))
>> + tcphy->need_software_training = 1;
>> +
>> return 0;
>> }
>>
>> @@ -1171,6 +1258,7 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
>> }
>> }
>>
>> + tcphy->typec_phy_config = type_c_dp_phy_config;
> type_c_dp_phy_config -> typec_dp_phy_config
>
>> pm_runtime_enable(dev);
>>
>> for_each_available_child_of_node(np, child_np) {
>> diff --git a/include/soc/rockchip/rockchip_phy_typec.h b/include/soc/rockchip/rockchip_phy_typec.h
>> new file mode 100644
>> index 0000000..e25840e
>> --- /dev/null
>> +++ b/include/soc/rockchip/rockchip_phy_typec.h
>> @@ -0,0 +1,72 @@
>> +/*
> Add the SPDX License identifier ...
>
>> + * Copyright (c) 2018, Fuzhou Rockchip Electronics Co., Ltd
>> + * Author: Lin Huang <hl@rock-chips.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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.
> ... and remove the above notice.
>
>> + */
>> +#ifndef __SOC_ROCKCHIP_PHY_TYPEC_H
>> +#define __SOC_ROCKCHIP_PHY_TYPEC_H
>> +
>> +
> Remove the extra new line.
>
>> +struct usb3phy_reg {
>> + u32 offset;
>> + u32 enable_bit;
>> + u32 write_enable;
>> +};
>> +
>> +/**
>> + * struct rockchip_usb3phy_port_cfg: usb3-phy port configuration.
>> + * @reg: the base address for usb3-phy config.
>> + * @typec_conn_dir: the register of type-c connector direction.
>> + * @usb3tousb2_en: the register of type-c force usb2 to usb2 enable.
>> + * @external_psm: the register of type-c phy external psm clock.
>> + * @pipe_status: the register of type-c phy pipe status.
>> + * @usb3_host_disable: the register of type-c usb3 host disable.
>> + * @usb3_host_port: the register of type-c usb3 host port.
>> + * @uphy_dp_sel: the register of type-c phy DP select control.
>> + */
>> +struct rockchip_usb3phy_port_cfg {
>> + unsigned int reg;
>> + struct usb3phy_reg typec_conn_dir;
>> + struct usb3phy_reg usb3tousb2_en;
>> + struct usb3phy_reg external_psm;
>> + struct usb3phy_reg pipe_status;
>> + struct usb3phy_reg usb3_host_disable;
>> + struct usb3phy_reg usb3_host_port;
>> + struct usb3phy_reg uphy_dp_sel;
>> +};
>> +
>> +struct phy_config {
>> + int swing;
>> + int pe;
>> +};
>> +
>> +struct rockchip_typec_phy {
>> + struct device *dev;
>> + void __iomem *base;
>> + struct extcon_dev *extcon;
>> + struct regmap *grf_regs;
>> + struct clk *clk_core;
>> + struct clk *clk_ref;
>> + struct reset_control *uphy_rst;
>> + struct reset_control *pipe_rst;
>> + struct reset_control *tcphy_rst;
>> + struct rockchip_usb3phy_port_cfg *port_cfgs;
>> + /* mutex to protect access to individual PHYs */
>> + struct mutex lock;
>> + struct phy_config config[3][4];
>> + u8 need_software_training;
>> + bool flip;
>> + u8 mode;
>> + int (*typec_phy_config)(struct phy *phy, int link_rate,
>> + int lanes, u8 swing, u8 pre_emp);
>> +};
>> +
>> +#endif
> Best regards,
> Enric
>
>> --
>> 2.7.4
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>
^ permalink raw reply
* [PATCH v7 11/24] ASoC: qdsp6: q6afe: Add support to MI2S ports
From: Banajit Goswami @ 2018-05-09 3:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180501120820.11016-12-srinivas.kandagatla@linaro.org>
On 5/1/2018 5:08 AM, Srinivas Kandagatla wrote:
> This patch adds support to 4 MI2S ports on LPASS.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Reviewed-and-tested-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
> sound/soc/qcom/qdsp6/q6afe.c | 224 +++++++++++++++++++++++++++++++++++++++++++
> sound/soc/qcom/qdsp6/q6afe.h | 13 +++
> 2 files changed, 237 insertions(+)
>
> diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
> index be55b3ab045f..ceb18aaa5af4 100644
> --- a/sound/soc/qcom/qdsp6/q6afe.c
> +++ b/sound/soc/qcom/qdsp6/q6afe.c
> @@ -14,6 +14,10 @@
> #include <linux/spinlock.h>
> #include <linux/delay.h>
> #include <linux/soc/qcom/apr.h>
> +#include <sound/soc.h>
> +#include <sound/soc-dai.h>
> +#include <sound/pcm.h>
<snip>
> union afe_port_config {
> struct afe_param_id_hdmi_multi_chan_audio_cfg hdmi_multi_ch;
> struct afe_param_id_slimbus_cfg slim_cfg;
> + struct afe_param_id_i2s_cfg i2s_cfg;
> } __packed;
>
> struct q6afe_port {
> @@ -210,6 +264,22 @@ static struct afe_port_map port_maps[AFE_PORT_MAX] = {
> SLIMBUS_4_RX, 1, 1},
> [SLIMBUS_5_RX] = { AFE_PORT_ID_SLIMBUS_MULTI_CHAN_5_RX,
> SLIMBUS_5_RX, 1, 1},
> + [QUATERNARY_MI2S_RX] = { AFE_PORT_ID_QUATERNARY_MI2S_RX,
> + QUATERNARY_MI2S_RX, 1, 1},
> + [QUATERNARY_MI2S_TX] = { AFE_PORT_ID_QUATERNARY_MI2S_TX,
> + QUATERNARY_MI2S_TX, 0, 1},
> + [SECONDARY_MI2S_RX] = { AFE_PORT_ID_SECONDARY_MI2S_RX,
> + SECONDARY_MI2S_RX, 1, 1},
> + [SECONDARY_MI2S_TX] = { AFE_PORT_ID_SECONDARY_MI2S_TX,
> + SECONDARY_MI2S_TX, 0, 1},
> + [TERTIARY_MI2S_RX] = { AFE_PORT_ID_TERTIARY_MI2S_RX,
> + TERTIARY_MI2S_RX, 1, 1},
> + [TERTIARY_MI2S_TX] = { AFE_PORT_ID_TERTIARY_MI2S_TX,
> + TERTIARY_MI2S_TX, 0, 1},
> + [PRIMARY_MI2S_RX] = { AFE_PORT_ID_PRIMARY_MI2S_RX,
> + PRIMARY_MI2S_RX, 1, 1},
> + [PRIMARY_MI2S_TX] = { AFE_PORT_ID_PRIMARY_MI2S_TX,
> + PRIMARY_MI2S_RX, 0, 1},
Can these be added after SLIMBUS_6_RX?
Also, for better arrangement, please add the port_maps from
PRIMARY_MI2S_RX to QUATERNARY_MI2S_TX in the same sequence of definition
of AFE ports.
Otherwise LGTM.
Once fixed the above comment add-
Acked-by: Banajit Goswami <bgoswami@codeaurora.org>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH] hwmon: (aspeed-pwm-tacho) Use 24MHz clock
From: Lei YU @ 2018-05-09 3:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8a801894-4293-d2e8-4673-3d9ff5c2a5bb@roeck-us.net>
On Tue, May 8, 2018 at 9:51 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> No mixed C/C++ comments in hwmon drivers.
>
>> aspeed_set_clock_source(priv->regmap, 0);
>> + priv->clk_freq = 24000000;
>>
>
>
> Are you saying that clk_get_rate() is wrong ? Anyway, if the DT is bad, it
> should be fixed.
Nope, clk_get_rate() is OK.
The reason I make this change is because the PWM supports two types of clock
source, the 24MHz or the clock from memory controller.
If the DT uses 24MHz clock, this code is OK.
But if the DT configs this pwm to use mclk (memory controller clk), this piece
of code becomes wrong, because the code
`aspeed_set_clock_source(priv->regmap, 0)` configs the device to use the 24MHz
clock.
So no matter what DT configs the clk, this driver *always* uses 24MHz clock.
That's why I want to make this change.
> I am not a friend of hacking drivers to fix up bad DTs, and much less so
> without explanation.
> Plus, how do we know that the next chip supported by the driver doesn't have
> a 32MHz clock ?
This driver currently supports ast2400 and ast2500, and they both use 24MHz
clock.
In case future device uses a different clock, we can update this code, right?
> Really, please fix the DT.
Sure, I will send patch to config the clock to use fixed 24MHz clock as well.
>
> Guenter
>
>> aspeed_create_type(priv);
>>
>
>
^ permalink raw reply
* [PATCH 1/4] drm/rockchip: add transfer function for cdn-dp
From: hl @ 2018-05-09 2:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFqH_522eDgyknF+sRGyZzusL7Ld0o_N-SP1jXCG1iwmav-n0Q@mail.gmail.com>
Hi Enric,
On Monday, May 07, 2018 07:27 PM, Enric Balletbo Serra wrote:
> Hi Lin,
>
> I am interested in these patches, could you cc me on newer versions? Thanks.
>
> Some comments below.
Sure, will cc to you next version.
>
> 2018-05-04 10:08 GMT+02:00 Lin Huang <hl@rock-chips.com>:
>> From: Chris Zhong <zyw@rock-chips.com>
>>
>> We may support training outside firmware, so we need support
>> dpcd read/write to get the message or do some setting with
>> display.
>>
>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>> Signed-off-by: Lin Huang <hl@rock-chips.com>
>> ---
>> drivers/gpu/drm/rockchip/cdn-dp-core.c | 55 ++++++++++++++++++++++++----
>> drivers/gpu/drm/rockchip/cdn-dp-core.h | 1 +
>> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 66 +++++++++++++++++++++++++++++-----
>> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 14 ++++++--
>> 4 files changed, 119 insertions(+), 17 deletions(-)
>>
> In general, for this patch and all the other patches in the series I
> saw that checkpatch spits out some warnings, could you fix these and
> ideally run checkpatch with the --strict --subjective option?
Okay.
>
>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>> index c6fbdcd..268c190 100644
>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>> @@ -176,8 +176,8 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
>> u8 value;
>>
>> *sink_count = 0;
>> - ret = cdn_dp_dpcd_read(dp, DP_SINK_COUNT, &value, 1);
>> - if (ret)
>> + ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, &value, 1);
>> + if (ret < 0)
>> return ret;
>>
>> *sink_count = DP_GET_SINK_COUNT(value);
>> @@ -374,9 +374,9 @@ static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp)
>> if (!cdn_dp_check_sink_connection(dp))
>> return -ENODEV;
>>
>> - ret = cdn_dp_dpcd_read(dp, DP_DPCD_REV, dp->dpcd,
>> - DP_RECEIVER_CAP_SIZE);
>> - if (ret) {
>> + ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
>> + sizeof(dp->dpcd));
>> + if (ret < 0) {
>> DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
>> return ret;
>> }
>> @@ -582,8 +582,8 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
>> if (!port || !dp->link.rate || !dp->link.num_lanes)
>> return false;
>>
>> - if (cdn_dp_dpcd_read(dp, DP_LANE0_1_STATUS, link_status,
>> - DP_LINK_STATUS_SIZE)) {
>> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
>> + DP_LINK_STATUS_SIZE) {
>> DRM_ERROR("Failed to get link status\n");
>> return false;
>> }
>> @@ -1012,6 +1012,40 @@ static int cdn_dp_pd_event(struct notifier_block *nb,
>> return NOTIFY_DONE;
>> }
>>
>> +static ssize_t cdn_dp_aux_transfer(struct drm_dp_aux *aux,
>> + struct drm_dp_aux_msg *msg)
>> +{
>> + struct cdn_dp_device *dp = container_of(aux, struct cdn_dp_device, aux);
>> + int ret;
>> + u8 status;
>> +
>> + switch (msg->request & ~DP_AUX_I2C_MOT) {
>> + case DP_AUX_NATIVE_WRITE:
>> + case DP_AUX_I2C_WRITE:
>> + case DP_AUX_I2C_WRITE_STATUS_UPDATE:
>> + ret = cdn_dp_dpcd_write(dp, msg->address, msg->buffer,
>> + msg->size);
>> + break;
>> + case DP_AUX_NATIVE_READ:
>> + case DP_AUX_I2C_READ:
>> + ret = cdn_dp_dpcd_read(dp, msg->address, msg->buffer,
>> + msg->size);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + status = cdn_dp_get_aux_status(dp);
>> + if (status == AUX_STAUS_ACK)
>> + msg->reply = DP_AUX_NATIVE_REPLY_ACK;
>> + else if (status == AUX_STAUS_NACK)
>> + msg->reply = DP_AUX_NATIVE_REPLY_NACK;
>> + else if (status == AUX_STAUS_DEFER)
>> + msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
>> +
> I think that you would mean STATUS instead of STAUS on these defines.
>
> What happens if the status is AUX_STATUS_SINK_ERROR or AUX_STATUS_BUS_ERROR?
drm_dp_i2c_do_msg() will mark it as invalid i2c relay and return error.
>
>> + return ret;
>> +}
>> +
>> static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>> {
>> struct cdn_dp_device *dp = dev_get_drvdata(dev);
>> @@ -1030,6 +1064,13 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>> dp->active = false;
>> dp->active_port = -1;
>> dp->fw_loaded = false;
>> + dp->aux.name = "DP-AUX";
>> + dp->aux.transfer = cdn_dp_aux_transfer;
>> + dp->aux.dev = dev;
>> +
>> + ret = drm_dp_aux_register(&dp->aux);
>> + if (ret)
>> + return ret;
>>
>> INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
>>
>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>> index f57e296..46159b2 100644
>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>> @@ -78,6 +78,7 @@ struct cdn_dp_device {
>> struct platform_device *audio_pdev;
>> struct work_struct event_work;
>> struct edid *edid;
>> + struct drm_dp_aux aux;
>>
>> struct mutex lock;
>> bool connected;
>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>> index eb3042c..b2f532a 100644
>> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>> @@ -221,7 +221,11 @@ static int cdn_dp_reg_write_bit(struct cdn_dp_device *dp, u16 addr,
>> sizeof(field), field);
>> }
>>
>> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>> +/*
>> + * Returns the number of bytes transferred on success, or a negative error
>> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
>> + */
> Returns the number of bytes or -ETIMEDOUT, no other negative errors
> can be returned, right?
>
> I am not English native but the last phrase sounds incorrect to me,
> I'd rephrase it: (open to suggestions)
>
> -ETIMEDOUT if fails to receive the mailbox message.
>
>> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>> {
>> u8 msg[5], reg[5];
>> int ret;
>> @@ -247,24 +251,40 @@ int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>> goto err_dpcd_read;
>>
>> ret = cdn_dp_mailbox_read_receive(dp, data, len);
>> + if (!ret)
>> + return len;
>>
>> err_dpcd_read:
>> + DRM_DEV_ERROR(dp->dev, "dpcd read failed: %d\n", ret);
>> return ret;
>> }
>>
>> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
>> +#define CDN_AUX_HEADER_SIZE 5
>> +#define CDN_AUX_MSG_SIZE 20
>> +/*
>> + * Returns the number of bytes transferred on success, or a negative error
>> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
> Same as above. Sounds incorrect to me.
>
>> + * -EINVAL is return if get the wrong data size after message send.
> Same here.
>
>> + */
>> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>> {
>> - u8 msg[6], reg[5];
>> + u8 msg[CDN_AUX_MSG_SIZE + CDN_AUX_HEADER_SIZE];
>> + u8 reg[CDN_AUX_HEADER_SIZE];
>> int ret;
>>
>> - msg[0] = 0;
>> - msg[1] = 1;
>> + if (WARN_ON(len > CDN_AUX_MSG_SIZE) || WARN_ON(len <= 0))
>> + return -EINVAL;
>> +
>> + msg[0] = (len >> 8) & 0xff;
>> + msg[1] = len & 0xff;
>> msg[2] = (addr >> 16) & 0xff;
>> msg[3] = (addr >> 8) & 0xff;
>> msg[4] = addr & 0xff;
>> - msg[5] = value;
>> +
>> + memcpy(msg + CDN_AUX_HEADER_SIZE, data, len);
>> +
>> ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_WRITE_DPCD,
>> - sizeof(msg), msg);
>> + CDN_AUX_HEADER_SIZE + len, msg);
>> if (ret)
>> goto err_dpcd_write;
>>
>> @@ -277,8 +297,12 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
>> if (ret)
>> goto err_dpcd_write;
>>
>> - if (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))
>> + if ((len != (reg[0] << 8 | reg[1])) ||
>> + (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))) {
>> ret = -EINVAL;
>> + } else {
>> + return len;
>> + }
>>
>> err_dpcd_write:
>> if (ret)
>> @@ -286,6 +310,32 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
>> return ret;
>> }
>>
>> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp)
>> +{
>> + u8 status;
>> + int ret;
>> +
>> + ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_GET_LAST_AUX_STAUS,
>> + 0, NULL);
>> + if (ret)
>> + goto err_get_hpd;
>> +
>> + ret = cdn_dp_mailbox_validate_receive(dp, MB_MODULE_ID_DP_TX,
>> + DPTX_GET_LAST_AUX_STAUS, sizeof(status));
>> + if (ret)
>> + goto err_get_hpd;
>> +
>> + ret = cdn_dp_mailbox_read_receive(dp, &status, sizeof(status));
>> + if (ret)
>> + goto err_get_hpd;
>> +
>> + return status;
>> +
>> +err_get_hpd:
>> + DRM_DEV_ERROR(dp->dev, "get aux status failed: %d\n", ret);
>> + return ret;
>> +}
>> +
>> int cdn_dp_load_firmware(struct cdn_dp_device *dp, const u32 *i_mem,
>> u32 i_size, const u32 *d_mem, u32 d_size)
>> {
>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>> index c4bbb4a83..aedf2dc 100644
>> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>> @@ -328,6 +328,13 @@
>> #define GENERAL_BUS_SETTINGS 0x03
>> #define GENERAL_TEST_ACCESS 0x04
>>
>> +/* AUX status*/
>> +#define AUX_STAUS_ACK 0
>> +#define AUX_STAUS_NACK 1
>> +#define AUX_STAUS_DEFER 2
>> +#define AUX_STAUS_SINK_ERROR 3
>> +#define AUX_STAUS_BUS_ERROR 4
>> +
> For the five defines, s/STAUS/STATUS/
will fix it.
>
>> #define DPTX_SET_POWER_MNG 0x00
>> #define DPTX_SET_HOST_CAPABILITIES 0x01
>> #define DPTX_GET_EDID 0x02
>> @@ -469,8 +476,11 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 lanes, bool flip);
>> int cdn_dp_event_config(struct cdn_dp_device *dp);
>> u32 cdn_dp_get_event(struct cdn_dp_device *dp);
>> int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
>> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value);
>> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len);
>> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
>> + u8 *data, u16 len);
>> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
>> + u8 *data, u16 len);
>> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp);
>> int cdn_dp_get_edid_block(void *dp, u8 *edid,
>> unsigned int block, size_t length);
>> int cdn_dp_train_link(struct cdn_dp_device *dp);
>> --
>> 2.7.4
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>
^ permalink raw reply
* [PATCH] alpha: io: reorder barriers to guarantee writeX() and iowriteX() ordering #2
From: Matt Turner @ 2018-05-09 2:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2fd54f7c-ff73-5935-a253-c924f4373cc6@codeaurora.org>
On Fri, Apr 20, 2018 at 9:20 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> Hi Matt,
>
> On 4/17/2018 2:43 PM, Sinan Kaya wrote:
>> On 4/16/2018 6:16 PM, Sinan Kaya wrote:
>>> memory-barriers.txt has been updated with the following requirement.
>>>
>>> "When using writel(), a prior wmb() is not needed to guarantee that the
>>> cache coherent memory writes have completed before writing to the MMIO
>>> region."
>>>
>>> Current writeX() and iowriteX() implementations on alpha are not
>>> satisfying this requirement as the barrier is after the register write.
>>>
>>> Move mb() in writeX() and iowriteX() functions to guarantee that HW
>>> observes memory changes before performing register operations.
>>>
>>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>> arch/alpha/kernel/io.c | 14 +++++++-------
>>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> Sorry for catching this late but this also needs to go to 4.17 after
>> review.
>>
>> I missed the writel() implementation on arch/alpha/kernel/io.c file
>> on my first patch.
>>
>
> Can you also queue this for 4.17?
>
> There are already drivers checked into 4.17 that dropped the unnecessary
> barriers.
>
> I really hate to see Alpha broken because of this.
Yes, I will pick it up for 4.17.
Thanks for the patch.
^ permalink raw reply
* [PATCH v7 10/24] ASoC: qdsp6: qdafe: Add SLIMBus port Support
From: Banajit Goswami @ 2018-05-09 2:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180501120820.11016-11-srinivas.kandagatla@linaro.org>
On 5/1/2018 5:08 AM, Srinivas Kandagatla wrote:
> This patch adds support to 6 SLIMBus AFE ports, which are used as
> backend dais.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Reviewed-and-tested-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
> sound/soc/qcom/qdsp6/q6afe.c | 129 +++++++++++++++++++++++++++++++++++++++++++
> sound/soc/qcom/qdsp6/q6afe.h | 14 +++++
> 2 files changed, 143 insertions(+)
>
>
Acked-by: Banajit Goswami <bgoswami@codeaurora.org>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v7 09/24] ASoC: qdsp6: q6afe: Add q6afe driver
From: Banajit Goswami @ 2018-05-09 2:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180501120820.11016-10-srinivas.kandagatla@linaro.org>
On 5/1/2018 5:08 AM, Srinivas Kandagatla wrote:
> This patch adds support to Q6AFE (Audio Front End) module on Q6DSP.
>
> AFE module sits right at the other end of cpu where the codec/audio
> devices are connected.
>
> AFE provides abstraced interfaces to both hardware and virtual devices.
> Each AFE tx/rx port can be configured to connect to one of the hardware
> devices like codec, hdmi, slimbus, i2s and so on. AFE services include
> starting, stopping, and if needed, any configurations of the ports.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Reviewed-and-tested-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
> sound/soc/qcom/Kconfig | 4 +
> sound/soc/qcom/qdsp6/Makefile | 1 +
> sound/soc/qcom/qdsp6/q6afe.c | 536 ++++++++++++++++++++++++++++++++++++++++++
> sound/soc/qcom/qdsp6/q6afe.h | 35 +++
> 4 files changed, 576 insertions(+)
> create mode 100644 sound/soc/qcom/qdsp6/q6afe.c
> create mode 100644 sound/soc/qcom/qdsp6/q6afe.h
>
Acked-by: Banajit Goswami <bgoswami@codeaurora.org>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [RESEND PATCH] pinctrl: rockchip: Disable interrupt when changing it's capability
From: JeffyChen @ 2018-05-09 2:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=W_v+aoNT09k-2exVABTFzOp0heY-XWcLi95uPisZhiGQ@mail.gmail.com>
Hi Doug,
On 05/09/2018 03:46 AM, Doug Anderson wrote:
> One note is that in the case Brian points at (where we need to
> simulate EDGE_BOTH by swapping edges) we purposely ignored the TRM and
> we needed to do that to avoid losing interrupts. For details, see
> commit 53b1bfc76df2 ("pinctrl: rockchip: Avoid losing interrupts when
> supporting both edges"). We did this because:
>
> 1. We believed that the IP block in Rockchip SoCs has nearly the same
> logic as "gpio-dwapb.c" and that's what "gpio-dwapb.c" did.
>
hmm, but i saw the gpio-dwapb.c actually toggle trigger after handle
irq, which might avoid the race Brian mentioned:
+ generic_handle_irq(gpio_irq);
+ irq_status &= ~BIT(hwirq);
+
+ if ((irq_get_trigger_type(gpio_irq) & IRQ_TYPE_SENSE_MASK)
+ == IRQ_TYPE_EDGE_BOTH)
+ dwapb_toggle_trigger(gpio, hwirq);
and i also saw ./qcom/pinctrl-msm.c do the
toggle(msm_gpio_update_dual_edge_pos) at the end of
msm_gpio_irq_set_type and msm_gpio_irq_ack, that seems can avoid the
polarity races too.
> 2. We were actually losing real interrupts and this was the only way
> we could figure out how to fix it.
>
> When I tested that back in the day I was fairly convinced that we
> weren't losing any interrupts in the EDGE_BOTH case after my fix, but
> I certainly could have messed up.
>
>
> For the EDGE_BOTH case it was important not to lose an interrupt
> because, as you guys are talking about, we could end up configured the
> wrong way. I think in your case where you're just picking one
> polarity losing an interrupt shouldn't matter since it's undefined
> exactly if an edge happens while you're in the middle of executing
> rockchip_irq_set_type(). Is that right?
right, so we now have 2 cases: rockchip_irq_demux/ rockchip_irq_set_type
if i'm right about the spurious irq(only happen when set rising for a
high gpio, or set falling for a low gpio), then:
1/ rockchip_irq_demux
it's important to not losing irqs in this case, maybe we can
a) ack irq
b) update polarity for edge both irq
we don't need to disable irq in b), since we would not hit the spurious
irq cases here(always check gpio level to toggle it)
2/ rockchip_irq_set_type
it's important to not having spurious irqs
so we can disable irq during changing polarity only in these case:
((rising && gpio is heigh) || (falling && gpio is low))
i'm still confirming the spurious irq with IC guys.
>
>
> -Doug
>
>
>
^ permalink raw reply
* [PATCH] arm64: dts: sprd: fix typo in 'remote-endpoint'
From: Chunyan Zhang @ 2018-05-09 1:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180508150952.24562-4-robh@kernel.org>
Hi Rob,
On 8 May 2018 at 23:09, Rob Herring <robh@kernel.org> wrote:
> dtc now warns on incomplete OF graph endpoint connections:
>
> arch/arm64/boot/dts/sprd/sp9860g-1h10.dtb: Warning (graph_endpoint): /soc/stm at 10006000/port/endpoint: graph connection to node '/soc/funnel at 10001000/ports/port at 2/endpoint' is not bidirectional
>
> The cause is a typo in 'remote-endpoint'.
Thanks for fixing this and,
Acked-by: Chunyan Zhang <zhang.lyra@gmail.com>
>
> Cc: Orson Zhai <orsonzhai@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Chunyan Zhang <zhang.lyra@gmail.com>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> arch/arm64/boot/dts/sprd/sc9860.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/sprd/sc9860.dtsi b/arch/arm64/boot/dts/sprd/sc9860.dtsi
> index 5dbfb796d9f9..dcd03ffcda03 100644
> --- a/arch/arm64/boot/dts/sprd/sc9860.dtsi
> +++ b/arch/arm64/boot/dts/sprd/sc9860.dtsi
> @@ -326,7 +326,7 @@
> reg = <4>;
> soc_funnel_in_port1: endpoint {
> slave-mode;
> - remote-endpioint =
> + remote-endpoint =
> <&stm_out_port>;
> };
> };
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH] clk: imx6ull: use OSC clock during AXI rate change
From: Jacky Bai @ 2018-05-09 1:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <152580354289.138124.13657232356773397650@swboyd.mtv.corp.google.com>
> Subject: Re: [PATCH] clk: imx6ull: use OSC clock during AXI rate change
>
> Quoting Stefan Agner (2018-05-08 06:20:03)
> > On 08.05.2018 09:32, Jacky Bai wrote:
> > >
> > > I have tried two 6ULL board, I don't meet such issue. System can
> > > boot up successfully with commit 6f9575e55632 included.
> > > Anyway, the change in this patch is ok for me. it is no harm to the
> > > BUS clock change flow.
> >
> > Hm, that is interesting, maybe it is because we use a different SKU?
> > Or maybe bootloader?
> >
> > I tested here with a 800 MHz clocked variant on a Colibri iMX6ULL
> > using U-Boot 2016.11.
> >
>
> I'll throw this into fixes today because it fixes something to be useful. It's still
> interesting to see what's different between the two setups though.
Thanks.
I will find some 800MHz parts and have a try with different bootloader later.
Will let you know, when I have some results.
Jacky
^ permalink raw reply
* [PATCH 2/4] pid: Export find_task_by_vpid for use in external modules
From: Eric W. Biederman @ 2018-05-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180508140640.0e312dba025df75cbf205cdb@arm.com>
Kim Phillips <kim.phillips@arm.com> writes:
> This patch is in the context of allowing the Coresight h/w
> trace driver suite to be loaded as modules. Coresight uses
> find_task_by_vpid when running in direct capture mode (via sysfs)
> when getting/setting the context ID comparator to trigger on
> (/sys/bus/coresight/devices/<x>.etm/ctxid_pid).
Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
There is no way to implement a sysfs file that takes a pid correctly.
Don't do it.
Pids are tied to pid namespaces and sysfs deliberately does not do
anything remotely resembly a pid namespace.
Eric
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Gargi Sharma <gs051095@gmail.com>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
> Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> ---
> Current CoreSight callsite:
>
> https://lxr.missinglinkelectronics.com/linux/include/linux/coresight.h#L285
>
> A quick look didn't find anything, but if Coresight needs to do
> something differently, please comment.
>
> kernel/pid.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/pid.c b/kernel/pid.c
> index 157fe4b19971..92b1b623f3e0 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -342,6 +342,7 @@ struct task_struct *find_task_by_vpid(pid_t vnr)
> {
> return find_task_by_pid_ns(vnr, task_active_pid_ns(current));
> }
> +EXPORT_SYMBOL_GPL(find_task_by_vpid);
>
> struct task_struct *find_get_task_by_vpid(pid_t nr)
> {
^ permalink raw reply
* [PATCH v7 7/7] arm64: defconfig: update config for Rockchip PCIe
From: Shawn Lin @ 2018-05-09 1:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525828278-69800-1-git-send-email-shawn.lin@rock-chips.com>
Now Rockchip PCIe drivers could support both of RC mode and EP
mode, so we need rename the config name. This patch updates
defconfig to reflect the fact that we want to build Rockchip PCIe
controller as RC mode, into a module as before.
Cc: linux-arm-kernel at lists.infradead.org
Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
arch/arm64/configs/defconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 1d3b406..8fd8bae 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -78,7 +78,8 @@ CONFIG_PCIE_ARMADA_8K=y
CONFIG_PCI_AARDVARK=y
CONFIG_PCI_TEGRA=y
CONFIG_PCIE_RCAR=y
-CONFIG_PCIE_ROCKCHIP=m
+CONFIG_PCIE_ROCKCHIP=y
+CONFIG_PCIE_ROCKCHIP_HOST=m
CONFIG_PCI_HOST_GENERIC=y
CONFIG_PCI_XGENE=y
CONFIG_PCI_HOST_THUNDER_PEM=y
--
1.9.1
^ permalink raw reply related
* Incoming sms problem on Motorola Droid 4
From: Tony Lindgren @ 2018-05-09 1:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180508215102.GA5253@amd>
* Pavel Machek <pavel@ucw.cz> [180508 21:53]:
> Hi!
>
> I have problems with incoming SMS. ofono tries to use +CNMI=1,2,2,1,0
>
> > AT+CNMI=?
> < +CNMI: (0,1,2),(0,1,2,3),(0,2),(0,1,2),(0,1)
> < OK
> ofonod[3070]: drivers/atmodem/sms.c:build_cnmi_string()
> ofonod[3070]: drivers/atmodem/sms.c:construct_ack_pdu()
> > AT+CNMI=1,2,2,1,0
> < OK
> ofonod[3070]: src/network.c:__ofono_netreg_add_status_watch() 0x5bbbf0
>
> ... unfortunately, with that configuration no messages are comming to
> ofono and the other phone sees them as "delievery failed".
>
> I had some luck with unicsy_demo using AT+CNMI=1,2 with text mode (not
> PDU) messages. That works well enough for me.
>
> Unfortunately, if I force ofono to pass "AT+CNMI=1,2", it does not
> work well, either.
>
> Any ideas how to debug this / what to try?
Well you can try to see what Android is doing for SMS with:
# echo 0x7fffffff > /sys/module/ts27010mux/parameters/debug_level
# dmesg | grep ts27010 | grep AT
To send SMS, looks like Android RIL first does:
2 AT+CMGS=327 where 327 seems to be the size of the whatever
encoded message. Then the next packet to dlci 2 contains the
encoded message that is of size 327.
When receiving, mdm6600 sends these on dlci 1:
~+WAKEUP
~+WAKEUP
~+WAKEUP
Then mdm6600 sends this on dlci 9:
~+CMT=372
And that's probably the incoming SMS size. But I don't see
anything for what actually reads the incoming SMS.
Regards,
Tony
^ permalink raw reply
* [PATCH 1/2] perf cs-etm: Support unknown_thread in cs_etm_auxtrace
From: Leo Yan @ 2018-05-09 0:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANLsYky4QatoCv_4k1r8eb4Rjugx9g2YZLG1dtHKJy9AsHtCSg@mail.gmail.com>
On Tue, May 08, 2018 at 11:01:30AM -0600, Mathieu Poirier wrote:
> On 3 May 2018 at 09:18, Leo Yan <leo.yan@linaro.org> wrote:
> > CoreSight doesn't allocate thread structure for unknown_thread in etm
> > auxtrace, so unknown_thread is NULL pointer. If the perf data doesn't
> > contain valid tid and then cs_etm__mem_access() uses unknown_thread
> > instead as thread handler, this results in segmentation fault when
> > thread__find_addr_map() accesses thread handler.
> >
> > This commit creates new thread data which is used by unknown_thread, so
> > CoreSight tracing can roll back to use unknown_thread if perf data
> > doesn't include valid thread info. This commit also releases thread
> > data for initialization failure case and for normal auxtrace free flow.
> >
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> > tools/perf/util/cs-etm.c | 25 +++++++++++++++++++++++--
> > 1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 6533b1a..417302c 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -239,6 +239,7 @@ static void cs_etm__free(struct perf_session *session)
> > for (i = 0; i < aux->num_cpu; i++)
> > zfree(&aux->metadata[i]);
> >
> > + thread__zput(aux->unknown_thread);
> > zfree(&aux->metadata);
> > zfree(&aux);
> > }
> > @@ -266,6 +267,7 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u64 address,
> > if (!thread) {
> > if (cpumode != PERF_RECORD_MISC_KERNEL)
> > return -EINVAL;
> > +
>
> Extra line, please remove.
>
> With this change:
>
> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks for reviewing, Mathieu. Will spin new patches and send out.
[...]
Thanks,
Leo Yan
^ permalink raw reply
* [PATCH] drivers/perf: arm-ccn: stop spamming dmesg in event_init
From: Kim Phillips @ 2018-05-08 23:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180504104117.8086-1-mark.rutland@arm.com>
On Fri, 4 May 2018 11:41:17 +0100
Mark Rutland <mark.rutland@arm.com> wrote:
[adding Pawel, arm-ccn driver author]
> The ARM CCN PMU driver uses dev_warn() to complain about parameters in
> the user-provided perf_event_attr. This means that under normal
> operation (e.g. a single invocation of the perf tool), dmesg may be
> spammed with multiple messages.
Nothing new there: perf does that all the time: E.g., a single, basic
x86 invocation causes all this:
[77360.630571] perf: interrupt took too long (2549 > 2500), lowering kernel.perf_event_max_sample_rate to 78250
[77360.630800] perf: interrupt took too long (3492 > 3186), lowering kernel.perf_event_max_sample_rate to 57250
[77360.631045] perf: interrupt took too long (4379 > 4365), lowering kernel.perf_event_max_sample_rate to 45500
[77360.631437] perf: interrupt took too long (5845 > 5473), lowering kernel.perf_event_max_sample_rate to 34000
[77360.631967] perf: interrupt took too long (7736 > 7306), lowering kernel.perf_event_max_sample_rate to 25750
[77360.632520] perf: interrupt took too long (9921 > 9670), lowering kernel.perf_event_max_sample_rate to 20000
[77360.633344] perf: interrupt took too long (12778 > 12401), lowering kernel.perf_event_max_sample_rate to 15500
[77360.634294] perf: interrupt took too long (16030 > 15972), lowering kernel.perf_event_max_sample_rate to 12250
[77360.635587] perf: interrupt took too long (20105 > 20037), lowering kernel.perf_event_max_sample_rate to 9750
[77360.637301] perf: interrupt took too long (25319 > 25131), lowering kernel.perf_event_max_sample_rate to 7750
> Tools may issue multiple syscalls to probe for feature support, and
Why isn't it helpful? A user can see the tool is trying different
options, and as they are tried, to see which ones are they can do
something about. Arm-ccn has plenty of h/w specific errors, and users
need to know things like node specific details.
> multiple applications (from multiple users) can attempt to open events
> simultaneously, so this is not very helpful,
Does running perf on the same PMU h/w against other users even work?
Is it even practical, if they get it to? In any case, for Arm-ccn,
since it's system-wide PMU h/w, this use-case is completely unrealistic.
> even if a user happens to have access to dmesg.
Again, unrealistic: If they have access to run arm-ccn, kptrs are
unrestricted, so they have way more access than to just dmesg.
> Worse, this can push important information out of
> the dmesg ring buffer,
Like what, specifically? People about to run perf do so on a stable
system, for measurement of performance accuracy reasons. Anyway,
there are logging daemons to help with that, and arm-ccn doesn't emit
*that* many messages such as to even come close to filling the buffer.
> and can significantly slow down syscall fuzzers,
> vastly increasing the time it takes to find critical bugs.
Facilitating first user experience - esp. for Arm perf - trumps fuzzer
runner convenience any day, in my book. Fuzzer runners can patch their
kernels prior to running the fuzzers.
> Demote the dev_warn() instances to dev_dbg(), as is the case for all
> other PMU drivers under drivers/perf/. Users who wish to debug PMU event
> initialisation can enable dynamic debug to receive these messages.
After having already debugged things to the point where they find the
problem is the driver's event_init() function? Then they find they
have to recompile their kernels yet again, with DYNAMIC_DEBUG set?
No, please, this is a definite usability regression for Arm-ccn users.
In fact, are you sure it shouldn't be the other way around?:
{kernel,PMU driver} developers that wish to run fuzzers should disable
warnings using /proc/sys/kernel/printk, or boot with a different
loglevel on the boot command line. I think our users would appreciate
that more than this patch.
Thanks,
Kim
^ permalink raw reply
* [PATCH v2 27/27] coresight: etm-perf: Add support for ETR backend
From: Mathieu Poirier @ 2018-05-08 22:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525165857-11096-28-git-send-email-suzuki.poulose@arm.com>
On Tue, May 01, 2018 at 10:10:57AM +0100, Suzuki K Poulose wrote:
> Add necessary support for using ETR as a sink in ETM perf tracing.
> We try make the best use of the available modes of buffers to
> try and avoid software double buffering.
>
> We can use the perf ring buffer for ETR directly if all of the
> conditions below are met :
> 1) ETR is DMA coherent
> 2) perf is used in snapshot mode. In full tracing mode, we cannot
> guarantee that the ETR will stop before it overwrites the data
> at the beginning of the trace buffer leading to loss of trace
> data. (The buffer which is being consumed by the perf is still
> hidden from the ETR).
> 3) ETR supports save-restore with a scatter-gather mechanism
> which can use a given set of pages we use the perf ring buffer
> directly. If we have an in-built TMC ETR Scatter Gather unit,
> we make use of a circular SG list to restart from a given head.
> However, we need to align the starting offset to 4K in this case.
> With CATU and ETR Save restore feature, we don't have to necessarily
> align the head of the buffer.
>
> If the ETR doesn't support either of this, we fallback to software
> double buffering.
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>
> Note: The conditions above need some rethink.
>
> For (1) : We always sync the buffer for CPU, before we update the
> pointers. So, we should be safe here and should be able to remove
> this condition.
>
> (2) is a bit more of problem, as the ETR (without SFIFO_2 mode)
> doesn't stop writing out the trace buffer, eventhough we exclude
> the part of the ring buffer currently consumed by perf, leading
> to loss of data. Also, since we don't have an interrupt (without
> SFIFO_2), we can't wake up the userspace reliably to consume
> the data.
>
> One possible option is to use an hrtimer to wake up the userspace
> early enough, using a low wakeup mark. But that doesn't necessarily
> guarantee that the ETR will not wrap around overwriting the data,
> as we can't modify the ETR pointers, unless we disable it, which
> could again potentially cause data loss in Circular Buffer mode.
> We may still be able to detect if there was a data loss by checking
> how far the userspace has consumed the data.
I thought about timers before but as you point out it comes with a wealth of
problems. Not having a buffer overflow interrupt is just a HW limitation we
need to live with until something better comes along.
> ---
> drivers/hwtracing/coresight/coresight-tmc-etr.c | 387 +++++++++++++++++++++++-
> drivers/hwtracing/coresight/coresight-tmc.h | 2 +
> 2 files changed, 386 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 8159e84..3e9ba02 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -31,6 +31,32 @@ struct etr_flat_buf {
> };
>
> /*
> + * etr_perf_buffer - Perf buffer used for ETR
> + * @etr_buf - Actual buffer used by the ETR
> + * @snaphost - Perf session mode
> + * @head - handle->head at the beginning of the session.
> + * @nr_pages - Number of pages in the ring buffer.
> + * @pages - Pages in the ring buffer.
> + * @flags - Capabilities of the hardware buffer used in the
> + * session. If flags == 0, we use software double
> + * buffering.
> + */
> +struct etr_perf_buffer {
> + struct etr_buf *etr_buf;
> + bool snapshot;
> + unsigned long head;
> + int nr_pages;
> + void **pages;
> + u32 flags;
> +};
> +
> +/* Convert the perf index to an offset within the ETR buffer */
> +#define PERF_IDX2OFF(idx, buf) ((idx) % ((buf)->nr_pages << PAGE_SHIFT))
> +
> +/* Lower limit for ETR hardware buffer in double buffering mode */
> +#define TMC_ETR_PERF_MIN_BUF_SIZE SZ_1M
> +
> +/*
> * The TMC ETR SG has a page size of 4K. The SG table contains pointers
> * to 4KB buffers. However, the OS may use a PAGE_SIZE different from
> * 4K (i.e, 16KB or 64KB). This implies that a single OS page could
> @@ -1164,7 +1190,7 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata)
> tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
> }
>
> -static int __maybe_unused
> +static int
> tmc_restore_etr_buf(struct tmc_drvdata *drvdata, struct etr_buf *etr_buf,
> unsigned long r_offset, unsigned long w_offset,
> unsigned long size, u32 status)
> @@ -1415,10 +1441,361 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
> return ret;
> }
>
> +/*
> + * tmc_etr_setup_perf_buf: Allocate ETR buffer for use by perf. We try to
> + * use perf ring buffer pages for the ETR when we can. In the worst case
> + * we fallback to software double buffering. The size of the hardware buffer
> + * in this case is dependent on the size configured via sysfs, if we can't
> + * match the perf ring buffer size. We scale down the size by half until
> + * it reaches a limit of 1M, beyond which we give up.
> + */
> +static struct etr_perf_buffer *
> +tmc_etr_setup_perf_buf(struct tmc_drvdata *drvdata, int node, int nr_pages,
> + void **pages, bool snapshot)
> +{
> + int i;
> + struct etr_buf *etr_buf;
> + struct etr_perf_buffer *etr_perf;
> + unsigned long size;
> + unsigned long buf_flags[] = {
> + ETR_BUF_F_RESTORE_FULL,
> + ETR_BUF_F_RESTORE_MINIMAL,
> + 0,
> + };
> +
> + etr_perf = kzalloc_node(sizeof(*etr_perf), GFP_KERNEL, node);
> + if (!etr_perf)
> + return ERR_PTR(-ENOMEM);
> +
> + size = nr_pages << PAGE_SHIFT;
> + /*
> + * TODO: We need to refine the following rule.
> + *
> + * We can use the perf ring buffer for ETR only if it is coherent
> + * and used in snapshot mode.
> + *
> + * The ETR (without SFIFO_2 mode) cannot stop writing when a
> + * certain limit is reached, nor can it interrupt driver.
> + * We can protect the data which is being consumed by the
> + * userspace, by hiding it from the ETR's tables. So, we could
> + * potentially loose the trace data only for the current session
> + * session if the ETR wraps around.
> + */
> + if (tmc_etr_has_cap(drvdata, TMC_ETR_COHERENT) && snapshot) {
> + for (i = 0; buf_flags[i]; i++) {
> + etr_buf = tmc_alloc_etr_buf(drvdata, size,
> + buf_flags[i], node, pages)
Indentation
> + if (!IS_ERR(etr_buf)) {
> + etr_perf->flags = buf_flags[i];
> + goto done;
> + }
> + }
> + }
> +
> + /*
> + * We have to now fallback to software double buffering.
> + * The tricky decision is choosing a size for the hardware buffer.
> + * We could start with drvdata->size (configurable via sysfs) and
> + * scale it down until we can allocate the data.
> + */
> + etr_buf = tmc_alloc_etr_buf(drvdata, size, 0, node, NULL);
The above comment doesn't match the code. We start with a buffer size equal to
what was requested and then fall back to drvdata->size if something goes wrong.
I don't see why drvdata->size gets involved at all. I would simply try to
reduce @size until we get a successful allocation.
> + if (!IS_ERR(etr_buf))
> + goto done;
> + size = drvdata->size;
> + do {
> + etr_buf = tmc_alloc_etr_buf(drvdata, size, 0, node, NULL);
> + if (!IS_ERR(etr_buf))
> + goto done;
> + size /= 2;
> + } while (size >= TMC_ETR_PERF_MIN_BUF_SIZE);
> +
> + kfree(etr_perf);
> + return ERR_PTR(-ENOMEM);
> +
> +done:
> + etr_perf->etr_buf = etr_buf;
> + return etr_perf;
> +}
> +
> +
> +static void *tmc_etr_alloc_perf_buffer(struct coresight_device *csdev,
> + int cpu, void **pages, int nr_pages,
> + bool snapshot)
> +{
> + struct etr_perf_buffer *etr_perf;
> + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + if (cpu == -1)
> + cpu = smp_processor_id();
> +
> + etr_perf = tmc_etr_setup_perf_buf(drvdata, cpu_to_node(cpu),
> + nr_pages, pages, snapshot);
> + if (IS_ERR(etr_perf)) {
> + dev_dbg(drvdata->dev, "Unable to allocate ETR buffer\n");
> + return NULL;
> + }
> +
> + etr_perf->snapshot = snapshot;
> + etr_perf->nr_pages = nr_pages;
> + etr_perf->pages = pages;
> +
> + return etr_perf;
> +}
> +
> +static void tmc_etr_free_perf_buffer(void *config)
> +{
> + struct etr_perf_buffer *etr_perf = config;
> +
> + if (etr_perf->etr_buf)
> + tmc_free_etr_buf(etr_perf->etr_buf);
> + kfree(etr_perf);
> +}
> +
> +/*
> + * Pad the etr buffer with barrier packets to align the head to 4K aligned
> + * offset. This is required for ETR SG backed buffers, so that we can rotate
> + * the buffer easily and avoid a software double buffering.
> + */
> +static long tmc_etr_pad_perf_buffer(struct etr_perf_buffer *etr_perf, long head)
> +{
> + long new_head;
> + struct etr_buf *etr_buf = etr_perf->etr_buf;
> +
> + head = PERF_IDX2OFF(head, etr_perf);
> + new_head = ALIGN(head, SZ_4K);
> + if (head == new_head)
> + return head;
> + /*
> + * If the padding is not aligned to barrier packet size
> + * we can't do much.
> + */
> + if ((new_head - head) % CORESIGHT_BARRIER_PKT_SIZE)
> + return -EINVAL;
> + return tmc_etr_buf_insert_barrier_packets(etr_buf, head,
> + new_head - head);
> +}
> +
> +static int tmc_etr_set_perf_buffer(struct coresight_device *csdev,
> + struct perf_output_handle *handle,
> + void *config)
> +{
> + int rc;
> + unsigned long flags;
> + long head, new_head;
> + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct etr_perf_buffer *etr_perf = config;
> + struct etr_buf *etr_buf = etr_perf->etr_buf;
> +
> + etr_perf->head = handle->head;
> + head = PERF_IDX2OFF(etr_perf->head, etr_perf);
> + switch (etr_perf->flags) {
> + case ETR_BUF_F_RESTORE_MINIMAL:
> + new_head = tmc_etr_pad_perf_buffer(etr_perf, head);
> + if (new_head < 0)
> + return new_head;
> + if (head != new_head) {
> + rc = perf_aux_output_skip(handle, new_head - head);
> + if (rc)
> + return rc;
> + etr_perf->head = handle->head;
> + head = new_head;
> + }
> + /* Fall through */
> + case ETR_BUF_F_RESTORE_FULL:
> + rc = tmc_restore_etr_buf(drvdata, etr_buf,
> + head, head, handle->size, 0);
> + break;
> + case 0:
> + /* Nothing to do here. */
> + rc = 0;
> + break;
> + default:
> + dev_warn(drvdata->dev, "Unexpected flags in etr_perf buffer\n");
> + WARN_ON(1);
> + rc = -EINVAL;
> + }
> +
> + /*
> + * This sink is going to be used in perf mode. No other session can
> + * grab it from us. So set the perf mode specific data here. This will
> + * be released just before we disable the sink from update_buffer call
> + * back.
> + */
> + if (!rc) {
> + spin_lock_irqsave(&drvdata->spinlock, flags);
> + if (WARN_ON(drvdata->perf_data))
> + rc = -EBUSY;
> + else
> + drvdata->perf_data = etr_perf;
> + spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + }
> + return rc;
> +}
> +
> +/*
> + * tmc_etr_sync_perf_buffer: Copy the actual trace data from the hardware
> + * buffer to the perf ring buffer.
> + */
> +static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf)
> +{
> + struct etr_buf *etr_buf = etr_perf->etr_buf;
> + long bytes, to_copy;
> + unsigned long head = etr_perf->head;
> + unsigned long pg_idx, pg_offset, src_offset;
> + char **dst_pages, *src_buf;
> +
> + head = PERF_IDX2OFF(etr_perf->head, etr_perf);
> + pg_idx = head >> PAGE_SHIFT;
> + pg_offset = head & (PAGE_SIZE - 1);
> + dst_pages = (char **)etr_perf->pages;
> + src_offset = etr_buf->offset;
> + to_copy = etr_buf->len;
> +
> + while (to_copy > 0) {
> + /*
> + * We can copy minimum of :
> + * 1) what is available in the source buffer,
> + * 2) what is available in the source buffer, before it
> + * wraps around.
> + * 3) what is available in the destination page.
> + * in one iteration.
> + */
> + bytes = tmc_etr_buf_get_data(etr_buf, src_offset, to_copy,
> + &src_buf);
> + if (WARN_ON_ONCE(bytes <= 0))
> + break;
> + if (PAGE_SIZE - pg_offset < bytes)
> + bytes = PAGE_SIZE - pg_offset;
> +
> + memcpy(dst_pages[pg_idx] + pg_offset, src_buf, bytes);
> + to_copy -= bytes;
> + /* Move destination pointers */
> + pg_offset += bytes;
> + if (pg_offset == PAGE_SIZE) {
> + pg_offset = 0;
> + if (++pg_idx == etr_perf->nr_pages)
> + pg_idx = 0;
> + }
> +
> + /* Move source pointers */
> + src_offset += bytes;
> + if (src_offset >= etr_buf->size)
> + src_offset -= etr_buf->size;
> + }
> +}
> +
> +/*
> + * XXX: What is the expected behavior here in the following cases ?
> + * 1) Full trace mode, without double buffering : What should be the size
> + * reported back when the buffer is full and has wrapped around. Ideally,
> + * we should report for the lost trace to make sure the "head" in the ring
> + * buffer comes back to the position as in the trace buffer, rather than
> + * returning "total size" of the buffer.
I agree with the above strategy as there isn't much else to do. But do we
actually have a DMA coherent ETR SG or CATU? From the documentation available
to me I don't see it for ERT SG and I don't have the one for CATU. My hope
would be to get an IP with an overflow interrupt _before_ one that is DMA
coherent.
> + * 2) In snapshot mode, should we always return "full buffer size" ?
Snapshot mode is currently broken, something I intend to fix shortly. Until
then and to follow what is done for other IPs I think it is best to return the
full size.
> + */
> +static unsigned long
> +tmc_etr_update_perf_buffer(struct coresight_device *csdev,
> + struct perf_output_handle *handle,
> + void *config)
> +{
> + bool double_buffer, lost = false;
> + unsigned long flags, offset, size = 0;
> + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct etr_perf_buffer *etr_perf = config;
> + struct etr_buf *etr_buf = etr_perf->etr_buf;
> +
> + double_buffer = (etr_perf->flags == 0);
> +
> + spin_lock_irqsave(&drvdata->spinlock, flags);
> + if (WARN_ON(drvdata->perf_data != etr_perf)) {
> + lost = true;
> + spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + goto out;
> + }
> +
> + CS_UNLOCK(drvdata->base);
> +
> + tmc_flush_and_stop(drvdata);
> +
> + tmc_sync_etr_buf(drvdata);
> + CS_UNLOCK(drvdata->base);
> + /* Reset perf specific data */
> + drvdata->perf_data = NULL;
> + spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +
> + offset = etr_buf->offset + etr_buf->len;
> + if (offset > etr_buf->size)
> + offset -= etr_buf->size;
> +
> + if (double_buffer) {
> + /*
> + * If we use software double buffering, update the ring buffer.
> + * And the size is what we have in the hardware buffer.
> + */
> + size = etr_buf->len;
> + tmc_etr_sync_perf_buffer(etr_perf);
> + } else {
> + /*
> + * If the hardware uses perf ring buffer the size of the data
> + * we have is from the old-head to the current head of the
> + * buffer. This also means in non-snapshot mode, we have lost
> + * one-full-buffer-size worth data, if the buffer wraps around.
> + */
> + unsigned long old_head;
> +
> + old_head = PERF_IDX2OFF(etr_perf->head, etr_perf);
> + size = (offset - old_head + etr_buf->size) % etr_buf->size;
> + }
> +
> + /*
> + * Update handle->head in snapshot mode. Also update the size to the
> + * hardware buffer size if there was an overflow.
> + */
> + if (etr_perf->snapshot) {
> + if (double_buffer)
> + handle->head += size;
> + else
> + handle->head = offset;
> + if (etr_buf->full)
> + size = etr_buf->size;
> + }
> +
> + lost |= etr_buf->full;
> +out:
> + if (lost)
> + perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
> + return size;
> +}
> +
> static int tmc_enable_etr_sink_perf(struct coresight_device *csdev)
> {
> - /* We don't support perf mode yet ! */
> - return -EINVAL;
> + int rc = 0;
> + unsigned long flags;
> + struct etr_perf_buffer *etr_perf;
> + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + spin_lock_irqsave(&drvdata->spinlock, flags);
> + /*
> + * There can be only one writer per sink in perf mode. If the sink
> + * is already open in SYSFS mode, we can't use it.
> + */
> + if (drvdata->mode != CS_MODE_DISABLED) {
> + rc = -EBUSY;
> + goto unlock_out;
> + }
> +
> + etr_perf = drvdata->perf_data;
> + if (WARN_ON(!etr_perf || !etr_perf->etr_buf)) {
> + rc = -EINVAL;
> + goto unlock_out;
> + }
> +
> + drvdata->mode = CS_MODE_PERF;
> + tmc_etr_enable_hw(drvdata, etr_perf->etr_buf);
> +
> +unlock_out:
> + spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + return rc;
> }
>
> static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode)
> @@ -1459,6 +1836,10 @@ static void tmc_disable_etr_sink(struct coresight_device *csdev)
> static const struct coresight_ops_sink tmc_etr_sink_ops = {
> .enable = tmc_enable_etr_sink,
> .disable = tmc_disable_etr_sink,
> + .alloc_buffer = tmc_etr_alloc_perf_buffer,
> + .update_buffer = tmc_etr_update_perf_buffer,
> + .set_buffer = tmc_etr_set_perf_buffer,
> + .free_buffer = tmc_etr_free_perf_buffer,
> };
>
> const struct coresight_ops tmc_etr_cs_ops = {
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 185dc12..aa42f5d 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -197,6 +197,7 @@ struct etr_buf {
> * @trigger_cntr: amount of words to store after a trigger.
> * @etr_caps: Bitmask of capabilities of the TMC ETR, inferred from the
> * device configuration register (DEVID)
> + * @perf_data: PERF buffer for ETR.
> * @sysfs_data: SYSFS buffer for ETR.
> */
> struct tmc_drvdata {
> @@ -218,6 +219,7 @@ struct tmc_drvdata {
> u32 trigger_cntr;
> u32 etr_caps;
> struct etr_buf *sysfs_buf;
> + void *perf_data;
> };
>
> struct etr_buf_operations {
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v2 23/27] coresight: tmc-etr: Handle driver mode specific ETR buffers
From: Suzuki K Poulose @ 2018-05-08 21:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180508171800.GA3389@xps15>
On 05/08/2018 06:18 PM, Mathieu Poirier wrote:
> On Tue, May 01, 2018 at 10:10:53AM +0100, Suzuki K Poulose wrote:
>> Since the ETR could be driven either by SYSFS or by perf, it
>> becomes complicated how we deal with the buffers used for each
>> of these modes. The ETR driver cannot simply free the current
>> attached buffer without knowing the provider (i.e, sysfs vs perf).
>>
>> To solve this issue, we provide:
>> 1) the driver-mode specific etr buffer to be retained in the drvdata
>> 2) the etr_buf for a session should be passed on when enabling the
>> hardware, which will be stored in drvdata->etr_buf. This will be
>> replaced (not free'd) as soon as the hardware is disabled, after
>> necessary sync operation.
>>
>> The advantages of this are :
>>
>> 1) The common code path doesn't need to worry about how to dispose
>> an existing buffer, if it is about to start a new session with a
>> different buffer, possibly in a different mode.
>> 2) The driver mode can control its buffers and can get access to the
>> saved session even when the hardware is operating in a different
>> mode. (e.g, we can still access a trace buffer from a sysfs mode
>> even if the etr is now used in perf mode, without disrupting the
>> current session.)
>>
>> Towards this, we introduce a sysfs specific data which will hold the
>> etr_buf used for sysfs mode of operation, controlled solely by the
>> sysfs mode handling code.
>
> Thinking further on this... I toyed with the idea of doing the same thing when
> working on the original driver and decided against it. Do we really have a case
> where users would want to use sysFS and perf alternatively? To me this looks
> overdesigned.
>
> If we are going to go that way we need to enact the same behavior for ETB10 and
> ETF... And take it out of this set as it is already substantial enough.
The difference between ETB10/ETF and ETR is the usage of the buffer. The
former uses an internal buffer and we always have to copy it out to an
external buffer for consumption. Now this external buffer is actually
separate for each mode, i.e sysfs and perf. Also the data is copied
out right after we disable the HW. This ensures that the interleaved
mode doesn't corrupt each others data.
However, the ETR doesn't have an internal buffer and uses the System
RAM. That brings in the problem of one mode using the "buffer" as
described by the drvdata. So, eventually either mode could write to
the buffer allocated by the other mode before it is consumed by the
end user (via syfs read or perf). That brings in the challenge of
managing the buffer safely, switching back and forth the buffer
(with the right size and pages) for each mode without any interferences.
That also implies, one mode must be able to free the left-over buffer
from the previous mode safely (which could be potentially linked to
other data structures maintained by the mode). And that makes it more
complex. e.g, we must leave the sysfs trace data for collection and
meanwhile the perf could grab the ETR for its usage. The perf mode
might not know the mode of the existing buffer and thus wouldn't know
how to free it properly.
This is why we need buffers per mode which can be managed by
each mode. i.e, both allocated, used and more importantly free'd
appropriately.
Cheers
Suzuki
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox