[PATCH] powerpc : dma-mapping : Check null condition for dev->archdata.dma_ops

Modifies get_dma_ops() implementation on ppc arch to check null condition
for dev->archdata.dma_ops; returns common dma_direct_ops structure in
case its NULL

Signed-off-by: Nikhil Badola <redacted>
---
 arch/powerpc/include/asm/dma-mapping.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
index 150866b..d73bae8 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h

@@ -86,10 +86,12 @@ static inline struct dma_map_ops *get_dma_ops(struct device *dev)
 	 */
 	if (unlikely(dev == NULL))
 		return NULL;
-
-	return dev->archdata.dma_ops;
+	if (dev->archdata.dma_ops)
+		return dev->archdata.dma_ops;
+	return &dma_direct_ops;
 }
 
+
 static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
 {
 	dev->archdata.dma_ops = ops;

-- 
1.7.11.7

On 7/18/14, Nikhil Badola [off-list ref] wrote:

Modifies get_dma_ops() implementation on ppc arch to check null condition

 which means that dma is not supported.

Could you please describe the use case where the ops is null.

-- 
Regards,
Denis

-----Original Message-----
From: Denis Kirjanov [mailto:kirjanov@gmail.com]
Sent: Friday, July 18, 2014 7:21 PM
To: Badola Nikhil-B46172
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc : dma-mapping : Check null condition for dev=

-

quoted

archdata.dma_ops

=20
On 7/18/14, Nikhil Badola [off-list ref] wrote:

quoted

Modifies get_dma_ops() implementation on ppc arch to check null
condition

 which means that dma is not supported.
=20
Could you please describe the use case where the ops is null.

The use case in which ops are null is while running USB in Gadget and Otg m=
ode.

For PPC architecture, whenever a platform device is registered, pdev->dev i=
s=20
assigned dma_ops.=20
In USB, one single chipidea platform device is registered for any mode (hos=
t, gadget or otg)
whose dev, as explained above, has dma_ops set and this dev is assigned to =
 ci->dev(dev of chipidea struct)
which is used by host controller device. That is why we don't need the abov=
e null case checking in
host mode. But when we run usb in gadget/otg mode, the device structure use=
d is ci->gadget.dev
which does not have dma_ops set and it crashes when dma transaction starts =
when it calls get_dma_ops()
which returns NULL.

A similar approach is used in ARM architecture which checks for null condit=
ion and returns=20
common dma_ops

=20

quoted

for dev->archdata.dma_ops; returns common dma_direct_ops structure in
case its NULL

Signed-off-by: Nikhil Badola <redacted>
---
 arch/powerpc/include/asm/dma-mapping.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/dma-mapping.h

b/arch/powerpc/include/asm/dma-mapping.h
index 150866b..d73bae8 100644

--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h

@@ -86,10 +86,12 @@ static inline struct dma_map_ops

*get_dma_ops(struct device *dev)
 	 */
 	if (unlikely(dev =3D=3D NULL))
 		return NULL;
-
-	return dev->archdata.dma_ops;
+	if (dev->archdata.dma_ops)
+		return dev->archdata.dma_ops;
+	return &dma_direct_ops;
 }

+
 static inline void set_dma_ops(struct device *dev, struct dma_map_ops
*ops)  {
 	dev->archdata.dma_ops =3D ops;
--
1.7.11.7

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

=20
=20
--
Regards,
Denis

Regards,
Nikhil

On Fri, Jul 18, 2014 at 03:34:52PM +0530, Nikhil Badola wrote:

Modifies get_dma_ops() implementation on ppc arch to check null condition
for dev->archdata.dma_ops; returns common dma_direct_ops structure in
case its NULL

Signed-off-by: Nikhil Badola <redacted>
---
 arch/powerpc/include/asm/dma-mapping.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

I'm not sure why this was delegated to me in patchwork, as it doesn't
appear to be FSL-specific...  I wish patchwork showed the history of
changes to a patch.

The use case in which ops are null is while running USB in Gadget and
Otg mode.

For PPC architecture, whenever a platform device is registered,
pdev->dev is assigned dma_ops.  In USB, one single chipidea platform
device is registered for any mode (host, gadget or otg) whose dev, as
explained above, has dma_ops set and this dev is assigned to
ci->dev(dev of chipidea struct) which is used by host controller
device.  That is why we don't need the above null case checking in host
mode.  But when we run usb in gadget/otg mode, the device structure
used is ci->gadget.dev which does not have dma_ops set and it crashes
when dma transaction starts when it calls get_dma_ops() which returns
NULL.

A similar approach is used in ARM architecture which checks for null
condition and returns common dma_ops

The above doesn't make it clear to me why dma_ops is NULL in non-host
modes, or why "direct" is necessarily the right ops -- what if swiotlb or
an iommu is needed?

If the problem is that you're using a "struct device" other than the
platform device, shouldn't that be fixed?  Or make sure that this other
"struct device" has been properly set up by arch code for doing DMA.

 }
 
+
 static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)

Don't add this newline

-Scott