Re: [PATCH 1/1] soc: qcom: geni: Provide parameter error checking

[Bjorn Andersson]

On Tue 03 Sep 06:50 PDT 2019, Lee Jones wrote:

> When booting with ACPI, the Geni Serial Engine is not set as the I2C/SPI
> parent and thus, the wrapper (parent device) is unassigned.  This causes
> the kernel to crash with a null dereference error.
> 

Now I see what you did in 8bc529b25354; i.e. stubbed all the other calls
between the SE and wrapper.

Do you think it would be possible to resolve the _DEP link to QGP[01]
somehow? For the clocks workarounds this could be resolved by us
representing that relationship using device_link and just rely on
pm_runtime to propagate the clock state.

For the DMA operation, iiuc it's the wrapper that implements the DMA
engine involved, but I'm guessing the main reason for mapping buffers on
the wrapper is so that it ends up being associated with the iommu
context of the wrapper.

Are the SMMU contexts at all represented in the ACPI world and if so do
you know how the wrapper vs SEs are bound to contexts? Can we map on
se->dev when wrapper is NULL (or perhaps always?)?

Regards,
Bjorn

> Fixes: 8bc529b25354 ("soc: qcom: geni: Add support for ACPI")
> Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx>
> ---
> Since we are already at -rc7 this patch should be processed ASAP - thank you.
> 
> drivers/soc/qcom/qcom-geni-se.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index d5cf953b4337..7d622ea1274e 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -630,6 +630,9 @@ int geni_se_tx_dma_prep(struct geni_se *se, void *buf, size_t len,
>  	struct geni_wrapper *wrapper = se->wrapper;
>  	u32 val;
>  
> +	if (!wrapper)
> +		return -EINVAL;
> +
>  	*iova = dma_map_single(wrapper->dev, buf, len, DMA_TO_DEVICE);
>  	if (dma_mapping_error(wrapper->dev, *iova))
>  		return -EIO;
> @@ -663,6 +666,9 @@ int geni_se_rx_dma_prep(struct geni_se *se, void *buf, size_t len,
>  	struct geni_wrapper *wrapper = se->wrapper;
>  	u32 val;
>  
> +	if (!wrapper)
> +		return -EINVAL;
> +
>  	*iova = dma_map_single(wrapper->dev, buf, len, DMA_FROM_DEVICE);
>  	if (dma_mapping_error(wrapper->dev, *iova))
>  		return -EIO;
> -- 
> 2.17.1
>