Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2013-11-19 at 16:11 +0800, Li Zhong wrote:
> I encountered following issue:
> [    0.283035] ibmvscsi 30000015: couldn't initialize event pool
> [    5.688822] ibmvscsi: probe of 30000015 failed with error -1
> 
> which prevents the storage from being recognized, and the machine from
> booting.
> 
> After some digging, it seems that it is caused by commit 4886c399da
> 
> as dma_mask pointer in viodev->dev is not set, so in
> dma_set_mask_and_coherent(), dma_set_coherent_mask() is not called
> because dma_set_mask(), which is dma_set_mask_pSeriesLP() returned EIO.
> While before the commit, dma_set_coherent_mask() is always called. 
> 
> I tried to replace dma_set_mask_and_coherent() with
> dma_coerce_mask_and_coherent(), and the machine could boot again. 
> 
> But I'm not sure whether this is the correct fix...

Russell, care to chime in ? I can't make sense of the semantics...

The original commit was fairly clear:

<<
    Replace the following sequence:
    
    	dma_set_mask(dev, mask);
    	dma_set_coherent_mask(dev, mask);
    
    with a call to the new helper dma_set_mask_and_coherent().
>>

It all makes sense so far ... but doesn't work for some odd reason,
and the "fix" uses a function whose name doesn't make much sense to
me ... what is the difference between "setting" and "coercing"
the mask ? And why doe replacing two "set" with a "set both" doesn't
work and require a coerce ?

I'm asking because I'm worried about breakage elsewhere...

Cheers,
Ben. 

> ---
>  arch/powerpc/kernel/vio.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
> index e7d0c88..76a6482 100644
> --- a/arch/powerpc/kernel/vio.c
> +++ b/arch/powerpc/kernel/vio.c
> @@ -1419,7 +1419,7 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
>  
>  		/* needed to ensure proper operation of coherent allocations
>  		 * later, in case driver doesn't set it explicitly */
> -		dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
> +		dma_coerce_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
>  	}
>  
>  	/* register with generic device framework */
> 
> 
> 
>