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