From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9B3B4C433DB for ; Mon, 21 Dec 2020 05:17:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 56D8F22CB2 for ; Mon, 21 Dec 2020 05:17:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725793AbgLUFRX (ORCPT ); Mon, 21 Dec 2020 00:17:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42310 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725308AbgLUFRX (ORCPT ); Mon, 21 Dec 2020 00:17:23 -0500 Received: from mail-pg1-x534.google.com (mail-pg1-x534.google.com [IPv6:2607:f8b0:4864:20::534]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2749FC061282 for ; Sun, 20 Dec 2020 21:16:43 -0800 (PST) Received: by mail-pg1-x534.google.com with SMTP id p18so5646672pgm.11 for ; Sun, 20 Dec 2020 21:16:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=FY6z7JqllprlHMBYTUwFN6QjJFrlD6KrEW0CH05eYAQ=; b=R5jqp/JCDiAl3vkcOvnBFYNZhIBwEVup8M88NBZ90iM2d+RsHW2tIJdsz4zEJP8HDV PGSN+FdwA08HUWTXjsSmVqkjGBjj/DFDSa+wnOLQ1hBtjYN3hcwDLRVccIDoSWZuCVzX yAkRD5CfFTxOyjAyh32CA5lpwzAv4CA7bHrl4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=FY6z7JqllprlHMBYTUwFN6QjJFrlD6KrEW0CH05eYAQ=; b=rDh5sKjfKecXvXkFSACWMRHJdW1QQoji+j+clirq7fgxAbTsm17WwY1i8lq1ah3vXQ rLtDumGGNXyLJ1PTmH1L84U+08D3GXXFPerzH5ZCmG89DT6qKn33jr7yPUwXr9z4tczv hRxygnNN8NvV4Eha7w5tb1V4FhtB42kCIoRDsKzB2OJKbp9y6XwHTVJLrqh/aNXlUAgA Vtm6bSX0vZcHTCLFIktfSPnPiRMm9e0XpnplUtLg72i4BNaRvhudte9BZkg4XG39eM6V S5wGurJiZ3vM79qEiwdAxnAhuts1jTX6esQvr07ejIeodGSK0d/5uitqHEnQhLP8jzwn M5cg== X-Gm-Message-State: AOAM533joYYY9wZOlxvzza6Bsa96BLQt5KzT3CnNE1c7ihu3NUiIxKKC atz5MGjbHHymPBHDKGbDp0HBVudQ4QdUNgRhO+HqnpZbURDFIQ== X-Google-Smtp-Source: ABdhPJwqJjAstIPLLxsW1pe9oXKXOonBFCoT1lAtap0NVVd+ocFlsOOdNe1FWwMq647hX6mxfUZuyRapZInbgS/mrlw= X-Received: by 2002:a1f:6743:: with SMTP id m3mr11694643vki.10.1608517992770; Sun, 20 Dec 2020 18:33:12 -0800 (PST) MIME-Version: 1.0 References: <1605700894-32699-1-git-send-email-hsin-hsiung.wang@mediatek.com> <1605700894-32699-3-git-send-email-hsin-hsiung.wang@mediatek.com> In-Reply-To: <1605700894-32699-3-git-send-email-hsin-hsiung.wang@mediatek.com> From: Nicolas Boichat Date: Mon, 21 Dec 2020 10:33:02 +0800 Message-ID: Subject: Re: [PATCH v4 2/5] soc: mediatek: pwrap: add arbiter capability To: Hsin-Hsiung Wang Cc: Rob Herring , Matthias Brugger , Fei Shao , Argus Lin , Devicetree List , srv_heupstream , lkml , "moderated list:ARM/Mediatek SoC support" , linux-arm Mailing List Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Wed, Nov 18, 2020 at 8:08 PM Hsin-Hsiung Wang wrote: > > Add arbiter capability for pwrap driver. > The arbiter capability uses new design to judge the priority and latency > for multi-channel. > This patch is preparing for adding mt6873/8192 pwrap support. > > Signed-off-by: Hsin-Hsiung Wang > --- > drivers/soc/mediatek/mtk-pmic-wrap.c | 57 ++++++++++++++++++++++++++++++------ > 1 file changed, 48 insertions(+), 9 deletions(-) > > diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c > index c897205..5678f46 100644 > --- a/drivers/soc/mediatek/mtk-pmic-wrap.c > +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c > @@ -25,10 +25,12 @@ > > /* macro for wrapper status */ > #define PWRAP_GET_WACS_RDATA(x) (((x) >> 0) & 0x0000ffff) > +#define PWRAP_GET_WACS_ARB_FSM(x) (((x) >> 1) & 0x00000007) > #define PWRAP_GET_WACS_FSM(x) (((x) >> 16) & 0x00000007) > #define PWRAP_GET_WACS_REQ(x) (((x) >> 19) & 0x00000001) > #define PWRAP_STATE_SYNC_IDLE0 BIT(20) > #define PWRAP_STATE_INIT_DONE0 BIT(21) > +#define PWRAP_STATE_INIT_DONE1 BIT(15) > > /* macro for WACS FSM */ > #define PWRAP_WACS_FSM_IDLE 0x00 > @@ -74,6 +76,7 @@ > #define PWRAP_CAP_DCM BIT(2) > #define PWRAP_CAP_INT1_EN BIT(3) > #define PWRAP_CAP_WDT_SRC1 BIT(4) > +#define PWRAP_CAP_ARB BIT(5) > > /* defines for slave device wrapper registers */ > enum dew_regs { > @@ -340,6 +343,8 @@ enum pwrap_regs { > PWRAP_DCM_DBC_PRD, > PWRAP_EINT_STA0_ADR, > PWRAP_EINT_STA1_ADR, > + PWRAP_SWINF_2_WDATA_31_0, > + PWRAP_SWINF_2_RDATA_31_0, > > /* MT2701 only regs */ > PWRAP_ADC_CMD_ADDR, > @@ -1108,14 +1113,22 @@ static void pwrap_writel(struct pmic_wrapper *wrp, u32 val, enum pwrap_regs reg) > > static bool pwrap_is_fsm_idle(struct pmic_wrapper *wrp) > { > - u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA); > + u32 val; > + > + val = pwrap_readl(wrp, PWRAP_WACS2_RDATA); > + if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) > + return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_IDLE; > > return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_IDLE; > } > > static bool pwrap_is_fsm_vldclr(struct pmic_wrapper *wrp) > { > - u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA); > + u32 val; > + > + val = pwrap_readl(wrp, PWRAP_WACS2_RDATA); > + if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) > + return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR; This code is now copied twice. Do you think it'd be better to create a new function? static u32 pwrap_get_fsm_state(struct pmic_wrapper *wrp) { if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) return PWRAP_GET_WACS_ARB_FSM(val); else return PWRAP_GET_WACS_FSM(val); } > > return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR; > } > @@ -1165,6 +1178,7 @@ static int pwrap_wait_for_state(struct pmic_wrapper *wrp, > static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata) > { > int ret; > + u32 val; > > ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle); > if (ret) { > @@ -1172,13 +1186,21 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata) > return ret; > } > > - pwrap_writel(wrp, (adr >> 1) << 16, PWRAP_WACS2_CMD); > + if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) > + val = adr; > + else > + val = (adr >> 1) << 16; > + pwrap_writel(wrp, val, PWRAP_WACS2_CMD); > > ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr); > if (ret) > return ret; > > - *rdata = PWRAP_GET_WACS_RDATA(pwrap_readl(wrp, PWRAP_WACS2_RDATA)); > + if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) > + val = pwrap_readl(wrp, PWRAP_SWINF_2_RDATA_31_0); > + else > + val = pwrap_readl(wrp, PWRAP_WACS2_RDATA); > + *rdata = PWRAP_GET_WACS_RDATA(val); > > pwrap_writel(wrp, 1, PWRAP_WACS2_VLDCLR); > > @@ -1228,8 +1250,13 @@ static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata) > return ret; > } > > - pwrap_writel(wrp, (1 << 31) | ((adr >> 1) << 16) | wdata, > - PWRAP_WACS2_CMD); > + if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) { > + pwrap_writel(wrp, wdata, PWRAP_SWINF_2_WDATA_31_0); > + pwrap_writel(wrp, BIT(29) | adr, PWRAP_WACS2_CMD); > + } else { > + pwrap_writel(wrp, BIT(31) | ((adr >> 1) << 16) | wdata, > + PWRAP_WACS2_CMD); > + } > > return 0; > } > @@ -2022,6 +2049,7 @@ MODULE_DEVICE_TABLE(of, of_pwrap_match_tbl); > static int pwrap_probe(struct platform_device *pdev) > { > int ret, irq; > + u32 mask_done; > struct pmic_wrapper *wrp; > struct device_node *np = pdev->dev.of_node; > const struct of_device_id *of_slave_id = NULL; > @@ -2116,14 +2144,21 @@ static int pwrap_probe(struct platform_device *pdev) > } > } > > - if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & PWRAP_STATE_INIT_DONE0)) { > + if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) > + mask_done = PWRAP_STATE_INIT_DONE1; > + else > + mask_done = PWRAP_STATE_INIT_DONE0; > + > + if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & mask_done)) { > dev_dbg(wrp->dev, "initialization isn't finished\n"); > ret = -ENODEV; > goto err_out2; > } > > /* Initialize watchdog, may not be done by the bootloader */ > - pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT); > + if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) > + pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT); > + To expand on Matthias' question on v3 (https://patchwork.kernel.org/project/linux-mediatek/patch/1600686235-27979-3-git-send-email-hsin-hsiung.wang@mediatek.com/): is there any PWRAP implementation where a design with an arbiter is still able to control the watchdog? If not, at the very least, it'd be good to expand the comment above (e.g. "designs with arbiter support cannot change the watchdog timer"). > /* > * Since STAUPD was not used on mt8173 platform, > * so STAUPD of WDT_SRC which should be turned off > @@ -2132,7 +2167,11 @@ static int pwrap_probe(struct platform_device *pdev) > if (HAS_CAP(wrp->master->caps, PWRAP_CAP_WDT_SRC1)) > pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN_1); > > - pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN); > + if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) Please invert this if test: if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) ... 0x3 ... else ... 0x1 ... > + pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN); > + else > + pwrap_writel(wrp, 0x3, PWRAP_TIMER_EN); > + > pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN); > /* > * We add INT1 interrupt to handle starvation and request exception > -- > 2.6.4 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek