[PATCH] of: set dma_mask to point to coherent_dma_mask

[PATCH] of: set dma_mask to point to coherent_dma_mask

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

Platform devices created by DT code don't initialize dma_mask pointer to
anything. Set it to coherent_dma_mask by default if the architecture
code has not set it.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
I think this is at least part of what is needed to fix dma_mask issue
raised by Stefano [1]. Things should work AFAICT with just this, but
I suppose the xgmac driver needs to set the mask as well (not relying
on the default), but some pointing the dma_mask to a valid value is
needed first.

Rob

[1] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg528236.html

 drivers/of/platform.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index f6dcde2..fce088e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -215,6 +215,8 @@ static struct platform_device *of_platform_device_create_pdata(
 	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;
 	dev->dev.bus = &platform_bus_type;
 	dev->dev.platform_data = platform_data;
 
-- 
1.8.1.2

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Grant Likely]

On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Platform devices created by DT code don't initialize dma_mask pointer to
> anything. Set it to coherent_dma_mask by default if the architecture
> code has not set it.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>

I believe this is okay. I haven't done any testing to back up that
opinion though.

g.

> ---
> I think this is at least part of what is needed to fix dma_mask issue
> raised by Stefano [1]. Things should work AFAICT with just this, but
> I suppose the xgmac driver needs to set the mask as well (not relying
> on the default), but some pointing the dma_mask to a valid value is
> needed first.
> 
> Rob
> 
> [1] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg528236.html
> 
>  drivers/of/platform.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index f6dcde2..fce088e 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -215,6 +215,8 @@ static struct platform_device *of_platform_device_create_pdata(
>  	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;
>  	dev->dev.bus = &platform_bus_type;
>  	dev->dev.platform_data = platform_data;
>  
> -- 
> 1.8.1.2
> 

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Stefano Stabellini]

On Sat, 2 Nov 2013, Grant Likely wrote:
> On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2@gmail.com> wrote:
> > From: Rob Herring <rob.herring@calxeda.com>
> > 
> > Platform devices created by DT code don't initialize dma_mask pointer to
> > anything. Set it to coherent_dma_mask by default if the architecture
> > code has not set it.
> > 
> > Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> 
> I believe this is okay. I haven't done any testing to back up that
> opinion though.

Tested-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

 
> > I think this is at least part of what is needed to fix dma_mask issue
> > raised by Stefano [1]. Things should work AFAICT with just this, but
> > I suppose the xgmac driver needs to set the mask as well (not relying
> > on the default), but some pointing the dma_mask to a valid value is
> > needed first.
> > 
> > Rob
> > 
> > [1] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg528236.html
> > 
> >  drivers/of/platform.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index f6dcde2..fce088e 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -215,6 +215,8 @@ static struct platform_device *of_platform_device_create_pdata(
> >  	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;
> >  	dev->dev.bus = &platform_bus_type;
> >  	dev->dev.platform_data = platform_data;
> >  

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Stefano Stabellini]

On Sat, 2 Nov 2013, Grant Likely wrote:
> On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > 
> > Platform devices created by DT code don't initialize dma_mask pointer to
> > anything. Set it to coherent_dma_mask by default if the architecture
> > code has not set it.
> > 
> > Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> I believe this is okay. I haven't done any testing to back up that
> opinion though.

Tested-by: Stefano Stabellini <stefano.stabellini-mvvWK6WmYclDPfheJLI6IQ@public.gmane.org>

 
> > I think this is at least part of what is needed to fix dma_mask issue
> > raised by Stefano [1]. Things should work AFAICT with just this, but
> > I suppose the xgmac driver needs to set the mask as well (not relying
> > on the default), but some pointing the dma_mask to a valid value is
> > needed first.
> > 
> > Rob
> > 
> > [1] http://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg528236.html
> > 
> >  drivers/of/platform.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index f6dcde2..fce088e 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -215,6 +215,8 @@ static struct platform_device *of_platform_device_create_pdata(
> >  	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;
> >  	dev->dev.bus = &platform_bus_type;
> >  	dev->dev.platform_data = platform_data;
> >  
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Stefano Stabellini]

On Wed, 6 Nov 2013, Stefano Stabellini wrote:
> On Sat, 2 Nov 2013, Grant Likely wrote:
> > On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2@gmail.com> wrote:
> > > From: Rob Herring <rob.herring@calxeda.com>
> > > 
> > > Platform devices created by DT code don't initialize dma_mask pointer to
> > > anything. Set it to coherent_dma_mask by default if the architecture
> > > code has not set it.
> > > 
> > > Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> > 
> > I believe this is okay. I haven't done any testing to back up that
> > opinion though.
> 
> Tested-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

BTW I hope you are planning on sending this upstream soon, because
without it I can very easily corrupt my root fs by simply booting the system.


> > > I think this is at least part of what is needed to fix dma_mask issue
> > > raised by Stefano [1]. Things should work AFAICT with just this, but
> > > I suppose the xgmac driver needs to set the mask as well (not relying
> > > on the default), but some pointing the dma_mask to a valid value is
> > > needed first.
> > > 
> > > Rob
> > > 
> > > [1] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg528236.html
> > > 
> > >  drivers/of/platform.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > index f6dcde2..fce088e 100644
> > > --- a/drivers/of/platform.c
> > > +++ b/drivers/of/platform.c
> > > @@ -215,6 +215,8 @@ static struct platform_device *of_platform_device_create_pdata(
> > >  	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;
> > >  	dev->dev.bus = &platform_bus_type;
> > >  	dev->dev.platform_data = platform_data;
> > >  
> 

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Stefano Stabellini]

On Wed, 6 Nov 2013, Stefano Stabellini wrote:
> On Sat, 2 Nov 2013, Grant Likely wrote:
> > On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > > 
> > > Platform devices created by DT code don't initialize dma_mask pointer to
> > > anything. Set it to coherent_dma_mask by default if the architecture
> > > code has not set it.
> > > 
> > > Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > 
> > I believe this is okay. I haven't done any testing to back up that
> > opinion though.
> 
> Tested-by: Stefano Stabellini <stefano.stabellini-mvvWK6WmYclDPfheJLI6IQ@public.gmane.org>

BTW I hope you are planning on sending this upstream soon, because
without it I can very easily corrupt my root fs by simply booting the system.


> > > I think this is at least part of what is needed to fix dma_mask issue
> > > raised by Stefano [1]. Things should work AFAICT with just this, but
> > > I suppose the xgmac driver needs to set the mask as well (not relying
> > > on the default), but some pointing the dma_mask to a valid value is
> > > needed first.
> > > 
> > > Rob
> > > 
> > > [1] http://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg528236.html
> > > 
> > >  drivers/of/platform.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > index f6dcde2..fce088e 100644
> > > --- a/drivers/of/platform.c
> > > +++ b/drivers/of/platform.c
> > > @@ -215,6 +215,8 @@ static struct platform_device *of_platform_device_create_pdata(
> > >  	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;
> > >  	dev->dev.bus = &platform_bus_type;
> > >  	dev->dev.platform_data = platform_data;
> > >  
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Grant Likely]

On Mon, 11 Nov 2013 16:59:14 +0000, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> On Wed, 6 Nov 2013, Stefano Stabellini wrote:
> > On Sat, 2 Nov 2013, Grant Likely wrote:
> > > On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2@gmail.com> wrote:
> > > > From: Rob Herring <rob.herring@calxeda.com>
> > > > 
> > > > Platform devices created by DT code don't initialize dma_mask pointer to
> > > > anything. Set it to coherent_dma_mask by default if the architecture
> > > > code has not set it.
> > > > 
> > > > Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> > > 
> > > I believe this is okay. I haven't done any testing to back up that
> > > opinion though.
> > 
> > Tested-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> BTW I hope you are planning on sending this upstream soon, because
> without it I can very easily corrupt my root fs by simply booting the system.

Gah! That's pretty nasty. It is in Rob's for-next branch, but it hasn't
been marked for backporting to stable. Best thing to do here is for
either you or Rob to repost the patch to stable@vger.kernel.org and
include the commit id from Rob's tree. It's not upstream yet, but it
will be soon.

How far back in kernel releases do you want the patch backported to?

g.

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Grant Likely]

On Mon, 11 Nov 2013 16:59:14 +0000, Stefano Stabellini <stefano.stabellini-mvvWK6WmYclDPfheJLI6IQ@public.gmane.org> wrote:
> On Wed, 6 Nov 2013, Stefano Stabellini wrote:
> > On Sat, 2 Nov 2013, Grant Likely wrote:
> > > On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > > From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > > > 
> > > > Platform devices created by DT code don't initialize dma_mask pointer to
> > > > anything. Set it to coherent_dma_mask by default if the architecture
> > > > code has not set it.
> > > > 
> > > > Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > > 
> > > I believe this is okay. I haven't done any testing to back up that
> > > opinion though.
> > 
> > Tested-by: Stefano Stabellini <stefano.stabellini-mvvWK6WmYclDPfheJLI6IQ@public.gmane.org>
> 
> BTW I hope you are planning on sending this upstream soon, because
> without it I can very easily corrupt my root fs by simply booting the system.

Gah! That's pretty nasty. It is in Rob's for-next branch, but it hasn't
been marked for backporting to stable. Best thing to do here is for
either you or Rob to repost the patch to stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org and
include the commit id from Rob's tree. It's not upstream yet, but it
will be soon.

How far back in kernel releases do you want the patch backported to?

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Russell King - ARM Linux]

On Mon, Nov 11, 2013 at 04:59:14PM +0000, Stefano Stabellini wrote:
> On Wed, 6 Nov 2013, Stefano Stabellini wrote:
> > On Sat, 2 Nov 2013, Grant Likely wrote:
> > > On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2@gmail.com> wrote:
> > > > From: Rob Herring <rob.herring@calxeda.com>
> > > > 
> > > > Platform devices created by DT code don't initialize dma_mask pointer to
> > > > anything. Set it to coherent_dma_mask by default if the architecture
> > > > code has not set it.
> > > > 
> > > > Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> > > 
> > > I believe this is okay. I haven't done any testing to back up that
> > > opinion though.
> > 
> > Tested-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> BTW I hope you are planning on sending this upstream soon, because
> without it I can very easily corrupt my root fs by simply booting the system.

What's the mechanism for that corruption?  Having the DMA mask not set
should not cause DMA corruption - that hints at something else being
wrong, and that needs to be investigated.

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Russell King - ARM Linux]

On Mon, Nov 11, 2013 at 04:59:14PM +0000, Stefano Stabellini wrote:
> On Wed, 6 Nov 2013, Stefano Stabellini wrote:
> > On Sat, 2 Nov 2013, Grant Likely wrote:
> > > On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > > From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > > > 
> > > > Platform devices created by DT code don't initialize dma_mask pointer to
> > > > anything. Set it to coherent_dma_mask by default if the architecture
> > > > code has not set it.
> > > > 
> > > > Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > > 
> > > I believe this is okay. I haven't done any testing to back up that
> > > opinion though.
> > 
> > Tested-by: Stefano Stabellini <stefano.stabellini-mvvWK6WmYclDPfheJLI6IQ@public.gmane.org>
> 
> BTW I hope you are planning on sending this upstream soon, because
> without it I can very easily corrupt my root fs by simply booting the system.

What's the mechanism for that corruption?  Having the DMA mask not set
should not cause DMA corruption - that hints at something else being
wrong, and that needs to be investigated.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Stefano Stabellini]

On Tue, 12 Nov 2013, Russell King - ARM Linux wrote:
> On Mon, Nov 11, 2013 at 04:59:14PM +0000, Stefano Stabellini wrote:
> > On Wed, 6 Nov 2013, Stefano Stabellini wrote:
> > > On Sat, 2 Nov 2013, Grant Likely wrote:
> > > > On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2@gmail.com> wrote:
> > > > > From: Rob Herring <rob.herring@calxeda.com>
> > > > > 
> > > > > Platform devices created by DT code don't initialize dma_mask pointer to
> > > > > anything. Set it to coherent_dma_mask by default if the architecture
> > > > > code has not set it.
> > > > > 
> > > > > Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> > > > 
> > > > I believe this is okay. I haven't done any testing to back up that
> > > > opinion though.
> > > 
> > > Tested-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > BTW I hope you are planning on sending this upstream soon, because
> > without it I can very easily corrupt my root fs by simply booting the system.
> 
> What's the mechanism for that corruption?  Having the DMA mask not set
> should not cause DMA corruption - that hints at something else being
> wrong, and that needs to be investigated.

Russell, you are right.
I went through the swiotlb-xen code and I realized that I missed a
map_page call on one of the (usually unused) fallback paths.
Thanks!!

Re: [PATCH] of: set dma_mask to point to coherent_dma_mask

[Stefano Stabellini]

On Tue, 12 Nov 2013, Russell King - ARM Linux wrote:
> On Mon, Nov 11, 2013 at 04:59:14PM +0000, Stefano Stabellini wrote:
> > On Wed, 6 Nov 2013, Stefano Stabellini wrote:
> > > On Sat, 2 Nov 2013, Grant Likely wrote:
> > > > On Wed, 30 Oct 2013 00:05:22 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > > > From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > > > > 
> > > > > Platform devices created by DT code don't initialize dma_mask pointer to
> > > > > anything. Set it to coherent_dma_mask by default if the architecture
> > > > > code has not set it.
> > > > > 
> > > > > Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > > > 
> > > > I believe this is okay. I haven't done any testing to back up that
> > > > opinion though.
> > > 
> > > Tested-by: Stefano Stabellini <stefano.stabellini-mvvWK6WmYclDPfheJLI6IQ@public.gmane.org>
> > 
> > BTW I hope you are planning on sending this upstream soon, because
> > without it I can very easily corrupt my root fs by simply booting the system.
> 
> What's the mechanism for that corruption?  Having the DMA mask not set
> should not cause DMA corruption - that hints at something else being
> wrong, and that needs to be investigated.

Russell, you are right.
I went through the swiotlb-xen code and I realized that I missed a
map_page call on one of the (usually unused) fallback paths.
Thanks!!
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html