Linux Virtualization - Jun 2019 - [PATCH v3 2/8] s390/cio: introduce DMA pools to cio

On Mon, 3 Jun 2019 14:09:02 +0200
Michael Mueller <mimu at linux.ibm.com> wrote:
> >> @@ -224,6 +226,8 @@ struct subchannel *css_alloc_subchannel(struct
subchannel_id schid,
> >>   	INIT_WORK(&sch->todo_work, css_sch_todo);
> >>   	sch->dev.release = &css_subchannel_release;
> >>   	device_initialize(&sch->dev);  
> > 
> > It might be helpful to add a comment why you use 31 bit here...  
> 
> @Halil, please let me know what comment you prefere here...
> 
How about?

/*
 * The physical addresses of some the dma structures that
 * can belong  to a subchannel need to fit 31 bit width (examples ccw,).
 */

> >   
> >> +	sch->dev.coherent_dma_mask = DMA_BIT_MASK(31);
> >> +	sch->dev.dma_mask = &sch->dev.coherent_dma_mask;
> >>   	return sch;
> >>   
> >>   err:
> >> @@ -899,6 +903,8 @@ static int __init setup_css(int nr)
> >>   	dev_set_name(&css->device, "css%x", nr);
> >>   	css->device.groups = cssdev_attr_groups;
> >>   	css->device.release = channel_subsystem_release;  
> > 
> > ...and 64 bit here.  
> 
> and here.
/*
 * We currently allocate notifier bits with this (using css->device
 * as the device argument with the DMA API), and are fine with 64 bit
 * addresses.
 */

Regards,
Halil

On Mon, 3 Jun 2019 14:57:30 +0200
Halil Pasic <pasic at linux.ibm.com> wrote:
> On Mon, 3 Jun 2019 14:09:02 +0200
> Michael Mueller <mimu at linux.ibm.com> wrote:
> 
> > >> @@ -224,6 +226,8 @@ struct subchannel
*css_alloc_subchannel(struct subchannel_id schid,
> > >>   	INIT_WORK(&sch->todo_work, css_sch_todo);
> > >>   	sch->dev.release = &css_subchannel_release;
> > >>   	device_initialize(&sch->dev);    
> > > 
> > > It might be helpful to add a comment why you use 31 bit here...
> > 
> > @Halil, please let me know what comment you prefere here...
> >   
> 
> How about?
> 
> /*
>  * The physical addresses of some the dma structures that
>  * can belong  to a subchannel need to fit 31 bit width (examples ccw,).
>  */
"e.g. ccw"?
> 
> 
> > >     
> > >> +	sch->dev.coherent_dma_mask = DMA_BIT_MASK(31);
> > >> +	sch->dev.dma_mask = &sch->dev.coherent_dma_mask;
> > >>   	return sch;
> > >>   
> > >>   err:
> > >> @@ -899,6 +903,8 @@ static int __init setup_css(int nr)
> > >>   	dev_set_name(&css->device, "css%x", nr);
> > >>   	css->device.groups = cssdev_attr_groups;
> > >>   	css->device.release = channel_subsystem_release;    
> > > 
> > > ...and 64 bit here.    
> > 
> > and here.  
> 
> /*
>  * We currently allocate notifier bits with this (using css->device
>  * as the device argument with the DMA API), and are fine with 64 bit
>  * addresses.
>  */
Thanks, that makes things hopefully clearer if we look at it some time
in the future ;)

On 03.06.19 15:34, Cornelia Huck wrote:
> On Mon, 3 Jun 2019 14:57:30 +0200
> Halil Pasic <pasic at linux.ibm.com> wrote:
> 
>> On Mon, 3 Jun 2019 14:09:02 +0200
>> Michael Mueller <mimu at linux.ibm.com> wrote:
>>
>>>>> @@ -224,6 +226,8 @@ struct subchannel
*css_alloc_subchannel(struct subchannel_id schid,
>>>>>    	INIT_WORK(&sch->todo_work, css_sch_todo);
>>>>>    	sch->dev.release = &css_subchannel_release;
>>>>>    	device_initialize(&sch->dev);
>>>>
>>>> It might be helpful to add a comment why you use 31 bit here...
>>>
>>> @Halil, please let me know what comment you prefere here...
>>>    
>>
>> How about?
>>
>> /*
>>   * The physical addresses of some the dma structures that
>>   * can belong  to a subchannel need to fit 31 bit width (examples
ccw,).
>>   */
> 
> "e.g. ccw"?
> 
>>
>>
>>>>      
>>>>> +	sch->dev.coherent_dma_mask = DMA_BIT_MASK(31);
>>>>> +	sch->dev.dma_mask =
&sch->dev.coherent_dma_mask;
>>>>>    	return sch;
>>>>>    
>>>>>    err:
>>>>> @@ -899,6 +903,8 @@ static int __init setup_css(int nr)
>>>>>    	dev_set_name(&css->device, "css%x",
nr);
>>>>>    	css->device.groups = cssdev_attr_groups;
>>>>>    	css->device.release = channel_subsystem_release;
>>>>
>>>> ...and 64 bit here.
>>>
>>> and here.
>>
>> /*
>>   * We currently allocate notifier bits with this (using css->device
>>   * as the device argument with the DMA API), and are fine with 64 bit
>>   * addresses.
>>   */
> 
> Thanks, that makes things hopefully clearer if we look at it some time
> in the future ;)
> 
Applied both with with requested change.

Michael