On Tue, Jun 5, 2018 at 5:45 PM Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> wrote: > > Cc: Arnd > > On 06/05/2018 07:08 AM, Tomasz Figa wrote: > > On Mon, Jun 4, 2018 at 10:56 PM Stanimir Varbanov > > <stanimir.varbanov@xxxxxxxxxx> wrote: > >> > >> Hi Tomasz, > >> > >> On 06/04/2018 04:18 PM, Tomasz Figa wrote: > >>> Hi Vikash, > >>> > >>> On Sat, Jun 2, 2018 at 5:27 AM Vikash Garodia <vgarodia@xxxxxxxxxxxxxx> wrote: > >>>> +static int __init venus_init(void) > >>>> +{ > >>>> + int ret; > >>>> + > >>>> + ret = platform_driver_register(&qcom_video_firmware_driver); > >>>> + if (ret) > >>>> + return ret; > >>> > >>> Do we really need this firmware driver? As far as I can see, the > >>> approach used here should work even without any driver bound to the > >>> firmware device. > >> > >> We need device/driver bind because we need to call dma_configure() which > >> internally doing iommus sID parsing. > > > > I can see some drivers calling of_dma_configure() directly: > > https://elixir.bootlin.com/linux/latest/ident/of_dma_configure > > > > I'm not sure if it's more elegant, but should at least require less code. > > I think that in this case of non-TZ where we do iommu mapping by hand we > can use shared-dma-pool reserved memory see how venus_boot has been > implemented in the beginning [1]. I might have misunderstood something, but wasn't the shared-dma-pool about reserving physical memory, while the venus firmware problem is about reserving certain range of IOVA? > > Arnd what do you think? > > Some background, we have a use-case where the memory for firmware needs > to be mapped by the venus driver by hand instead of TZ firmware calls. > I.e. we want to support both, iommu mapping from the driver and mapping > done by TZ firmware. How we will differentiate what mapping (TZ or > non-TZ) will be used is a separate issue. > > > > > By the way, can we really assume that probe of firmware platform > > device really completes before we call venus_boot()? > > I'd say we cannot. Looking at current implementation in driver core, of_platform_populate() would actually trigger a synchronous probe, so I guess it could work. However, I'm not sure if this is a general guarantee here or it's an implementation detail that shouldn't be relied on. If we end up really need to have this platform_driver, I guess we could call platform_driver_probe() after of_platform_populate(), rather than pre-registering the driver. That seems to be the way to ensure that the probe is synchronous and we can also check that a matching device was found by the return value. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html