Re: [PATCH v7 2/2] soc: mediatek: add mt6779 devapc driver

[Neal Liu <neal.liu@mediatek.com>]

Hi Chun-Kuang,

On Sat, 2020-10-03 at 00:24 +0800, Chun-Kuang Hu wrote:
> Hi, Neal:
> 
> Neal Liu <neal.liu@mediatek.com> 於 2020年8月27日 週四 上午11:07寫道：
> >
> > MediaTek bus fabric provides TrustZone security support and data
> > protection to prevent slaves from being accessed by unexpected
> > masters.
> > The security violation is logged and sent to the processor for
> > further analysis or countermeasures.
> >
> > Any occurrence of security violation would raise an interrupt, and
> > it will be handled by mtk-devapc driver. The violation
> > information is printed in order to find the murderer.
> >
> > Signed-off-by: Neal Liu <neal.liu@mediatek.com>
> > ---
> >  drivers/soc/mediatek/Kconfig      |    9 ++
> >  drivers/soc/mediatek/Makefile     |    1 +
> >  drivers/soc/mediatek/mtk-devapc.c |  305 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 315 insertions(+)
> >  create mode 100644 drivers/soc/mediatek/mtk-devapc.c
> >
> 
> [snip]
> 
> > +
> > +static int mtk_devapc_probe(struct platform_device *pdev)
> > +{
> > +       struct device_node *node = pdev->dev.of_node;
> > +       struct mtk_devapc_context *ctx;
> > +       u32 devapc_irq;
> > +       int ret;
> > +
> > +       if (IS_ERR(node))
> > +               return -ENODEV;
> > +
> > +       ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
> > +       if (!ctx)
> > +               return -ENOMEM;
> > +
> > +       ctx->data = of_device_get_match_data(&pdev->dev);
> > +       ctx->dev = &pdev->dev;
> > +
> > +       ctx->infra_base = of_iomap(node, 0);
> > +       if (!ctx->infra_base)
> > +               return -EINVAL;
> > +
> > +       devapc_irq = irq_of_parse_and_map(node, 0);
> > +       if (!devapc_irq)
> > +               return -EINVAL;
> > +
> > +       ctx->infra_clk = devm_clk_get(&pdev->dev, "devapc-infra-clock");
> > +       if (IS_ERR(ctx->infra_clk))
> > +               return -EINVAL;
> > +
> > +       if (clk_prepare_enable(ctx->infra_clk))
> > +               return -EINVAL;
> 
> What would happen if you do not enable this clock? I think this
> hardware is already initialized in trust zone.
> 
> Regards,
> Chun-Kuang.

It cannot handle violation if the clock is disabled.
This parts of hardware is not initialized in TrustZone.

The another parts of hardware is initialized in TrustZone which is
responsible for permission control. I think that is the part what you
intend to express.

-Neal

> 
> > +
> > +       ret = devm_request_irq(&pdev->dev, devapc_irq,
> > +                              (irq_handler_t)devapc_violation_irq,
> > +                              IRQF_TRIGGER_NONE, "devapc", ctx);
> > +       if (ret) {
> > +               clk_disable_unprepare(ctx->infra_clk);
> > +               return ret;
> > +       }
> > +
> > +       platform_set_drvdata(pdev, ctx);
> > +
> > +       start_devapc(ctx);
> > +
> > +       return 0;
> > +}
> > +