| Message ID | 1468573443-4670-8-git-send-email-anup.patel@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 15, 2016 at 02:34:02PM +0530, Anup Patel wrote: > +static int uio_dmem_genirq_alloc_platdata(struct platform_device *pdev) > +{ > + struct uio_dmem_genirq_pdata pdata; > + u32 dma_bits, regions; > + u32 sizes[MAX_UIO_MAPS]; > + int ret; > + > + memset(&pdata, 0, sizeof(pdata)); > + > + ret = of_property_read_u32(pdev->dev.of_node, > + uio_of_dma_bits_prop, &dma_bits); > + if (ret) { > + dev_err(&pdev->dev, > + "Missing property %s\n", uio_of_dma_bits_prop); > + return ret; > + } > + if (dma_bits > 64) > + dma_bits = 64; > + > + dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(dma_bits)); You really need to check the return value from this: this function negotiates with the architecture, and if 64-bit DMA is not supported, then the call will fail and you as a driver are expected to fall back to 32-bit DMA only. In that case, you're expected to call the same function with a 32-bit mask, and if that fails, you're supposed to then decide that DMA is not possible.
On Fri, Jul 15, 2016 at 3:15 PM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Fri, Jul 15, 2016 at 02:34:02PM +0530, Anup Patel wrote: >> +static int uio_dmem_genirq_alloc_platdata(struct platform_device *pdev) >> +{ >> + struct uio_dmem_genirq_pdata pdata; >> + u32 dma_bits, regions; >> + u32 sizes[MAX_UIO_MAPS]; >> + int ret; >> + >> + memset(&pdata, 0, sizeof(pdata)); >> + >> + ret = of_property_read_u32(pdev->dev.of_node, >> + uio_of_dma_bits_prop, &dma_bits); >> + if (ret) { >> + dev_err(&pdev->dev, >> + "Missing property %s\n", uio_of_dma_bits_prop); >> + return ret; >> + } >> + if (dma_bits > 64) >> + dma_bits = 64; >> + >> + dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(dma_bits)); > > You really need to check the return value from this: this function > negotiates with the architecture, and if 64-bit DMA is not supported, > then the call will fail and you as a driver are expected to fall back > to 32-bit DMA only. > > In that case, you're expected to call the same function with a 32-bit > mask, and if that fails, you're supposed to then decide that DMA is > not possible. > Thanks for pointing out. I will fix this as-per your suggestion in next revision. Regards, Anup
[adding devicetree list] On Fri, Jul 15, 2016 at 02:34:02PM +0530, Anup Patel wrote: > From: Jan Viktorin <viktorin@rehivetech.com> > > The uio_dmem_genirq works in a similar ways as uio_pdrv_genirq now. > > It accepts the of_id module parameter to specify UIO compatible > string as module parameter. There are few other module parameters > to specify DT property name for number bits in DMA mask and details > about dynamic regions. > > Following are the newly added module parameters: > 1) of_id: The UIO compatible string to be used for DT probing > 2) of_dma_bits_prot: This is name of OF property which will be > used to specify number of DMA mask bits in UIO DT node. > 3) of_dmem_count_prop: This is name of OF property which will be > used to specify number of dynamic regions in UIO DT node. > 4) of_dmem_sizes_prop: This is name of OF property which will be > used to specify sizes of each dynamic regions in UIO DT node. > > Signed-off-by: Jan Viktorin <viktorin@rehivetech.com> > Signed-off-by: Anup Patel <anup.patel@broadcom.com> > --- > drivers/uio/uio_dmem_genirq.c | 113 +++++++++++++++++++++++++++++++++--------- > 1 file changed, 89 insertions(+), 24 deletions(-) > > diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c > index a4d6d81..0a0cc19 100644 > --- a/drivers/uio/uio_dmem_genirq.c > +++ b/drivers/uio/uio_dmem_genirq.c > @@ -144,46 +144,107 @@ static int uio_dmem_genirq_irqcontrol(struct uio_info *dev_info, s32 irq_on) > return 0; > } > > +static char uio_of_dma_bits_prop[128] = "uio,dma-bits"; > +static char uio_of_dmem_count_prop[128] = "uio,number-of-dynamic-regions"; > +static char uio_of_dmem_sizes_prop[128] = "uio,dynamic-regions-sizes"; What are these proeprties? What is a "dynamic region" in hardware terms? Why must they be in the DT, and why are the *property names* arbitrarily overridable as module parameters? What exactly do you expect there to be in a DT? DT bindings need documentation, but there was none as part of this series. I do not think this makes sense from a DT perspective. Thanks, Mark. > + > +static int uio_dmem_genirq_alloc_platdata(struct platform_device *pdev) > +{ > + struct uio_dmem_genirq_pdata pdata; > + u32 dma_bits, regions; > + u32 sizes[MAX_UIO_MAPS]; > + int ret; > + > + memset(&pdata, 0, sizeof(pdata)); > + > + ret = of_property_read_u32(pdev->dev.of_node, > + uio_of_dma_bits_prop, &dma_bits); > + if (ret) { > + dev_err(&pdev->dev, > + "Missing property %s\n", uio_of_dma_bits_prop); > + return ret; > + } > + if (dma_bits > 64) > + dma_bits = 64; > + > + dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(dma_bits)); > + > + ret = of_property_read_u32(pdev->dev.of_node, > + uio_of_dmem_count_prop, ®ions); > + if (ret) { > + dev_err(&pdev->dev, > + "Missing property %s\n", uio_of_dmem_count_prop); > + return ret; > + } > + > + regions = min_t(u32, regions, MAX_UIO_MAPS); > + > + ret = of_property_read_u32_array(pdev->dev.of_node, > + uio_of_dmem_sizes_prop, sizes, > + regions); > + if (ret) { > + dev_err(&pdev->dev, "Missing or invalid property %s\n", > + uio_of_dmem_sizes_prop); > + return ret; > + } > + > + pdata.dynamic_region_sizes = devm_kzalloc(&pdev->dev, > + sizeof(*pdata.dynamic_region_sizes) * > + pdata.num_dynamic_regions, GFP_KERNEL); > + if (!pdata.dynamic_region_sizes) { > + dev_err(&pdev->dev, "Unable to alloc dynamic_region_sizes\n"); > + ret = -ENOMEM; > + return ret; > + } > + > + pdata.num_dynamic_regions = regions; > + while (regions--) > + pdata.dynamic_region_sizes[regions] = sizes[regions]; > + > + pdata.uioinfo.name = pdev->dev.of_node->name; > + pdata.uioinfo.version = "devicetree"; > + > + return platform_device_add_data(pdev, &pdata, sizeof(pdata)); > +} > + > static int uio_dmem_genirq_probe(struct platform_device *pdev) > { > - struct uio_dmem_genirq_pdata *pdata = dev_get_platdata(&pdev->dev); > - struct uio_info *uioinfo = &pdata->uioinfo; > + struct uio_dmem_genirq_pdata *pdata; > + struct uio_info *uioinfo; > struct uio_dmem_genirq_platdata *priv; > struct uio_mem *uiomem; > int ret = -EINVAL; > int i; > > if (pdev->dev.of_node) { > - /* alloc uioinfo for one device */ > - uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL); > - if (!uioinfo) { > - ret = -ENOMEM; > - dev_err(&pdev->dev, "unable to kmalloc\n"); > + ret = uio_dmem_genirq_alloc_platdata(pdev); > + if (ret) > goto bad2; > - } > - uioinfo->name = pdev->dev.of_node->name; > - uioinfo->version = "devicetree"; > } > > + pdata = dev_get_platdata(&pdev->dev); > + uioinfo = &pdata->uioinfo; > + > if (!uioinfo || !uioinfo->name || !uioinfo->version) { > dev_err(&pdev->dev, "missing platform_data\n"); > - goto bad0; > + goto bad2; > } > > if (uioinfo->handler || uioinfo->irqcontrol || > uioinfo->irq_flags & IRQF_SHARED) { > dev_err(&pdev->dev, "interrupt configuration error\n"); > - goto bad0; > + goto bad2; > } > > priv = kzalloc(sizeof(*priv), GFP_KERNEL); > if (!priv) { > ret = -ENOMEM; > dev_err(&pdev->dev, "unable to kmalloc\n"); > - goto bad0; > + goto bad2; > } > > - dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32)); > + if (!pdev->dev.of_node) > + dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32)); > > priv->uioinfo = uioinfo; > spin_lock_init(&priv->lock); > @@ -278,10 +339,6 @@ static int uio_dmem_genirq_probe(struct platform_device *pdev) > return 0; > bad1: > kfree(priv); > - bad0: > - /* kfree uioinfo for OF */ > - if (pdev->dev.of_node) > - kfree(uioinfo); > bad2: > return ret; > } > @@ -296,10 +353,6 @@ static int uio_dmem_genirq_remove(struct platform_device *pdev) > priv->uioinfo->handler = NULL; > priv->uioinfo->irqcontrol = NULL; > > - /* kfree uioinfo for OF */ > - if (pdev->dev.of_node) > - kfree(priv->uioinfo); > - > kfree(priv); > return 0; > } > @@ -327,10 +380,22 @@ static const struct dev_pm_ops uio_dmem_genirq_dev_pm_ops = { > }; > > #ifdef CONFIG_OF > -static const struct of_device_id uio_of_genirq_match[] = { > - { /* empty for now */ }, > +static struct of_device_id uio_of_genirq_match[] = { > + { /* This is filled with module_parm */ }, > + { /* end of list */ }, > }; > MODULE_DEVICE_TABLE(of, uio_of_genirq_match); > +module_param_string(of_id, uio_of_genirq_match[0].compatible, 128, 0); > +MODULE_PARM_DESC(of_id, "Openfirmware id of the device to be handled by uio"); > +module_param_string(of_dma_bits_prop, uio_of_dma_bits_prop, 128, 0); > +MODULE_PARM_DESC(of_dma_bits_prop, > + "Openfirmware property for number of bits in DMA mask"); > +module_param_string(of_dmem_count_prop, uio_of_dmem_count_prop, 128, 0); > +MODULE_PARM_DESC(of_dmem_count_prop, > + "Openfirmware property for dynamic region count"); > +module_param_string(of_dmem_sizes_prop, uio_of_dmem_sizes_prop, 128, 0); > +MODULE_PARM_DESC(of_dmem_sizes_prop, > + "Openfirmware property for dynamic region sizes"); > #endif > > static struct platform_driver uio_dmem_genirq = { > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Fri, Jul 15, 2016 at 6:58 PM, Mark Rutland <mark.rutland@arm.com> wrote: > [adding devicetree list] > > On Fri, Jul 15, 2016 at 02:34:02PM +0530, Anup Patel wrote: >> From: Jan Viktorin <viktorin@rehivetech.com> >> >> The uio_dmem_genirq works in a similar ways as uio_pdrv_genirq now. >> >> It accepts the of_id module parameter to specify UIO compatible >> string as module parameter. There are few other module parameters >> to specify DT property name for number bits in DMA mask and details >> about dynamic regions. >> >> Following are the newly added module parameters: >> 1) of_id: The UIO compatible string to be used for DT probing >> 2) of_dma_bits_prot: This is name of OF property which will be >> used to specify number of DMA mask bits in UIO DT node. >> 3) of_dmem_count_prop: This is name of OF property which will be >> used to specify number of dynamic regions in UIO DT node. >> 4) of_dmem_sizes_prop: This is name of OF property which will be >> used to specify sizes of each dynamic regions in UIO DT node. >> >> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com> >> Signed-off-by: Anup Patel <anup.patel@broadcom.com> >> --- >> drivers/uio/uio_dmem_genirq.c | 113 +++++++++++++++++++++++++++++++++--------- >> 1 file changed, 89 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c >> index a4d6d81..0a0cc19 100644 >> --- a/drivers/uio/uio_dmem_genirq.c >> +++ b/drivers/uio/uio_dmem_genirq.c >> @@ -144,46 +144,107 @@ static int uio_dmem_genirq_irqcontrol(struct uio_info *dev_info, s32 irq_on) >> return 0; >> } >> >> +static char uio_of_dma_bits_prop[128] = "uio,dma-bits"; >> +static char uio_of_dmem_count_prop[128] = "uio,number-of-dynamic-regions"; >> +static char uio_of_dmem_sizes_prop[128] = "uio,dynamic-regions-sizes"; > > What are these proeprties? What is a "dynamic region" in hardware terms? "Dynamic region" is DMAble memory alloced using dma_alloc_xxx() APIs for the device. For DMA-capable devices accessed from user-space, we need DMAble memory available to user-space. Sometime such devices are also cache-coherent (or IO-coherent) so in such case user-space will need cacheable DMAble memory. Number of "Dynamic regions" required by UIO dmem device depends on the type of HW device hence we describe number of "Dynamic regions" and size of "Dynamic regions" via DT attributes. > > Why must they be in the DT, and why are the *property names* arbitrarily > overridable as module parameters? What exactly do you expect there to be > in a DT? They must be in DT for platform devices created using DT probing. The number of "Dynamic regions" required by UIO device will depend on the nature of device. We cannot have same number and sizes of "Dynamic regions" for different UIO dmem devices. The UIO dmem driver is a generic driver and not specific to any type of HW. In fact, UIO dmem driver is generic enough to access variety of platform devices from user-space using UIO framework. Based on this Rob had previously suggested to not have any (or enforce any) DT bindings for UIO dmem driver. Refer, https://lkml.org/lkml/2016/5/18/457 Link to original Jan's patchset is https://lkml.org/lkml/2016/5/17/141 We had two options: 1) Get details of "Dynamic regions" via module parameter but this would mean using same number and sizes of "Dynamic regions" for all UIO dmem devices. 2) Pass names of DT attributes used by UIO dmem driver as module parameters. > > DT bindings need documentation, but there was none as part of this series. > > I do not think this makes sense from a DT perspective. > As explained above, we are not fixing DT compatible string and DT attributes names for UIO dmem driver instead we pass all these as module parameters (preferable via kernel args or at time of module insertion). Due to this we don't require DT bindings document. Regards, Anup
On Fri, Jul 15, 2016 at 09:57:28PM +0530, Anup Patel wrote: > On Fri, Jul 15, 2016 at 6:58 PM, Mark Rutland <mark.rutland@arm.com> wrote: > > On Fri, Jul 15, 2016 at 02:34:02PM +0530, Anup Patel wrote: > >> +static char uio_of_dma_bits_prop[128] = "uio,dma-bits"; > >> +static char uio_of_dmem_count_prop[128] = "uio,number-of-dynamic-regions"; > >> +static char uio_of_dmem_sizes_prop[128] = "uio,dynamic-regions-sizes"; > > > > What are these proeprties? What is a "dynamic region" in hardware terms? > > "Dynamic region" is DMAble memory alloced using dma_alloc_xxx() APIs > for the device. > > For DMA-capable devices accessed from user-space, we need DMAble > memory available to user-space. Sometime such devices are also > cache-coherent (or IO-coherent) so in such case user-space will need > cacheable DMAble memory. > > Number of "Dynamic regions" required by UIO dmem device depends > on the type of HW device hence we describe number of "Dynamic regions" > and size of "Dynamic regions" via DT attributes. So this is something akin to the number of DMA channels a device might use. Either that's implicit knowledge of the device (if fixed), which doesn't need to be in the DT, or a real property that should be described explicitly in the device binding, that has nothing to do with UIO. In general, a device which you might want to use with UIO may not require such properties, so this is imposing a requirement on DTBs purely for the sake of userspace software. > > Why must they be in the DT, and why are the *property names* arbitrarily > > overridable as module parameters? What exactly do you expect there to be > > in a DT? > > They must be in DT for platform devices created using DT probing. The > number of "Dynamic regions" required by UIO device will depend on the > nature of device. We cannot have same number and sizes of "Dynamic > regions" for different UIO dmem devices. If the devices are so different, they should not be handled by the same driver that has no knowledge of those differences. Platform VFIO has the same problem, and requires device-specific reset drivers. > The UIO dmem driver is a generic driver and not specific to any type of > HW. In fact, UIO dmem driver is generic enough to access variety of > platform devices from user-space using UIO framework. > > Based on this Rob had previously suggested to not have any (or enforce > any) DT bindings for UIO dmem driver. > > Refer, https://lkml.org/lkml/2016/5/18/457 I don't follow. Here Rob stated: DT describes h/w. UIO is not a h/w block, so this does not belong in DT. A UIO vs. kernel driver is purely a kernel decision which shouldn't require a DT change. The properties should be part of match data for a compatible string that needs them set. Or if they can be defined in a way that is actually a property of the h/w, then it would be acceptible. You'd still need to define compatible strings that the properties apply to. Above, you have come up with a default set of property names which you go looking for, and you expect a class of property, even if you don't expect specific names. That constitutes a binding, and a poorly-defined one at that. Per my reading, Rob's point in https://lkml.org/lkml/2016/5/23/539 was that providing the *values* as module parameters was fine, not providing the *property names*. > Link to original Jan's patchset is https://lkml.org/lkml/2016/5/17/141 > > We had two options: > 1) Get details of "Dynamic regions" via module parameter but this > would mean using same number and sizes of "Dynamic regions" for > all UIO dmem devices. > 2) Pass names of DT attributes used by UIO dmem driver as > module parameters. > > > DT bindings need documentation, but there was none as part of this series. > > > > I do not think this makes sense from a DT perspective. > > As explained above, we are not fixing DT compatible string and > DT attributes names for UIO dmem driver instead we pass all > these as module parameters (preferable via kernel args or at > time of module insertion). Due to this we don't require DT bindings > document. If you're looking for properties in the DT, you're assuming a binding of some sort, regardless of whether you expect particular names. To be clear, NAK to this as it stands. Thanks, Mark.
On Fri, Jul 15, 2016 at 10:22 PM, Mark Rutland <mark.rutland@arm.com> wrote: > On Fri, Jul 15, 2016 at 09:57:28PM +0530, Anup Patel wrote: >> On Fri, Jul 15, 2016 at 6:58 PM, Mark Rutland <mark.rutland@arm.com> wrote: >> > On Fri, Jul 15, 2016 at 02:34:02PM +0530, Anup Patel wrote: >> >> +static char uio_of_dma_bits_prop[128] = "uio,dma-bits"; >> >> +static char uio_of_dmem_count_prop[128] = "uio,number-of-dynamic-regions"; >> >> +static char uio_of_dmem_sizes_prop[128] = "uio,dynamic-regions-sizes"; >> > >> > What are these proeprties? What is a "dynamic region" in hardware terms? >> >> "Dynamic region" is DMAble memory alloced using dma_alloc_xxx() APIs >> for the device. >> >> For DMA-capable devices accessed from user-space, we need DMAble >> memory available to user-space. Sometime such devices are also >> cache-coherent (or IO-coherent) so in such case user-space will need >> cacheable DMAble memory. >> >> Number of "Dynamic regions" required by UIO dmem device depends >> on the type of HW device hence we describe number of "Dynamic regions" >> and size of "Dynamic regions" via DT attributes. > > So this is something akin to the number of DMA channels a device might > use. > > Either that's implicit knowledge of the device (if fixed), which doesn't > need to be in the DT, or a real property that should be described > explicitly in the device binding, that has nothing to do with UIO. > > In general, a device which you might want to use with UIO may not > require such properties, so this is imposing a requirement on DTBs > purely for the sake of userspace software. > >> > Why must they be in the DT, and why are the *property names* arbitrarily >> > overridable as module parameters? What exactly do you expect there to be >> > in a DT? >> >> They must be in DT for platform devices created using DT probing. The >> number of "Dynamic regions" required by UIO device will depend on the >> nature of device. We cannot have same number and sizes of "Dynamic >> regions" for different UIO dmem devices. > > If the devices are so different, they should not be handled by the same > driver that has no knowledge of those differences. Platform VFIO has the > same problem, and requires device-specific reset drivers. > >> The UIO dmem driver is a generic driver and not specific to any type of >> HW. In fact, UIO dmem driver is generic enough to access variety of >> platform devices from user-space using UIO framework. >> >> Based on this Rob had previously suggested to not have any (or enforce >> any) DT bindings for UIO dmem driver. >> >> Refer, https://lkml.org/lkml/2016/5/18/457 > > I don't follow. Here Rob stated: > > DT describes h/w. UIO is not a h/w block, so this does not belong in DT. A > UIO vs. kernel driver is purely a kernel decision which shouldn't > require a DT change. > > The properties should be part of match data for a compatible string that > needs them set. Or if they can be defined in a way that is actually a > property of the h/w, then it would be acceptible. You'd still need to > define compatible strings that the properties apply to. > > Above, you have come up with a default set of property names which you > go looking for, and you expect a class of property, even if you don't > expect specific names. That constitutes a binding, and a poorly-defined > one at that. > > Per my reading, Rob's point in https://lkml.org/lkml/2016/5/23/539 was > that providing the *values* as module parameters was fine, not providing > the *property names*. > >> Link to original Jan's patchset is https://lkml.org/lkml/2016/5/17/141 >> >> We had two options: >> 1) Get details of "Dynamic regions" via module parameter but this >> would mean using same number and sizes of "Dynamic regions" for >> all UIO dmem devices. >> 2) Pass names of DT attributes used by UIO dmem driver as >> module parameters. >> >> > DT bindings need documentation, but there was none as part of this series. >> > >> > I do not think this makes sense from a DT perspective. >> >> As explained above, we are not fixing DT compatible string and >> DT attributes names for UIO dmem driver instead we pass all >> these as module parameters (preferable via kernel args or at >> time of module insertion). Due to this we don't require DT bindings >> document. > > If you're looking for properties in the DT, you're assuming a binding of > some sort, regardless of whether you expect particular names. > > To be clear, NAK to this as it stands. I think if we are not enforcing any DT bindings by passing compatible and property names as module parameters then we don't need any DT bindings document. We can certainly remove default property names and enforce UIO dmem users to always pass this as module parameters. Anyways, you have already NAKed this so I will just get all dynamic region info from module parameters only. Regards, Anup
diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c index a4d6d81..0a0cc19 100644 --- a/drivers/uio/uio_dmem_genirq.c +++ b/drivers/uio/uio_dmem_genirq.c @@ -144,46 +144,107 @@ static int uio_dmem_genirq_irqcontrol(struct uio_info *dev_info, s32 irq_on) return 0; } +static char uio_of_dma_bits_prop[128] = "uio,dma-bits"; +static char uio_of_dmem_count_prop[128] = "uio,number-of-dynamic-regions"; +static char uio_of_dmem_sizes_prop[128] = "uio,dynamic-regions-sizes"; + +static int uio_dmem_genirq_alloc_platdata(struct platform_device *pdev) +{ + struct uio_dmem_genirq_pdata pdata; + u32 dma_bits, regions; + u32 sizes[MAX_UIO_MAPS]; + int ret; + + memset(&pdata, 0, sizeof(pdata)); + + ret = of_property_read_u32(pdev->dev.of_node, + uio_of_dma_bits_prop, &dma_bits); + if (ret) { + dev_err(&pdev->dev, + "Missing property %s\n", uio_of_dma_bits_prop); + return ret; + } + if (dma_bits > 64) + dma_bits = 64; + + dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(dma_bits)); + + ret = of_property_read_u32(pdev->dev.of_node, + uio_of_dmem_count_prop, ®ions); + if (ret) { + dev_err(&pdev->dev, + "Missing property %s\n", uio_of_dmem_count_prop); + return ret; + } + + regions = min_t(u32, regions, MAX_UIO_MAPS); + + ret = of_property_read_u32_array(pdev->dev.of_node, + uio_of_dmem_sizes_prop, sizes, + regions); + if (ret) { + dev_err(&pdev->dev, "Missing or invalid property %s\n", + uio_of_dmem_sizes_prop); + return ret; + } + + pdata.dynamic_region_sizes = devm_kzalloc(&pdev->dev, + sizeof(*pdata.dynamic_region_sizes) * + pdata.num_dynamic_regions, GFP_KERNEL); + if (!pdata.dynamic_region_sizes) { + dev_err(&pdev->dev, "Unable to alloc dynamic_region_sizes\n"); + ret = -ENOMEM; + return ret; + } + + pdata.num_dynamic_regions = regions; + while (regions--) + pdata.dynamic_region_sizes[regions] = sizes[regions]; + + pdata.uioinfo.name = pdev->dev.of_node->name; + pdata.uioinfo.version = "devicetree"; + + return platform_device_add_data(pdev, &pdata, sizeof(pdata)); +} + static int uio_dmem_genirq_probe(struct platform_device *pdev) { - struct uio_dmem_genirq_pdata *pdata = dev_get_platdata(&pdev->dev); - struct uio_info *uioinfo = &pdata->uioinfo; + struct uio_dmem_genirq_pdata *pdata; + struct uio_info *uioinfo; struct uio_dmem_genirq_platdata *priv; struct uio_mem *uiomem; int ret = -EINVAL; int i; if (pdev->dev.of_node) { - /* alloc uioinfo for one device */ - uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL); - if (!uioinfo) { - ret = -ENOMEM; - dev_err(&pdev->dev, "unable to kmalloc\n"); + ret = uio_dmem_genirq_alloc_platdata(pdev); + if (ret) goto bad2; - } - uioinfo->name = pdev->dev.of_node->name; - uioinfo->version = "devicetree"; } + pdata = dev_get_platdata(&pdev->dev); + uioinfo = &pdata->uioinfo; + if (!uioinfo || !uioinfo->name || !uioinfo->version) { dev_err(&pdev->dev, "missing platform_data\n"); - goto bad0; + goto bad2; } if (uioinfo->handler || uioinfo->irqcontrol || uioinfo->irq_flags & IRQF_SHARED) { dev_err(&pdev->dev, "interrupt configuration error\n"); - goto bad0; + goto bad2; } priv = kzalloc(sizeof(*priv), GFP_KERNEL); if (!priv) { ret = -ENOMEM; dev_err(&pdev->dev, "unable to kmalloc\n"); - goto bad0; + goto bad2; } - dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32)); + if (!pdev->dev.of_node) + dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32)); priv->uioinfo = uioinfo; spin_lock_init(&priv->lock); @@ -278,10 +339,6 @@ static int uio_dmem_genirq_probe(struct platform_device *pdev) return 0; bad1: kfree(priv); - bad0: - /* kfree uioinfo for OF */ - if (pdev->dev.of_node) - kfree(uioinfo); bad2: return ret; } @@ -296,10 +353,6 @@ static int uio_dmem_genirq_remove(struct platform_device *pdev) priv->uioinfo->handler = NULL; priv->uioinfo->irqcontrol = NULL; - /* kfree uioinfo for OF */ - if (pdev->dev.of_node) - kfree(priv->uioinfo); - kfree(priv); return 0; } @@ -327,10 +380,22 @@ static const struct dev_pm_ops uio_dmem_genirq_dev_pm_ops = { }; #ifdef CONFIG_OF -static const struct of_device_id uio_of_genirq_match[] = { - { /* empty for now */ }, +static struct of_device_id uio_of_genirq_match[] = { + { /* This is filled with module_parm */ }, + { /* end of list */ }, }; MODULE_DEVICE_TABLE(of, uio_of_genirq_match); +module_param_string(of_id, uio_of_genirq_match[0].compatible, 128, 0); +MODULE_PARM_DESC(of_id, "Openfirmware id of the device to be handled by uio"); +module_param_string(of_dma_bits_prop, uio_of_dma_bits_prop, 128, 0); +MODULE_PARM_DESC(of_dma_bits_prop, + "Openfirmware property for number of bits in DMA mask"); +module_param_string(of_dmem_count_prop, uio_of_dmem_count_prop, 128, 0); +MODULE_PARM_DESC(of_dmem_count_prop, + "Openfirmware property for dynamic region count"); +module_param_string(of_dmem_sizes_prop, uio_of_dmem_sizes_prop, 128, 0); +MODULE_PARM_DESC(of_dmem_sizes_prop, + "Openfirmware property for dynamic region sizes"); #endif static struct platform_driver uio_dmem_genirq = {