From: Weiyi Lu <weiyi.lu@mediatek.com>
To: Nicolas Boichat <drinkcat@chromium.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
Rob Herring <robh@kernel.org>,
Sascha Hauer <kernel@pengutronix.de>,
James Liao <jamesjj.liao@mediatek.com>,
Fan Chen <fan.chen@mediatek.com>,
"linux-arm Mailing List" <linux-arm-kernel@lists.infradead.org>,
lkml <linux-kernel@vger.kernel.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-mediatek@lists.infradead.org>,
srv_heupstream <srv_heupstream@mediatek.com>
Subject: Re: [PATCH v10 06/12] soc: mediatek: Use bp_table for all compatibles
Date: Fri, 20 Dec 2019 11:30:56 +0800 [thread overview]
Message-ID: <1576812656.8410.9.camel@mtksdaap41> (raw)
In-Reply-To: <CANMq1KCbmwY_nZTfZcbxYQm27CXdADD48RWQOx0JuTmGBn=y=g@mail.gmail.com>
On Thu, 2019-12-19 at 11:54 +0800, Nicolas Boichat wrote:
> On Wed, Dec 18, 2019 at 4:31 PM Weiyi Lu <weiyi.lu@mediatek.com> wrote:
> >
> > Only use bp_table for bus protection of all compatibles,
> > instead of mixing bus_prot_mask and bus_prot_reg_update.
>
> ditto, I'd just squash in the previous patch.
>
OK, I'll update in next version.
> > Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> > ---
> > drivers/soc/mediatek/mtk-scpsys.c | 94 ++++++++++++++++++++-------------------
> > 1 file changed, 48 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> > index 5699d9f..c438c53 100644
> > --- a/drivers/soc/mediatek/mtk-scpsys.c
> > +++ b/drivers/soc/mediatek/mtk-scpsys.c
> > @@ -11,7 +11,6 @@
> > #include <linux/platform_device.h>
> > #include <linux/pm_domain.h>
> > #include <linux/regulator/consumer.h>
> > -#include <linux/soc/mediatek/infracfg.h>
> > #include <linux/soc/mediatek/scpsys-ext.h>
> >
> > #include <dt-bindings/power/mt2701-power.h>
> > @@ -88,7 +87,6 @@
> > * @ctl_offs: The offset for main power control register.
> > * @sram_pdn_bits: The mask for sram power control bits.
> > * @sram_pdn_ack_bits: The mask for sram power control acked bits.
> > - * @bus_prot_mask: The mask for single step bus protection.
> > * @basic_clk_name: The basic clocks required by this power domain.
> > * @caps: The flag for active wake-up action.
> > * @bp_table: The mask table for multiple step bus protection.
> > @@ -99,7 +97,6 @@ struct scp_domain_data {
> > int ctl_offs;
> > u32 sram_pdn_bits;
> > u32 sram_pdn_ack_bits;
> > - u32 bus_prot_mask;
> > const char *basic_clk_name[MAX_CLKS];
> > u8 caps;
> > struct bus_prot bp_table[MAX_STEPS];
> > @@ -128,7 +125,6 @@ struct scp {
> > struct regmap *infracfg;
> > struct regmap *smi_common;
> > struct scp_ctrl_reg ctrl_reg;
> > - bool bus_prot_reg_update;
> > };
> >
> > struct scp_subdomain {
> > @@ -142,7 +138,6 @@ struct scp_soc_data {
> > const struct scp_subdomain *subdomains;
> > int num_subdomains;
> > const struct scp_ctrl_reg regs;
> > - bool bus_prot_reg_update;
> > };
> >
> > static int scpsys_domain_is_on(struct scp_domain *scpd)
> > @@ -256,12 +251,6 @@ static int scpsys_bus_protect_enable(struct scp_domain *scpd)
> > {
> > struct scp *scp = scpd->scp;
> >
> > - if (scpd->data->bus_prot_mask) {
> > - return mtk_infracfg_set_bus_protection(scp->infracfg,
> > - scpd->data->bus_prot_mask,
> > - scp->bus_prot_reg_update);
> > - }
> > -
> > return mtk_scpsys_ext_set_bus_protection(scpd->data->bp_table,
> > scp->infracfg, scp->smi_common);
> > }
> > @@ -270,12 +259,6 @@ static int scpsys_bus_protect_disable(struct scp_domain *scpd)
> > {
> > struct scp *scp = scpd->scp;
> >
> > - if (scpd->data->bus_prot_mask) {
> > - return mtk_infracfg_clear_bus_protection(scp->infracfg,
> > - scpd->data->bus_prot_mask,
> > - scp->bus_prot_reg_update);
> > - }
> > -
> > return mtk_scpsys_ext_clear_bus_protection(scpd->data->bp_table,
> > scp->infracfg, scp->smi_common);
> > }
> > @@ -412,8 +395,7 @@ static int init_basic_clks(struct platform_device *pdev, struct clk **clk,
> >
> > static struct scp *init_scp(struct platform_device *pdev,
> > const struct scp_domain_data *scp_domain_data, int num,
> > - const struct scp_ctrl_reg *scp_ctrl_reg,
> > - bool bus_prot_reg_update)
> > + const struct scp_ctrl_reg *scp_ctrl_reg)
> > {
> > struct genpd_onecell_data *pd_data;
> > struct resource *res;
> > @@ -427,8 +409,6 @@ static struct scp *init_scp(struct platform_device *pdev,
> > scp->ctrl_reg.pwr_sta_offs = scp_ctrl_reg->pwr_sta_offs;
> > scp->ctrl_reg.pwr_sta2nd_offs = scp_ctrl_reg->pwr_sta2nd_offs;
> >
> > - scp->bus_prot_reg_update = bus_prot_reg_update;
> > -
> > scp->dev = &pdev->dev;
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > @@ -549,8 +529,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .name = "conn",
> > .sta_mask = PWR_STATUS_CONN,
> > .ctl_offs = SPM_CONN_PWR_CON,
> > - .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
> > - MT2701_TOP_AXI_PROT_EN_CONN_S,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + BIT(2) | BIT(8), BIT(2) | BIT(8)),
> > + },
>
> I'm a bit sad we lose the information about the BIT meaning.
>
I'll keep those information in next version.
> Of course this looks ugly and verbose:
> BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> MT2701_TOP_AXI_PROT_EN_CONN_M |
> MT2701_TOP_AXI_PROT_EN_CONN_S,
> MT2701_TOP_AXI_PROT_EN_CONN_M |
> MT2701_TOP_AXI_PROT_EN_CONN_S),
>
> But if you make "check_clr_mask" a boolean, you wouldn't have to
> repeat the mask twice and you could keep the nice register bit
> definitions.
>
Thanks for the suggestion, I'll add "ignore_clr_ack" for it in next
version.
> [snip, many similar occurences below]
next prev parent reply other threads:[~2019-12-20 3:31 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-18 8:30 [PATCH v10 00/12] Mediatek MT8183 scpsys support Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 01/12] dt-bindings: mediatek: Add property to mt8183 smi-common Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 02/12] dt-bindings: soc: Add MT8183 power dt-bindings Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 03/12] soc: mediatek: Add basic_clk_name to scp_power_data Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 04/12] soc: mediatek: Use basic_clk_name for all compatibles Weiyi Lu
2019-12-19 3:48 ` Nicolas Boichat
2019-12-20 3:29 ` Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 05/12] soc: mediatek: Add multiple step bus protection control Weiyi Lu
2019-12-19 3:51 ` Nicolas Boichat
2019-12-20 3:30 ` Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 06/12] soc: mediatek: Use bp_table for all compatibles Weiyi Lu
2019-12-19 3:54 ` Nicolas Boichat
2019-12-20 3:30 ` Weiyi Lu [this message]
2019-12-18 8:30 ` [PATCH v10 07/12] soc: mediatek: Remove infracfg misc driver support Weiyi Lu
2019-12-19 3:56 ` Nicolas Boichat
2019-12-20 3:31 ` Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 08/12] soc: mediatek: Add subsys clock control for bus protection Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 09/12] soc: mediatek: Add extra sram control Weiyi Lu
2019-12-19 3:57 ` Nicolas Boichat
2019-12-18 8:30 ` [PATCH v10 10/12] soc: mediatek: Add MT8183 scpsys support Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 11/12] arm64: dts: Add power controller device node of MT8183 Weiyi Lu
2019-12-18 8:30 ` [PATCH v10 12/12] arm64: dts: Add power-domains properity to mfgcfg Weiyi Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1576812656.8410.9.camel@mtksdaap41 \
--to=weiyi.lu@mediatek.com \
--cc=drinkcat@chromium.org \
--cc=fan.chen@mediatek.com \
--cc=jamesjj.liao@mediatek.com \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=robh@kernel.org \
--cc=srv_heupstream@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).