Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1450314

Re: [PATCH 1/3] memory: mediatek: Add a new interface mtk_smi_larb_is_ready

From Yong Wu <yong.wu@mediatek.com>
Newsgroups linux.kernel
Subject Re: [PATCH 1/3] memory: mediatek: Add a new interface mtk_smi_larb_is_ready
Date 2016-07-26 03:40 +0200
Message-ID <rYZvH-3YA-3@gated-at.bofh.it> (permalink)
References <rWQ3v-4jO-7@gated-at.bofh.it> <rYJKi-2nt-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, 2016-07-25 at 10:39 +0200, Matthias Brugger wrote:
> 
> On 20/07/16 05:01, Yong Wu wrote:
> > Currently the iommu consumer always call iommu_present to get whether
> > the iommu is ready. But in MTK IOMMU, this function can't indicate
> > this. The IOMMU call bus_set_iommu->mtk_iommu_add_device->
> > mtk_iommu_attach_device to parse the iommu data, then it's able to
> > transfer "struct mtk_smi_iommu" to SMI-LARB, and the iommu uses the
> > larbs as compoents, the iommu will finish its probe until all the larbs
> > probe done.
> >
> > If the iommu consumer(like DRM) begin to probe after the time of
> > calling bus_set_iommu and before the time of SMI probe finish, it
> > will hang like this:
> >
> > [    7.832359] Call trace:
> > [    7.834778] [<ffffffc000764424>] mtk_smi_larb_get+0x24/0xa8
> > [    7.840300] [<ffffffc0005a1390>] mtk_drm_crtc_enable+0x6c/0x450
> >
> > Because the larb->mmu is NULL at that time.
> >
> > In order to avoid this issue, we add a new interface
> > (mtk_smi_larb_is_ready) for checking whether the IOMMU and SMI have
> > finished their probe. If it return false, the iommu consumer should
> > probe-defer for the IOMMU and SMI.
> >
> 
> Can't we just skip the functions in the probe and call bus_set_iommu 
> only if we were able to bind all components?
> Something like this:
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index c3043d8..0bef49b 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -649,10 +649,14 @@ static int mtk_iommu_probe(struct platform_device 
> *pdev)
>          if (ret)
>                  return ret;
> 
> +       ret = component_master_add_with_match(dev, &mtk_iommu_com_ops, 
> match);
> +       if (ret)
> +               return ret;
> +
>          if (!iommu_present(&platform_bus_type))
>                  bus_set_iommu(&platform_bus_type, &mtk_iommu_ops);
> 
> -       return component_master_add_with_match(dev, &mtk_iommu_com_ops, 
> match);
> +       return ret;
>   } 

Thanks very much for your suggestion, I have tried and this seems don't
work.

I don't know much about component, so add some logs.
                                                                
 component_master_add_with_match only add the ops into a list, it won't
wait for the smi-larb probe done. Below is the log, it will return
quickly. 

[    0.095073] mtk-iommu 10205000.iommu: begin to probe 
[    0.095922] mtk-iommu 10205000.iommu: probe hw init
[    0.095948] mtk-iommu 10205000.iommu: trying to bring up master
[    0.095967] mtk-iommu 10205000.iommu: master has incomplete
components

The binder callback(mtk_iommu_bind) is called in this backtrace:

[    0.555718] [<ffff00000849e6ec>] mtk_iommu_bind+0x14/0x48
[    0.556405] [<ffff00000850a860>] try_to_bring_up_master.part.6
+0x38/0x88
[    0.557255] [<ffff00000850b014>] component_add+0x158/0x208
[    0.557954] [<ffff0000087352a4>] mtk_smi_larb_probe+0xf8/0x134
[    0.558696] [<ffff000008511838>] platform_drv_probe+0x50/0xc8
[    0.559427] [<ffff00000850fcfc>] driver_probe_device+0x224/0x2c4
[    0.560191] [<ffff00000850ff08>] __device_attach_driver+0x98/0xc8
[    0.560966] [<ffff00000850dea0>] bus_for_each_drv+0x58/0x98
[    0.561676] [<ffff00000850fa3c>] __device_attach+0xbc/0x124
[    0.562385] [<ffff0000085100a4>] device_initial_probe+0x10/0x18
[    0.563138] [<ffff00000850ef48>] bus_probe_device+0x90/0x98
[    0.563848] [<ffff00000850f3e4>] deferred_probe_work_func+0x7c/0xb0

Thus, It looks like mtk_iommu_bind is called in mtk_smi_larb_probe which
will be delayed by power-domain.
then iommu_present is true even though M4U and SMI-larb have not
finished their component binding.

> 
>  
> 
>   static int mtk_iommu_remove(struct platform_device *pdev)
> 
> Regards,
> Matthias

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 1/3] memory: mediatek: Add a new interface mtk_smi_larb_is_ready Yong Wu <yong.wu@mediatek.com> - 2016-07-20 05:10 +0200
  [PATCH 3/3] vcodec: mediatek: Add probe-defer for MTK IOMMU and SMI Yong Wu <yong.wu@mediatek.com> - 2016-07-20 05:10 +0200
  [PATCH 2/3] drm/mediatek: Add probe-defer for MTK IOMMU and SMI Yong Wu <yong.wu@mediatek.com> - 2016-07-20 05:10 +0200
  Re: [PATCH 1/3] memory: mediatek: Add a new interface  mtk_smi_larb_is_ready Matthias Brugger <matthias.bgg@gmail.com> - 2016-07-25 10:50 +0200
    Re: [PATCH 1/3] memory: mediatek: Add a new interface  mtk_smi_larb_is_ready Yong Wu <yong.wu@mediatek.com> - 2016-07-26 03:40 +0200
    Re: [PATCH 1/3] memory: mediatek: Add a new interface mtk_smi_larb_is_ready Tomasz Figa <tfiga@google.com> - 2016-07-26 04:40 +0200

csiph-web