Hi Nicolas,

Thank you for your comments.

On 27/10/20 3:44, Nicolas Boichat wrote:

On Tue, Oct 27, 2020 at 1:55 AM Enric Balletbo i Serra
[off-list ref] wrote:

quoted

From: Matthias Brugger <mbrugger@suse.com>

Apart from the infracfg block, the SMI block is used to enable the bus
protection for some power domains. Add support for this block.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Enric Balletbo i Serra <redacted>
---

Changes in v3:
- Do not reuse bpd for 2 different things.
- Disable pd->smi first and after that pd->infracfg.
- Rename BUT_PROT_UPDATE_MT8173 to BUS_PROT_UPDATE_TOPAXI as in all the
  other SoCs TOPAXI is mapped to the same address.

[snip]

quoted

 static int scpsys_power_on(struct generic_pm_domain *genpd)

@@ -266,6 +271,10 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
        if (IS_ERR(pd->infracfg))
                pd->infracfg = NULL;

+       pd->smi = syscon_regmap_lookup_by_phandle(node, "mediatek,smi");
+       if (IS_ERR(pd->smi))
+               pd->smi = NULL;

This is the second time I see this pattern, I think.

Do we want to create a new syscon_regmap_lookup_by_phandle_optional wrapper?

I think could be useful, yes. So I sent a patch to add this wrapper, let's see
what the MFD maintainers think.

Also, are we ok with ignoring all errors? I agree we can ignore
-ENODEV if optional, but I'm not sure about the others.

Right, we shouldn't ignore the other errors.

quoted

+
        pd->num_clks = of_clk_get_parent_count(node);
        if (pd->num_clks > 0) {
                pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);

diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index e428fe23a7e3..7b1abcfc4ba3 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h

@@ -34,8 +34,31 @@

 #define SPM_MAX_BUS_PROT_DATA          3

+#define _BUS_PROT(_mask, _set, _clr, _sta, _update) {  \
+               .bus_prot_mask = (_mask),               \
+               .bus_prot_set = _set,                   \
+               .bus_prot_clr = _clr,                   \
+               .bus_prot_sta = _sta,                   \
+               .bus_prot_reg_update = _update,         \
+       }
+
+#define BUS_PROT_WR(_mask, _set, _clr, _sta)           \
+               _BUS_PROT(_mask, _set, _clr, _sta, false)
+
+#define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)               \
+               _BUS_PROT(_mask, _set, _clr, _sta, true)
+
+#define BUS_PROT_UPDATE_TOPAXI(_mask)                          \
+               BUS_PROT_UPDATE(_mask,                          \
+                               INFRA_TOPAXI_PROTECTEN,         \
+                               INFRA_TOPAXI_PROTECTEN_CLR,     \
+                               INFRA_TOPAXI_PROTECTSTA1)
+
 struct scpsys_bus_prot_data {
        u32 bus_prot_mask;
+       u32 bus_prot_set;
+       u32 bus_prot_clr;
+       u32 bus_prot_sta;
        bool bus_prot_reg_update;
 };

@@ -47,6 +70,7 @@ struct scpsys_bus_prot_data {
  * @sram_pdn_ack_bits: The mask for sram power control acked bits.
  * @caps: The flag for active wake-up action.
  * @bp_infracfg: bus protection for infracfg subsystem
+ * @bp_smi: bus protection for smi subsystem
  */
 struct scpsys_domain_data {
        u32 sta_mask;

@@ -55,6 +79,7 @@ struct scpsys_domain_data {
        u32 sram_pdn_ack_bits;
        u8 caps;
        const struct scpsys_bus_prot_data bp_infracfg[SPM_MAX_BUS_PROT_DATA];
+       const struct scpsys_bus_prot_data bp_smi[SPM_MAX_BUS_PROT_DATA];
 };

 struct scpsys_soc_data {
--

2.28.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel