| Message ID | 1398353407-2345-5-git-send-email-santosh.shilimkar@ti.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 24 Apr 2014 11:30:04 -0400, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > Retrieve DMA configuration from DT and setup platform device's DMA > parameters. The DMA configuration in DT has to be specified using > "dma-ranges" and "dma-coherent" properties if supported. > > We setup dma_pfn_offset using "dma-ranges" and dma_coherent_ops > using "dma-coherent" device tree properties. > > The set_arch_dma_coherent_ops macro has to be defined by arch if > it supports coherent dma_ops. Otherwise, set_arch_dma_coherent_ops() is > declared as nop. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Olof Johansson <olof@lixom.net> > Cc: Grant Likely <grant.likely@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > drivers/of/platform.c | 48 ++++++++++++++++++++++++++++++++++++++++--- > include/linux/dma-mapping.h | 7 +++++++ > 2 files changed, 52 insertions(+), 3 deletions(-) > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 48de98f..270c0b9 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -187,6 +187,50 @@ struct platform_device *of_device_alloc(struct device_node *np, > EXPORT_SYMBOL(of_device_alloc); > > /** > + * of_dma_configure - Setup DMA configuration > + * @dev: Device to apply DMA configuration > + * > + * Try to get devices's DMA configuration from DT and update it > + * accordingly. > + * > + * In case if platform code need to use own special DMA configuration,it > + * can use Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event > + * to fix up DMA configuration. > + */ > +static void of_dma_configure(struct device *dev) > +{ > + u64 dma_addr, paddr, size; > + int ret; > + > + dev->coherent_dma_mask = DMA_BIT_MASK(32); > + if (!dev->dma_mask) > + dev->dma_mask = &dev->coherent_dma_mask; > + > + /* > + * if dma-coherent property exist, call arch hook to setup > + * dma coherent operations. > + */ > + if (of_dma_is_coherent(dev->of_node)) { > + set_arch_dma_coherent_ops(dev); > + dev_dbg(dev, "device is dma coherent\n"); > + } > + > + /* > + * if dma-ranges property doesn't exist - just return else > + * setup the dma offset > + */ > + ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); > + if ((ret == -ENODEV) || (ret < 0)) { > + dev_dbg(dev, "no dma range information to setup\n"); > + return; > + } > + > + /* DMA ranges found. Calculate and set dma_pfn_offset */ > + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr); > + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset); I've got two concerns here. of_dma_get_range() retrieves only the first tuple from the dma-ranges property, but it is perfectly valid for dma-ranges to contain multiple tuples. How should we handle it if a device has multiple ranges it can DMA from? Second, while the pfn offset is being determined, I don't see anything making use of either the base address or size. How is the device constrained to only getting DMA buffers from within that range? Is the driver expected to manage that directly? g. > +} > + > +/** > * of_platform_device_create_pdata - Alloc, initialize and register an of_device > * @np: pointer to node to create device for > * @bus_id: name to assign device > @@ -214,9 +258,7 @@ static struct platform_device *of_platform_device_create_pdata( > #if defined(CONFIG_MICROBLAZE) > dev->archdata.dma_mask = 0xffffffffUL; > #endif > - dev->dev.coherent_dma_mask = DMA_BIT_MASK(32); > - if (!dev->dev.dma_mask) > - dev->dev.dma_mask = &dev->dev.coherent_dma_mask; > + of_dma_configure(&dev->dev); > dev->dev.bus = &platform_bus_type; > dev->dev.platform_data = platform_data; > > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h > index fd4aee2..c7d9b1b 100644 > --- a/include/linux/dma-mapping.h > +++ b/include/linux/dma-mapping.h > @@ -123,6 +123,13 @@ static inline int dma_coerce_mask_and_coherent(struct device *dev, u64 mask) > > extern u64 dma_get_required_mask(struct device *dev); > > +#ifndef set_arch_dma_coherent_ops > +static inline int set_arch_dma_coherent_ops(struct device *dev) > +{ > + return 0; > +} > +#endif > + > static inline unsigned int dma_get_max_seg_size(struct device *dev) > { > return dev->dma_parms ? dev->dma_parms->max_segment_size : 65536; > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Grant, On Tuesday 29 April 2014 10:41 AM, Grant Likely wrote: > On Thu, 24 Apr 2014 11:30:04 -0400, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: >> Retrieve DMA configuration from DT and setup platform device's DMA >> parameters. The DMA configuration in DT has to be specified using >> "dma-ranges" and "dma-coherent" properties if supported. >> >> We setup dma_pfn_offset using "dma-ranges" and dma_coherent_ops >> using "dma-coherent" device tree properties. >> >> The set_arch_dma_coherent_ops macro has to be defined by arch if >> it supports coherent dma_ops. Otherwise, set_arch_dma_coherent_ops() is >> declared as nop. >> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Russell King <linux@arm.linux.org.uk> >> Cc: Arnd Bergmann <arnd@arndb.de> >> Cc: Olof Johansson <olof@lixom.net> >> Cc: Grant Likely <grant.likely@linaro.org> >> Cc: Rob Herring <robh+dt@kernel.org> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Linus Walleij <linus.walleij@linaro.org> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- >> drivers/of/platform.c | 48 ++++++++++++++++++++++++++++++++++++++++--- >> include/linux/dma-mapping.h | 7 +++++++ >> 2 files changed, 52 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/of/platform.c b/drivers/of/platform.c >> index 48de98f..270c0b9 100644 >> --- a/drivers/of/platform.c >> +++ b/drivers/of/platform.c >> @@ -187,6 +187,50 @@ struct platform_device *of_device_alloc(struct device_node *np, >> EXPORT_SYMBOL(of_device_alloc); >> >> /** >> + * of_dma_configure - Setup DMA configuration >> + * @dev: Device to apply DMA configuration >> + * >> + * Try to get devices's DMA configuration from DT and update it >> + * accordingly. >> + * >> + * In case if platform code need to use own special DMA configuration,it >> + * can use Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event >> + * to fix up DMA configuration. >> + */ >> +static void of_dma_configure(struct device *dev) >> +{ >> + u64 dma_addr, paddr, size; >> + int ret; >> + >> + dev->coherent_dma_mask = DMA_BIT_MASK(32); >> + if (!dev->dma_mask) >> + dev->dma_mask = &dev->coherent_dma_mask; >> + >> + /* >> + * if dma-coherent property exist, call arch hook to setup >> + * dma coherent operations. >> + */ >> + if (of_dma_is_coherent(dev->of_node)) { >> + set_arch_dma_coherent_ops(dev); >> + dev_dbg(dev, "device is dma coherent\n"); >> + } >> + >> + /* >> + * if dma-ranges property doesn't exist - just return else >> + * setup the dma offset >> + */ >> + ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); >> + if ((ret == -ENODEV) || (ret < 0)) { >> + dev_dbg(dev, "no dma range information to setup\n"); >> + return; >> + } >> + >> + /* DMA ranges found. Calculate and set dma_pfn_offset */ >> + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr); >> + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset); > > I've got two concerns here. of_dma_get_range() retrieves only the first > tuple from the dma-ranges property, but it is perfectly valid for > dma-ranges to contain multiple tuples. How should we handle it if a > device has multiple ranges it can DMA from? > We've not found any cases in current Linux where more than one dma-ranges would be used. Moreover, The MM (definitely for ARM) isn't supported such cases at all (if i understand everything right). - there are only one arm_dma_pfn_limit - there is only one MM zone is used for ARM - some arches like x86,mips can support 2 zones (per arch - not per device or bus) DMA & DMA32, but they configured once and forever per arch. Example: static void *loongson_dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle, gfp_t gfp, struct dma_attrs *attrs) { ... /* ignore region specifiers */ gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM); #ifdef CONFIG_ISA if (dev == NULL) gfp |= __GFP_DMA; else #endif #ifdef CONFIG_ZONE_DMA if (dev->coherent_dma_mask < DMA_BIT_MASK(32)) gfp |= __GFP_DMA; else #endif #ifdef CONFIG_ZONE_DMA32 if (dev->coherent_dma_mask < DMA_BIT_MASK(40)) gfp |= __GFP_DMA32; else #endif ... } Any ways, it can be added later if we have an usecase for that. > Second, while the pfn offset is being determined, I don't see anything > making use of either the base address or size. How is the device > constrained to only getting DMA buffers from within that range? Is the > driver expected to manage that directly? > Drivers don't have to do anything special apart from setting the correct mask. The pfn_offset case, we use DMA_ZONE which takes care of masks already. Size is suppose to be used for dma_mask setup which we discussed in previous threads. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 30 Apr 2014 10:19:15 -0400, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > Hi Grant, > > On Tuesday 29 April 2014 10:41 AM, Grant Likely wrote: > > On Thu, 24 Apr 2014 11:30:04 -0400, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > >> Retrieve DMA configuration from DT and setup platform device's DMA > >> parameters. The DMA configuration in DT has to be specified using > >> "dma-ranges" and "dma-coherent" properties if supported. > >> > >> We setup dma_pfn_offset using "dma-ranges" and dma_coherent_ops > >> using "dma-coherent" device tree properties. > >> > >> The set_arch_dma_coherent_ops macro has to be defined by arch if > >> it supports coherent dma_ops. Otherwise, set_arch_dma_coherent_ops() is > >> declared as nop. > >> > >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >> Cc: Russell King <linux@arm.linux.org.uk> > >> Cc: Arnd Bergmann <arnd@arndb.de> > >> Cc: Olof Johansson <olof@lixom.net> > >> Cc: Grant Likely <grant.likely@linaro.org> > >> Cc: Rob Herring <robh+dt@kernel.org> > >> Cc: Catalin Marinas <catalin.marinas@arm.com> > >> Cc: Linus Walleij <linus.walleij@linaro.org> > >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > >> --- > >> drivers/of/platform.c | 48 ++++++++++++++++++++++++++++++++++++++++--- > >> include/linux/dma-mapping.h | 7 +++++++ > >> 2 files changed, 52 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/of/platform.c b/drivers/of/platform.c > >> index 48de98f..270c0b9 100644 > >> --- a/drivers/of/platform.c > >> +++ b/drivers/of/platform.c > >> @@ -187,6 +187,50 @@ struct platform_device *of_device_alloc(struct device_node *np, > >> EXPORT_SYMBOL(of_device_alloc); > >> > >> /** > >> + * of_dma_configure - Setup DMA configuration > >> + * @dev: Device to apply DMA configuration > >> + * > >> + * Try to get devices's DMA configuration from DT and update it > >> + * accordingly. > >> + * > >> + * In case if platform code need to use own special DMA configuration,it > >> + * can use Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event > >> + * to fix up DMA configuration. > >> + */ > >> +static void of_dma_configure(struct device *dev) > >> +{ > >> + u64 dma_addr, paddr, size; > >> + int ret; > >> + > >> + dev->coherent_dma_mask = DMA_BIT_MASK(32); > >> + if (!dev->dma_mask) > >> + dev->dma_mask = &dev->coherent_dma_mask; > >> + > >> + /* > >> + * if dma-coherent property exist, call arch hook to setup > >> + * dma coherent operations. > >> + */ > >> + if (of_dma_is_coherent(dev->of_node)) { > >> + set_arch_dma_coherent_ops(dev); > >> + dev_dbg(dev, "device is dma coherent\n"); > >> + } > >> + > >> + /* > >> + * if dma-ranges property doesn't exist - just return else > >> + * setup the dma offset > >> + */ > >> + ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); > >> + if ((ret == -ENODEV) || (ret < 0)) { > >> + dev_dbg(dev, "no dma range information to setup\n"); > >> + return; > >> + } > >> + > >> + /* DMA ranges found. Calculate and set dma_pfn_offset */ > >> + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr); > >> + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset); > > > > I've got two concerns here. of_dma_get_range() retrieves only the first > > tuple from the dma-ranges property, but it is perfectly valid for > > dma-ranges to contain multiple tuples. How should we handle it if a > > device has multiple ranges it can DMA from? > > > > We've not found any cases in current Linux where more than one dma-ranges > would be used. Moreover, The MM (definitely for ARM) isn't supported such > cases at all (if i understand everything right). > - there are only one arm_dma_pfn_limit > - there is only one MM zone is used for ARM > - some arches like x86,mips can support 2 zones (per arch - not per device or bus) > DMA & DMA32, but they configured once and forever per arch. Okay. If anyone ever does implement multiple ranges then this code will need to be revisited. > > Second, while the pfn offset is being determined, I don't see anything > > making use of either the base address or size. How is the device > > constrained to only getting DMA buffers from within that range? Is the > > driver expected to manage that directly? > > > Drivers don't have to do anything special apart from setting > the correct mask. The pfn_offset case, we use DMA_ZONE which takes > care of masks already. Size is suppose to be used for dma_mask > setup which we discussed in previous threads. okay. g. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 01 May 2014 09:12 AM, Grant Likely wrote: > On Wed, 30 Apr 2014 10:19:15 -0400, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: >> Hi Grant, >> >> On Tuesday 29 April 2014 10:41 AM, Grant Likely wrote: >>> On Thu, 24 Apr 2014 11:30:04 -0400, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: >>>> Retrieve DMA configuration from DT and setup platform device's DMA >>>> parameters. The DMA configuration in DT has to be specified using >>>> "dma-ranges" and "dma-coherent" properties if supported. >>>> >>>> We setup dma_pfn_offset using "dma-ranges" and dma_coherent_ops >>>> using "dma-coherent" device tree properties. >>>> >>>> The set_arch_dma_coherent_ops macro has to be defined by arch if >>>> it supports coherent dma_ops. Otherwise, set_arch_dma_coherent_ops() is >>>> declared as nop. >>>> >>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>> Cc: Russell King <linux@arm.linux.org.uk> >>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>> Cc: Olof Johansson <olof@lixom.net> >>>> Cc: Grant Likely <grant.likely@linaro.org> >>>> Cc: Rob Herring <robh+dt@kernel.org> >>>> Cc: Catalin Marinas <catalin.marinas@arm.com> >>>> Cc: Linus Walleij <linus.walleij@linaro.org> >>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >>>> --- >>>> drivers/of/platform.c | 48 ++++++++++++++++++++++++++++++++++++++++--- >>>> include/linux/dma-mapping.h | 7 +++++++ >>>> 2 files changed, 52 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c >>>> index 48de98f..270c0b9 100644 >>>> --- a/drivers/of/platform.c >>>> +++ b/drivers/of/platform.c >>>> @@ -187,6 +187,50 @@ struct platform_device *of_device_alloc(struct device_node *np, >>>> EXPORT_SYMBOL(of_device_alloc); >>>> >>>> /** >>>> + * of_dma_configure - Setup DMA configuration >>>> + * @dev: Device to apply DMA configuration >>>> + * >>>> + * Try to get devices's DMA configuration from DT and update it >>>> + * accordingly. >>>> + * >>>> + * In case if platform code need to use own special DMA configuration,it >>>> + * can use Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event >>>> + * to fix up DMA configuration. >>>> + */ >>>> +static void of_dma_configure(struct device *dev) >>>> +{ >>>> + u64 dma_addr, paddr, size; >>>> + int ret; >>>> + >>>> + dev->coherent_dma_mask = DMA_BIT_MASK(32); >>>> + if (!dev->dma_mask) >>>> + dev->dma_mask = &dev->coherent_dma_mask; >>>> + >>>> + /* >>>> + * if dma-coherent property exist, call arch hook to setup >>>> + * dma coherent operations. >>>> + */ >>>> + if (of_dma_is_coherent(dev->of_node)) { >>>> + set_arch_dma_coherent_ops(dev); >>>> + dev_dbg(dev, "device is dma coherent\n"); >>>> + } >>>> + >>>> + /* >>>> + * if dma-ranges property doesn't exist - just return else >>>> + * setup the dma offset >>>> + */ >>>> + ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); >>>> + if ((ret == -ENODEV) || (ret < 0)) { >>>> + dev_dbg(dev, "no dma range information to setup\n"); >>>> + return; >>>> + } >>>> + >>>> + /* DMA ranges found. Calculate and set dma_pfn_offset */ >>>> + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr); >>>> + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset); >>> >>> I've got two concerns here. of_dma_get_range() retrieves only the first >>> tuple from the dma-ranges property, but it is perfectly valid for >>> dma-ranges to contain multiple tuples. How should we handle it if a >>> device has multiple ranges it can DMA from? >>> >> >> We've not found any cases in current Linux where more than one dma-ranges >> would be used. Moreover, The MM (definitely for ARM) isn't supported such >> cases at all (if i understand everything right). >> - there are only one arm_dma_pfn_limit >> - there is only one MM zone is used for ARM >> - some arches like x86,mips can support 2 zones (per arch - not per device or bus) >> DMA & DMA32, but they configured once and forever per arch. > > Okay. If anyone ever does implement multiple ranges then this code will > need to be revisited. > Sure. Thanks for the review !! Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 24, 2014 at 10:30 AM, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > Retrieve DMA configuration from DT and setup platform device's DMA > parameters. The DMA configuration in DT has to be specified using > "dma-ranges" and "dma-coherent" properties if supported. > > We setup dma_pfn_offset using "dma-ranges" and dma_coherent_ops > using "dma-coherent" device tree properties. > > The set_arch_dma_coherent_ops macro has to be defined by arch if > it supports coherent dma_ops. Otherwise, set_arch_dma_coherent_ops() is > declared as nop. > [...] > + /* > + * if dma-ranges property doesn't exist - just return else > + * setup the dma offset > + */ > + ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); > + if ((ret == -ENODEV) || (ret < 0)) { The 1st condition is redundant. > + dev_dbg(dev, "no dma range information to setup\n"); > + return; > + } > + > + /* DMA ranges found. Calculate and set dma_pfn_offset */ > + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr); > + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset); > +} > + > +/** > * of_platform_device_create_pdata - Alloc, initialize and register an of_device > * @np: pointer to node to create device for > * @bus_id: name to assign device > @@ -214,9 +258,7 @@ static struct platform_device *of_platform_device_create_pdata( > #if defined(CONFIG_MICROBLAZE) > dev->archdata.dma_mask = 0xffffffffUL; > #endif This is also dma related setup. Please move *all* dma setup into the function. > - dev->dev.coherent_dma_mask = DMA_BIT_MASK(32); > - if (!dev->dev.dma_mask) > - dev->dev.dma_mask = &dev->dev.coherent_dma_mask; > + of_dma_configure(&dev->dev); > dev->dev.bus = &platform_bus_type; > dev->dev.platform_data = platform_data; > > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h > index fd4aee2..c7d9b1b 100644 > --- a/include/linux/dma-mapping.h > +++ b/include/linux/dma-mapping.h > @@ -123,6 +123,13 @@ static inline int dma_coerce_mask_and_coherent(struct device *dev, u64 mask) > > extern u64 dma_get_required_mask(struct device *dev); > > +#ifndef set_arch_dma_coherent_ops > +static inline int set_arch_dma_coherent_ops(struct device *dev) > +{ > + return 0; > +} > +#endif > + > static inline unsigned int dma_get_max_seg_size(struct device *dev) > { > return dev->dma_parms ? dev->dma_parms->max_segment_size : 65536; > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 24, 2014 at 11:30:04AM -0400, Santosh Shilimkar wrote: > Retrieve DMA configuration from DT and setup platform device's DMA > parameters. The DMA configuration in DT has to be specified using > "dma-ranges" and "dma-coherent" properties if supported. > > We setup dma_pfn_offset using "dma-ranges" and dma_coherent_ops > using "dma-coherent" device tree properties. > > The set_arch_dma_coherent_ops macro has to be defined by arch if > it supports coherent dma_ops. Otherwise, set_arch_dma_coherent_ops() is > declared as nop. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Olof Johansson <olof@lixom.net> > Cc: Grant Likely <grant.likely@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > drivers/of/platform.c | 48 ++++++++++++++++++++++++++++++++++++++++--- > include/linux/dma-mapping.h | 7 +++++++ > 2 files changed, 52 insertions(+), 3 deletions(-) > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 48de98f..270c0b9 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -187,6 +187,50 @@ struct platform_device *of_device_alloc(struct device_node *np, > EXPORT_SYMBOL(of_device_alloc); > > /** > + * of_dma_configure - Setup DMA configuration > + * @dev: Device to apply DMA configuration > + * > + * Try to get devices's DMA configuration from DT and update it > + * accordingly. > + * > + * In case if platform code need to use own special DMA configuration,it > + * can use Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event > + * to fix up DMA configuration. > + */ > +static void of_dma_configure(struct device *dev) > +{ > + u64 dma_addr, paddr, size; > + int ret; > + > + dev->coherent_dma_mask = DMA_BIT_MASK(32); > + if (!dev->dma_mask) > + dev->dma_mask = &dev->coherent_dma_mask; > + > + /* > + * if dma-coherent property exist, call arch hook to setup > + * dma coherent operations. > + */ > + if (of_dma_is_coherent(dev->of_node)) { > + set_arch_dma_coherent_ops(dev); > + dev_dbg(dev, "device is dma coherent\n"); > + } > + > + /* > + * if dma-ranges property doesn't exist - just return else > + * setup the dma offset > + */ > + ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); > + if ((ret == -ENODEV) || (ret < 0)) { > + dev_dbg(dev, "no dma range information to setup\n"); > + return; > + } > + > + /* DMA ranges found. Calculate and set dma_pfn_offset */ > + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr); > + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset); Is this effectively the same as an IOMMU that applies a constant offset to the bus address? Could or should this be done by adding a simple IOMMU driver instead of adding dma_pfn_offset to struct device? If we had both dma-ranges (and we set dma_pfn_offset as you do here) and an IOMMU, how would the combination work? If the IOMMU driver managed dma_pfn_offset internally, it seems like we'd have two entities dealing with it. If the IOMMU driver doesn't use dma_pfn_offset, it seems like it would be exposing a weird intermediate address space that's not usable by either CPU or device. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 02 May 2014 10:54:59 Bjorn Helgaas wrote: > > +static void of_dma_configure(struct device *dev) > > +{ > > + u64 dma_addr, paddr, size; > > + int ret; > > + > > + dev->coherent_dma_mask = DMA_BIT_MASK(32); > > + if (!dev->dma_mask) > > + dev->dma_mask = &dev->coherent_dma_mask; > > + > > + /* > > + * if dma-coherent property exist, call arch hook to setup > > + * dma coherent operations. > > + */ > > + if (of_dma_is_coherent(dev->of_node)) { > > + set_arch_dma_coherent_ops(dev); > > + dev_dbg(dev, "device is dma coherent\n"); > > + } > > + > > + /* > > + * if dma-ranges property doesn't exist - just return else > > + * setup the dma offset > > + */ > > + ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); > > + if ((ret == -ENODEV) || (ret < 0)) { > > + dev_dbg(dev, "no dma range information to setup\n"); > > + return; > > + } > > + > > + /* DMA ranges found. Calculate and set dma_pfn_offset */ > > + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr); > > + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset); > > Is this effectively the same as an IOMMU that applies a constant offset > to the bus address? Could or should this be done by adding a simple IOMMU > driver instead of adding dma_pfn_offset to struct device? We currently have two dma_map_ops variants on ARM (plus another set for coherent/noncoherent differences, but we can ignore that for the sake of this discussion): one that handles linear mappings and one that handles IOMMUs by calling into the linux/iommu.h APIs. I guess what you mean by 'a simple IOMMU driver' would be another dma_map_ops implementation that is separate from real IOMMUs, right? That could certainly be done, but in effect it is almost the same as the linear mapping we already have. > If we had both dma-ranges (and we set dma_pfn_offset as you do here) and an > IOMMU, how would the combination work? If the IOMMU driver managed > dma_pfn_offset internally, it seems like we'd have two entities dealing > with it. If the IOMMU driver doesn't use dma_pfn_offset, it seems like > it would be exposing a weird intermediate address space that's not usable > by either CPU or device. The iommu dma_map_ops implementation does not need to worry about the dma_pfn_offset. We are still debating how to represent IOMMUs in DT at the moment, so it's not clear yet if we would consider a device node with both a dma-ranges property and an iommu reference as valid. What we will probably need is a way to represent the valid bus addresses that can be used for transfers from the DMA master through the IOMMU. This could be done through dma-ranges, or some other property, we will have to decide that soon. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[+cc Ben, Chris] On Fri, May 2, 2014 at 12:59 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 02 May 2014 10:54:59 Bjorn Helgaas wrote: >> > +static void of_dma_configure(struct device *dev) >> > +{ >> > + u64 dma_addr, paddr, size; >> > + int ret; >> > + >> > + dev->coherent_dma_mask = DMA_BIT_MASK(32); >> > + if (!dev->dma_mask) >> > + dev->dma_mask = &dev->coherent_dma_mask; >> > + >> > + /* >> > + * if dma-coherent property exist, call arch hook to setup >> > + * dma coherent operations. >> > + */ >> > + if (of_dma_is_coherent(dev->of_node)) { >> > + set_arch_dma_coherent_ops(dev); >> > + dev_dbg(dev, "device is dma coherent\n"); >> > + } >> > + >> > + /* >> > + * if dma-ranges property doesn't exist - just return else >> > + * setup the dma offset >> > + */ >> > + ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); >> > + if ((ret == -ENODEV) || (ret < 0)) { >> > + dev_dbg(dev, "no dma range information to setup\n"); >> > + return; >> > + } >> > + >> > + /* DMA ranges found. Calculate and set dma_pfn_offset */ >> > + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr); >> > + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset); >> >> Is this effectively the same as an IOMMU that applies a constant offset >> to the bus address? Could or should this be done by adding a simple IOMMU >> driver instead of adding dma_pfn_offset to struct device? > > We currently have two dma_map_ops variants on ARM (plus another set for > coherent/noncoherent differences, but we can ignore that for the sake > of this discussion): one that handles linear mappings and one that > handles IOMMUs by calling into the linux/iommu.h APIs. > > I guess what you mean by 'a simple IOMMU driver' would be another > dma_map_ops implementation that is separate from real IOMMUs, right? I suppose so; it seems like the offset could be managed inside arm_dma_ops. My idea of an IOMMU is something that maps bus addresses to physical memory addresses. That's what we're doing here; it's just that the mapping function is very simple. So why add something new in struct device for it? I think powerpc and tile do something similar in dma_direct_map_page() and tile_pci_dma_map_page() (they store the offset in struct dev_archdata). Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 48de98f..270c0b9 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -187,6 +187,50 @@ struct platform_device *of_device_alloc(struct device_node *np, EXPORT_SYMBOL(of_device_alloc); /** + * of_dma_configure - Setup DMA configuration + * @dev: Device to apply DMA configuration + * + * Try to get devices's DMA configuration from DT and update it + * accordingly. + * + * In case if platform code need to use own special DMA configuration,it + * can use Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event + * to fix up DMA configuration. + */ +static void of_dma_configure(struct device *dev) +{ + u64 dma_addr, paddr, size; + int ret; + + dev->coherent_dma_mask = DMA_BIT_MASK(32); + if (!dev->dma_mask) + dev->dma_mask = &dev->coherent_dma_mask; + + /* + * if dma-coherent property exist, call arch hook to setup + * dma coherent operations. + */ + if (of_dma_is_coherent(dev->of_node)) { + set_arch_dma_coherent_ops(dev); + dev_dbg(dev, "device is dma coherent\n"); + } + + /* + * if dma-ranges property doesn't exist - just return else + * setup the dma offset + */ + ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); + if ((ret == -ENODEV) || (ret < 0)) { + dev_dbg(dev, "no dma range information to setup\n"); + return; + } + + /* DMA ranges found. Calculate and set dma_pfn_offset */ + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr); + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset); +} + +/** * of_platform_device_create_pdata - Alloc, initialize and register an of_device * @np: pointer to node to create device for * @bus_id: name to assign device @@ -214,9 +258,7 @@ static struct platform_device *of_platform_device_create_pdata( #if defined(CONFIG_MICROBLAZE) dev->archdata.dma_mask = 0xffffffffUL; #endif - dev->dev.coherent_dma_mask = DMA_BIT_MASK(32); - if (!dev->dev.dma_mask) - dev->dev.dma_mask = &dev->dev.coherent_dma_mask; + of_dma_configure(&dev->dev); dev->dev.bus = &platform_bus_type; dev->dev.platform_data = platform_data; diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index fd4aee2..c7d9b1b 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -123,6 +123,13 @@ static inline int dma_coerce_mask_and_coherent(struct device *dev, u64 mask) extern u64 dma_get_required_mask(struct device *dev); +#ifndef set_arch_dma_coherent_ops +static inline int set_arch_dma_coherent_ops(struct device *dev) +{ + return 0; +} +#endif + static inline unsigned int dma_get_max_seg_size(struct device *dev) { return dev->dma_parms ? dev->dma_parms->max_segment_size : 65536;