Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
Arnd, Rob,
On Friday 14 March 2014 01:25 PM, Arnd Bergmann wrote:
> On Friday 14 March 2014, Rob Herring wrote:
>> On Wed, Mar 12, 2014 at 11:58 AM, Arnd Bergmann wrote:
>>> On Wednesday 12 March 2014 15:19:48 Grygorii Strashko wrote:
> Isn't the question here how do we handle restrictions added by the
> bus? It seems while this series adds support for handling dma-ranges
> to set the default case, it also needs to handle when the driver sets
> the mask. Is this not simply a matter of having dma_set_mask also
> parse dma-ranges (or reuse a saved bus mask)?
>>>
>>> With the current proposed implementation, the device also has to set
>>> a "dma-ranges" property to get any DMA support, we don't just take
>>> the mask of the parent bus. This is important because the translation
>>> does not have to be 1:1 but there can be an offset between the device
>>> and the parent. I'd prefer to leave this explicit.
>>
>> But according to Ben H, dma-ranges is a bridge (i.e. parent) property
>> and not a device property[1]. So I don't think the device's mask
>> (again, before any bus restrictions) belongs in DT. A given IP block
>> is going to have some number of address bits for DMA. How it is hooked
>> up into an SOC is a separate issue. The driver should know its mask
>> whether it is fixed, discoverable, or tied to the compatible string.
>>
>> Santosh's patch only looks for dma-ranges in parent nodes which I
>> believe is correct.
>
> Ok, good point.
>
>> I think this approach is much less useful for platform devices than it is
>> for PCI devices, where we don't explicitly describe the "ranges" for each
>> device. Are you thinking of off-chip or on-chip DMA masters here? If
>> on-chip, I don't think it's likely that we would end up with different
>> versions of the chip that have the same device on there but not the
>> same DMA capabilities.
>
> Sounds like a challenge to h/w designers. :)
>
> I'm mainly thinking about the midway case where all masters are 32-bit
> except AHCI which is 64-bit. The core libahci sets the mask based on
> the capabilities register regardless of PCI or platform bus. Requiring
> this to be setup by the platform or in the DT seems like a step
> backwards. A slightly more complicated case would be something like
> midway, but with RAM starting at say 2GB and DMA masters have the same
> view as the cpu (i.e. no offset). Then the platform does need to set
> the mask to (2^31 -1).
>>>
>>> So how would you show this in DT? I'd assume that some of the other
>>> devices on midway have drivers that may also try to set a 64-bit mask,
>>> like EHCI for instance does. Is AHCI the only one that sits on a 64-bit
>>> wide bus, like this?
>>>
>>> axi {
>>> #address-cells = <2>;
>>> #size-cells = <2>;
>>> dma-ranges = <0 0 0 0 0x100 0>; /* max phys address space */
>>>
>>> ahci {
>>> dma-ranges = <0 0 0 0 0x100 0>;
>>
>> There is no point to this dma-ranges here. Based on the capabilities
>> reg, we know that we can do either 32 or 64 bit DMA.
>
> Ok
>
>> In the case of
>> 64-bit DMA, the device should try to set its mask to DMA_MASK(64), but
>> the parent dma-ranges should restrict that to DMA_MASK(40).
>
> Now I'm confused about what dma_set_mask is actually supposed to do here.
> I /think/ it should fail the DMA_MASK(64) call if the bus supports
> less than 64 bits as in the above example. However it seems like a
> valid shortcut to always succeed here if the effective mask covers
> all of the installed RAM.
>
> That would mean that even if we only have a 32-bit bus, but all RAM
> resides below 4GB, a call to dma_set_mask using DMA_MASK(64) would
> also succeed.
>
>>> ...
>>> };
>>>
>>> ahb {
>>> #address-cells = <1>;
>>> #size-cells = <1>;
>>
>> This would need to be 2. ePAPR says the size cells size is determined
>> by #size-cells in the same node.
>
> Ah, of course.
>
>>> dma-ranges = <0 0 0 0x1 0>; /* only 4GB */
>>>
>>> ehci {
>>> dma-ranges = <0 0 0x>;
>>
>> Again, I don't think this is needed. Here, regardless of the device's
>> capabilities, the bus is restricting the mask to DMA_MASK(32).
>
> Right.
>
>>> /* side-question: is that the right way
>>> to describe 4GB length? it seems missing
>>> a byte */
>>> };
>>> };
>>> };
>>>
>>> BTW, I hope I understood you right that you wouldn't actually trust the
>>> capabilities register by itself but only allow setting the 64-bit mask
>>> in the arch code if the DT doesn't have any information about the
>>> bus being
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
Arnd, Rob,
On Friday 14 March 2014 01:25 PM, Arnd Bergmann wrote:
On Friday 14 March 2014, Rob Herring wrote:
On Wed, Mar 12, 2014 at 11:58 AM, Arnd Bergmann a...@arndb.de wrote:
On Wednesday 12 March 2014 15:19:48 Grygorii Strashko wrote:
Isn't the question here how do we handle restrictions added by the
bus? It seems while this series adds support for handling dma-ranges
to set the default case, it also needs to handle when the driver sets
the mask. Is this not simply a matter of having dma_set_mask also
parse dma-ranges (or reuse a saved bus mask)?
With the current proposed implementation, the device also has to set
a dma-ranges property to get any DMA support, we don't just take
the mask of the parent bus. This is important because the translation
does not have to be 1:1 but there can be an offset between the device
and the parent. I'd prefer to leave this explicit.
But according to Ben H, dma-ranges is a bridge (i.e. parent) property
and not a device property[1]. So I don't think the device's mask
(again, before any bus restrictions) belongs in DT. A given IP block
is going to have some number of address bits for DMA. How it is hooked
up into an SOC is a separate issue. The driver should know its mask
whether it is fixed, discoverable, or tied to the compatible string.
Santosh's patch only looks for dma-ranges in parent nodes which I
believe is correct.
Ok, good point.
I think this approach is much less useful for platform devices than it is
for PCI devices, where we don't explicitly describe the ranges for each
device. Are you thinking of off-chip or on-chip DMA masters here? If
on-chip, I don't think it's likely that we would end up with different
versions of the chip that have the same device on there but not the
same DMA capabilities.
Sounds like a challenge to h/w designers. :)
I'm mainly thinking about the midway case where all masters are 32-bit
except AHCI which is 64-bit. The core libahci sets the mask based on
the capabilities register regardless of PCI or platform bus. Requiring
this to be setup by the platform or in the DT seems like a step
backwards. A slightly more complicated case would be something like
midway, but with RAM starting at say 2GB and DMA masters have the same
view as the cpu (i.e. no offset). Then the platform does need to set
the mask to (2^31 -1).
So how would you show this in DT? I'd assume that some of the other
devices on midway have drivers that may also try to set a 64-bit mask,
like EHCI for instance does. Is AHCI the only one that sits on a 64-bit
wide bus, like this?
axi {
#address-cells = 2;
#size-cells = 2;
dma-ranges = 0 0 0 0 0x100 0; /* max phys address space */
ahci {
dma-ranges = 0 0 0 0 0x100 0;
There is no point to this dma-ranges here. Based on the capabilities
reg, we know that we can do either 32 or 64 bit DMA.
Ok
In the case of
64-bit DMA, the device should try to set its mask to DMA_MASK(64), but
the parent dma-ranges should restrict that to DMA_MASK(40).
Now I'm confused about what dma_set_mask is actually supposed to do here.
I /think/ it should fail the DMA_MASK(64) call if the bus supports
less than 64 bits as in the above example. However it seems like a
valid shortcut to always succeed here if the effective mask covers
all of the installed RAM.
That would mean that even if we only have a 32-bit bus, but all RAM
resides below 4GB, a call to dma_set_mask using DMA_MASK(64) would
also succeed.
...
};
ahb {
#address-cells = 1;
#size-cells = 1;
This would need to be 2. ePAPR says the size cells size is determined
by #size-cells in the same node.
Ah, of course.
dma-ranges = 0 0 0 0x1 0; /* only 4GB */
ehci {
dma-ranges = 0 0 0x;
Again, I don't think this is needed. Here, regardless of the device's
capabilities, the bus is restricting the mask to DMA_MASK(32).
Right.
/* side-question: is that the right way
to describe 4GB length? it seems missing
a byte */
};
};
};
BTW, I hope I understood you right that you wouldn't actually trust the
capabilities register by itself but only allow setting the 64-bit mask
in the arch code if the DT doesn't have any information about the
bus being incapable of doing this, right?
Ideally no, but it appears we are ATM for midway. We get away with it
since it is midway/highbank specific driver setup and know there are
not any restrictions in addressing RAM. I think we have to keep bus
and device masks separate and the final mask is the AND of them. There
isn't
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Friday 14 March 2014, Rob Herring wrote:
> On Wed, Mar 12, 2014 at 11:58 AM, Arnd Bergmann wrote:
> > On Wednesday 12 March 2014 15:19:48 Grygorii Strashko wrote:
> >> > Isn't the question here how do we handle restrictions added by the
> >> > bus? It seems while this series adds support for handling dma-ranges
> >> > to set the default case, it also needs to handle when the driver sets
> >> > the mask. Is this not simply a matter of having dma_set_mask also
> >> > parse dma-ranges (or reuse a saved bus mask)?
> >
> > With the current proposed implementation, the device also has to set
> > a "dma-ranges" property to get any DMA support, we don't just take
> > the mask of the parent bus. This is important because the translation
> > does not have to be 1:1 but there can be an offset between the device
> > and the parent. I'd prefer to leave this explicit.
>
> But according to Ben H, dma-ranges is a bridge (i.e. parent) property
> and not a device property[1]. So I don't think the device's mask
> (again, before any bus restrictions) belongs in DT. A given IP block
> is going to have some number of address bits for DMA. How it is hooked
> up into an SOC is a separate issue. The driver should know its mask
> whether it is fixed, discoverable, or tied to the compatible string.
>
> Santosh's patch only looks for dma-ranges in parent nodes which I
> believe is correct.
Ok, good point.
> >> >> I think this approach is much less useful for platform devices than it
> >> >> is
> >> >> for PCI devices, where we don't explicitly describe the "ranges" for
> >> >> each
> >> >> device. Are you thinking of off-chip or on-chip DMA masters here? If
> >> >> on-chip, I don't think it's likely that we would end up with different
> >> >> versions of the chip that have the same device on there but not the
> >> >> same DMA capabilities.
> >> >
> >> > Sounds like a challenge to h/w designers. :)
> >> >
> >> > I'm mainly thinking about the midway case where all masters are 32-bit
> >> > except AHCI which is 64-bit. The core libahci sets the mask based on
> >> > the capabilities register regardless of PCI or platform bus. Requiring
> >> > this to be setup by the platform or in the DT seems like a step
> >> > backwards. A slightly more complicated case would be something like
> >> > midway, but with RAM starting at say 2GB and DMA masters have the same
> >> > view as the cpu (i.e. no offset). Then the platform does need to set
> >> > the mask to (2^31 -1).
> >
> > So how would you show this in DT? I'd assume that some of the other
> > devices on midway have drivers that may also try to set a 64-bit mask,
> > like EHCI for instance does. Is AHCI the only one that sits on a 64-bit
> > wide bus, like this?
> >
> > axi {
> > #address-cells = <2>;
> > #size-cells = <2>;
> > dma-ranges = <0 0 0 0 0x100 0>; /* max phys address space */
> >
> > ahci {
> > dma-ranges = <0 0 0 0 0x100 0>;
>
> There is no point to this dma-ranges here. Based on the capabilities
> reg, we know that we can do either 32 or 64 bit DMA.
Ok
> In the case of
> 64-bit DMA, the device should try to set its mask to DMA_MASK(64), but
> the parent dma-ranges should restrict that to DMA_MASK(40).
Now I'm confused about what dma_set_mask is actually supposed to do here.
I /think/ it should fail the DMA_MASK(64) call if the bus supports
less than 64 bits as in the above example. However it seems like a
valid shortcut to always succeed here if the effective mask covers
all of the installed RAM.
That would mean that even if we only have a 32-bit bus, but all RAM
resides below 4GB, a call to dma_set_mask using DMA_MASK(64) would
also succeed.
> > ...
> > };
> >
> > ahb {
> > #address-cells = <1>;
> > #size-cells = <1>;
>
> This would need to be 2. ePAPR says the size cells size is determined
> by #size-cells in the same node.
Ah, of course.
> > dma-ranges = <0 0 0 0x1 0>; /* only 4GB */
> >
> > ehci {
> > dma-ranges = <0 0 0x>;
>
> Again, I don't think this is needed. Here, regardless of the device's
> capabilities, the bus is restricting the mask to DMA_MASK(32).
Right.
> > /* side-question: is that the right way
> > to describe 4GB length? it seems missing
> > a byte */
> > };
> > };
> > };
> >
> > BTW, I hope I understood you right that you wouldn't actually trust the
> > capabilities register by itself but only allow setting the 64-bit mask
> > in the arch code if the DT doesn't have any information about the
> > bus being incapable of doing this, right?
>
> Ideally no, but it appears we are ATM for midway. We get away
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Wed, Mar 12, 2014 at 11:58 AM, Arnd Bergmann wrote:
> On Wednesday 12 March 2014 15:19:48 Grygorii Strashko wrote:
>> On 03/12/2014 02:15 AM, Rob Herring wrote:
>> > On Sun, Mar 9, 2014 at 12:39 AM, Arnd Bergmann wrote:
>> >> On Saturday 08 March 2014, Rob Herring wrote:
>> >>> On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann wrote:
>> >>
>> a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32)
>> on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on
>> devices
>> that would like to do more than that, or call e.g.
>> dma_set_mask(DMA_MASK(28))
>> for devices that can do less than 32 bit, as given in the argument. This
>> approach would be most consistent with the way PCI works, but it doesn't
>> really work well for the case where the mask is less than 32-bit and the
>> device driver doesn't know that.
>>
>> b) Never have to call dma_set_mask() for platform devices and assume
>> that the
>> platform code sets it up correctly. This would probably be the simpler
>> solution, and I can't think of any downsides at the moment.
>> >>>
>> >>> I don't think we want this. In the case of setting up 64-bit masters,
>> >>> it is typically the device that knows if it can do 64-bit DMA either
>> >>> thru a capabilities register or compatible value. That device specific
>> >>> knowledge should really stay within the device's driver.
>> >>
>> >> So you think we should still set a 64-bit mask in the "ranges" property
>> >> for devices that can only do 32-bit and let the driver figure it out?
>> >
>> > No, I think option a makes more sense.
>
> Ok.
>
>> > BTW, don't you mean dma-ranges?
>
> Yes, sorry about the confusion.
>
>> > A device has it's own mask which is typically going to be 32 or
>> > 64-bit. The core code may not know what the device supports, so the
>> > device has to set it's own mask. I don't see a way around that. DT is
>> > not a good option because it is discoverable in some cases.
>
> What is "it"? Do you mean the mask of the device is discoverable,
> or something else?
Right, the mask of the device (before applying any restrictions of the
bus) is discoverable.
>> > Isn't the question here how do we handle restrictions added by the
>> > bus? It seems while this series adds support for handling dma-ranges
>> > to set the default case, it also needs to handle when the driver sets
>> > the mask. Is this not simply a matter of having dma_set_mask also
>> > parse dma-ranges (or reuse a saved bus mask)?
>
> With the current proposed implementation, the device also has to set
> a "dma-ranges" property to get any DMA support, we don't just take
> the mask of the parent bus. This is important because the translation
> does not have to be 1:1 but there can be an offset between the device
> and the parent. I'd prefer to leave this explicit.
But according to Ben H, dma-ranges is a bridge (i.e. parent) property
and not a device property[1]. So I don't think the device's mask
(again, before any bus restrictions) belongs in DT. A given IP block
is going to have some number of address bits for DMA. How it is hooked
up into an SOC is a separate issue. The driver should know its mask
whether it is fixed, discoverable, or tied to the compatible string.
Santosh's patch only looks for dma-ranges in parent nodes which I
believe is correct.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/01.html
>> >> I think this approach is much less useful for platform devices than it is
>> >> for PCI devices, where we don't explicitly describe the "ranges" for each
>> >> device. Are you thinking of off-chip or on-chip DMA masters here? If
>> >> on-chip, I don't think it's likely that we would end up with different
>> >> versions of the chip that have the same device on there but not the
>> >> same DMA capabilities.
>> >
>> > Sounds like a challenge to h/w designers. :)
>> >
>> > I'm mainly thinking about the midway case where all masters are 32-bit
>> > except AHCI which is 64-bit. The core libahci sets the mask based on
>> > the capabilities register regardless of PCI or platform bus. Requiring
>> > this to be setup by the platform or in the DT seems like a step
>> > backwards. A slightly more complicated case would be something like
>> > midway, but with RAM starting at say 2GB and DMA masters have the same
>> > view as the cpu (i.e. no offset). Then the platform does need to set
>> > the mask to (2^31 -1).
>
> So how would you show this in DT? I'd assume that some of the other
> devices on midway have drivers that may also try to set a 64-bit mask,
> like EHCI for instance does. Is AHCI the only one that sits on a 64-bit
> wide bus, like this?
>
> axi {
> #address-cells = <2>;
> #size-cells = <2>;
> dma-ranges = <0 0 0 0 0x100 0>; /* max phys address space */
>
> ahci {
> dma-ranges = <0 0 0 0 0x100
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Wed, Mar 12, 2014 at 11:58 AM, Arnd Bergmann a...@arndb.de wrote:
On Wednesday 12 March 2014 15:19:48 Grygorii Strashko wrote:
On 03/12/2014 02:15 AM, Rob Herring wrote:
On Sun, Mar 9, 2014 at 12:39 AM, Arnd Bergmann a...@arndb.de wrote:
On Saturday 08 March 2014, Rob Herring wrote:
On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann a...@arndb.de wrote:
a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32)
on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on
devices
that would like to do more than that, or call e.g.
dma_set_mask(DMA_MASK(28))
for devices that can do less than 32 bit, as given in the argument. This
approach would be most consistent with the way PCI works, but it doesn't
really work well for the case where the mask is less than 32-bit and the
device driver doesn't know that.
b) Never have to call dma_set_mask() for platform devices and assume
that the
platform code sets it up correctly. This would probably be the simpler
solution, and I can't think of any downsides at the moment.
I don't think we want this. In the case of setting up 64-bit masters,
it is typically the device that knows if it can do 64-bit DMA either
thru a capabilities register or compatible value. That device specific
knowledge should really stay within the device's driver.
So you think we should still set a 64-bit mask in the ranges property
for devices that can only do 32-bit and let the driver figure it out?
No, I think option a makes more sense.
Ok.
BTW, don't you mean dma-ranges?
Yes, sorry about the confusion.
A device has it's own mask which is typically going to be 32 or
64-bit. The core code may not know what the device supports, so the
device has to set it's own mask. I don't see a way around that. DT is
not a good option because it is discoverable in some cases.
What is it? Do you mean the mask of the device is discoverable,
or something else?
Right, the mask of the device (before applying any restrictions of the
bus) is discoverable.
Isn't the question here how do we handle restrictions added by the
bus? It seems while this series adds support for handling dma-ranges
to set the default case, it also needs to handle when the driver sets
the mask. Is this not simply a matter of having dma_set_mask also
parse dma-ranges (or reuse a saved bus mask)?
With the current proposed implementation, the device also has to set
a dma-ranges property to get any DMA support, we don't just take
the mask of the parent bus. This is important because the translation
does not have to be 1:1 but there can be an offset between the device
and the parent. I'd prefer to leave this explicit.
But according to Ben H, dma-ranges is a bridge (i.e. parent) property
and not a device property[1]. So I don't think the device's mask
(again, before any bus restrictions) belongs in DT. A given IP block
is going to have some number of address bits for DMA. How it is hooked
up into an SOC is a separate issue. The driver should know its mask
whether it is fixed, discoverable, or tied to the compatible string.
Santosh's patch only looks for dma-ranges in parent nodes which I
believe is correct.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/01.html
I think this approach is much less useful for platform devices than it is
for PCI devices, where we don't explicitly describe the ranges for each
device. Are you thinking of off-chip or on-chip DMA masters here? If
on-chip, I don't think it's likely that we would end up with different
versions of the chip that have the same device on there but not the
same DMA capabilities.
Sounds like a challenge to h/w designers. :)
I'm mainly thinking about the midway case where all masters are 32-bit
except AHCI which is 64-bit. The core libahci sets the mask based on
the capabilities register regardless of PCI or platform bus. Requiring
this to be setup by the platform or in the DT seems like a step
backwards. A slightly more complicated case would be something like
midway, but with RAM starting at say 2GB and DMA masters have the same
view as the cpu (i.e. no offset). Then the platform does need to set
the mask to (2^31 -1).
So how would you show this in DT? I'd assume that some of the other
devices on midway have drivers that may also try to set a 64-bit mask,
like EHCI for instance does. Is AHCI the only one that sits on a 64-bit
wide bus, like this?
axi {
#address-cells = 2;
#size-cells = 2;
dma-ranges = 0 0 0 0 0x100 0; /* max phys address space */
ahci {
dma-ranges = 0 0 0 0 0x100 0;
There is no point to this dma-ranges here. Based on the capabilities
reg, we know that we can do either 32 or 64 bit DMA. In the case of
64-bit DMA, the device should try to set its mask to DMA_MASK(64), but
the parent dma-ranges
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Friday 14 March 2014, Rob Herring wrote:
On Wed, Mar 12, 2014 at 11:58 AM, Arnd Bergmann a...@arndb.de wrote:
On Wednesday 12 March 2014 15:19:48 Grygorii Strashko wrote:
Isn't the question here how do we handle restrictions added by the
bus? It seems while this series adds support for handling dma-ranges
to set the default case, it also needs to handle when the driver sets
the mask. Is this not simply a matter of having dma_set_mask also
parse dma-ranges (or reuse a saved bus mask)?
With the current proposed implementation, the device also has to set
a dma-ranges property to get any DMA support, we don't just take
the mask of the parent bus. This is important because the translation
does not have to be 1:1 but there can be an offset between the device
and the parent. I'd prefer to leave this explicit.
But according to Ben H, dma-ranges is a bridge (i.e. parent) property
and not a device property[1]. So I don't think the device's mask
(again, before any bus restrictions) belongs in DT. A given IP block
is going to have some number of address bits for DMA. How it is hooked
up into an SOC is a separate issue. The driver should know its mask
whether it is fixed, discoverable, or tied to the compatible string.
Santosh's patch only looks for dma-ranges in parent nodes which I
believe is correct.
Ok, good point.
I think this approach is much less useful for platform devices than it
is
for PCI devices, where we don't explicitly describe the ranges for
each
device. Are you thinking of off-chip or on-chip DMA masters here? If
on-chip, I don't think it's likely that we would end up with different
versions of the chip that have the same device on there but not the
same DMA capabilities.
Sounds like a challenge to h/w designers. :)
I'm mainly thinking about the midway case where all masters are 32-bit
except AHCI which is 64-bit. The core libahci sets the mask based on
the capabilities register regardless of PCI or platform bus. Requiring
this to be setup by the platform or in the DT seems like a step
backwards. A slightly more complicated case would be something like
midway, but with RAM starting at say 2GB and DMA masters have the same
view as the cpu (i.e. no offset). Then the platform does need to set
the mask to (2^31 -1).
So how would you show this in DT? I'd assume that some of the other
devices on midway have drivers that may also try to set a 64-bit mask,
like EHCI for instance does. Is AHCI the only one that sits on a 64-bit
wide bus, like this?
axi {
#address-cells = 2;
#size-cells = 2;
dma-ranges = 0 0 0 0 0x100 0; /* max phys address space */
ahci {
dma-ranges = 0 0 0 0 0x100 0;
There is no point to this dma-ranges here. Based on the capabilities
reg, we know that we can do either 32 or 64 bit DMA.
Ok
In the case of
64-bit DMA, the device should try to set its mask to DMA_MASK(64), but
the parent dma-ranges should restrict that to DMA_MASK(40).
Now I'm confused about what dma_set_mask is actually supposed to do here.
I /think/ it should fail the DMA_MASK(64) call if the bus supports
less than 64 bits as in the above example. However it seems like a
valid shortcut to always succeed here if the effective mask covers
all of the installed RAM.
That would mean that even if we only have a 32-bit bus, but all RAM
resides below 4GB, a call to dma_set_mask using DMA_MASK(64) would
also succeed.
...
};
ahb {
#address-cells = 1;
#size-cells = 1;
This would need to be 2. ePAPR says the size cells size is determined
by #size-cells in the same node.
Ah, of course.
dma-ranges = 0 0 0 0x1 0; /* only 4GB */
ehci {
dma-ranges = 0 0 0x;
Again, I don't think this is needed. Here, regardless of the device's
capabilities, the bus is restricting the mask to DMA_MASK(32).
Right.
/* side-question: is that the right way
to describe 4GB length? it seems missing
a byte */
};
};
};
BTW, I hope I understood you right that you wouldn't actually trust the
capabilities register by itself but only allow setting the 64-bit mask
in the arch code if the DT doesn't have any information about the
bus being incapable of doing this, right?
Ideally no, but it appears we are ATM for midway. We get away with it
since it is midway/highbank specific driver setup and know there are
not any restrictions in addressing RAM. I think we have to keep bus
and device masks separate and the final mask is the AND of them.
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Wednesday 12 March 2014 15:19:48 Grygorii Strashko wrote:
> On 03/12/2014 02:15 AM, Rob Herring wrote:
> > On Sun, Mar 9, 2014 at 12:39 AM, Arnd Bergmann wrote:
> >> On Saturday 08 March 2014, Rob Herring wrote:
> >>> On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann wrote:
> >>
> a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32)
> on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on
> devices
> that would like to do more than that, or call e.g.
> dma_set_mask(DMA_MASK(28))
> for devices that can do less than 32 bit, as given in the argument. This
> approach would be most consistent with the way PCI works, but it doesn't
> really work well for the case where the mask is less than 32-bit and the
> device driver doesn't know that.
>
> b) Never have to call dma_set_mask() for platform devices and assume
> that the
> platform code sets it up correctly. This would probably be the simpler
> solution, and I can't think of any downsides at the moment.
> >>>
> >>> I don't think we want this. In the case of setting up 64-bit masters,
> >>> it is typically the device that knows if it can do 64-bit DMA either
> >>> thru a capabilities register or compatible value. That device specific
> >>> knowledge should really stay within the device's driver.
> >>
> >> So you think we should still set a 64-bit mask in the "ranges" property
> >> for devices that can only do 32-bit and let the driver figure it out?
> >
> > No, I think option a makes more sense.
Ok.
> > BTW, don't you mean dma-ranges?
Yes, sorry about the confusion.
> > A device has it's own mask which is typically going to be 32 or
> > 64-bit. The core code may not know what the device supports, so the
> > device has to set it's own mask. I don't see a way around that. DT is
> > not a good option because it is discoverable in some cases.
What is "it"? Do you mean the mask of the device is discoverable,
or something else?
> > Isn't the question here how do we handle restrictions added by the
> > bus? It seems while this series adds support for handling dma-ranges
> > to set the default case, it also needs to handle when the driver sets
> > the mask. Is this not simply a matter of having dma_set_mask also
> > parse dma-ranges (or reuse a saved bus mask)?
With the current proposed implementation, the device also has to set
a "dma-ranges" property to get any DMA support, we don't just take
the mask of the parent bus. This is important because the translation
does not have to be 1:1 but there can be an offset between the device
and the parent. I'd prefer to leave this explicit.
> >> I think this approach is much less useful for platform devices than it is
> >> for PCI devices, where we don't explicitly describe the "ranges" for each
> >> device. Are you thinking of off-chip or on-chip DMA masters here? If
> >> on-chip, I don't think it's likely that we would end up with different
> >> versions of the chip that have the same device on there but not the
> >> same DMA capabilities.
> >
> > Sounds like a challenge to h/w designers. :)
> >
> > I'm mainly thinking about the midway case where all masters are 32-bit
> > except AHCI which is 64-bit. The core libahci sets the mask based on
> > the capabilities register regardless of PCI or platform bus. Requiring
> > this to be setup by the platform or in the DT seems like a step
> > backwards. A slightly more complicated case would be something like
> > midway, but with RAM starting at say 2GB and DMA masters have the same
> > view as the cpu (i.e. no offset). Then the platform does need to set
> > the mask to (2^31 -1).
So how would you show this in DT? I'd assume that some of the other
devices on midway have drivers that may also try to set a 64-bit mask,
like EHCI for instance does. Is AHCI the only one that sits on a 64-bit
wide bus, like this?
axi {
#address-cells = <2>;
#size-cells = <2>;
dma-ranges = <0 0 0 0 0x100 0>; /* max phys address space */
ahci {
dma-ranges = <0 0 0 0 0x100 0>;
...
};
ahb {
#address-cells = <1>;
#size-cells = <1>;
dma-ranges = <0 0 0 0x1 0>; /* only 4GB */
ehci {
dma-ranges = <0 0 0x>;
/* side-question: is that the right way
to describe 4GB length? it seems missing
a byte */
};
};
};
BTW, I hope I understood you right that you wouldn't actually trust the
capabilities register by itself but only allow setting the 64-bit mask
in the arch code if the DT doesn't have any information about the
bus being incapable of doing this, right?
>
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
Hi Rob, Arnd, On 03/12/2014 02:15 AM, Rob Herring wrote: > On Sun, Mar 9, 2014 at 12:39 AM, Arnd Bergmann wrote: >> On Saturday 08 March 2014, Rob Herring wrote: >>> On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann wrote: >> a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32) on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices that would like to do more than that, or call e.g. dma_set_mask(DMA_MASK(28)) for devices that can do less than 32 bit, as given in the argument. This approach would be most consistent with the way PCI works, but it doesn't really work well for the case where the mask is less than 32-bit and the device driver doesn't know that. b) Never have to call dma_set_mask() for platform devices and assume that the platform code sets it up correctly. This would probably be the simpler solution, and I can't think of any downsides at the moment. >>> >>> I don't think we want this. In the case of setting up 64-bit masters, >>> it is typically the device that knows if it can do 64-bit DMA either >>> thru a capabilities register or compatible value. That device specific >>> knowledge should really stay within the device's driver. >> >> So you think we should still set a 64-bit mask in the "ranges" property >> for devices that can only do 32-bit and let the driver figure it out? > > No, I think option a makes more sense. BTW, don't you mean dma-ranges? > A device has it's own mask which is typically going to be 32 or > 64-bit. The core code may not know what the device supports, so the > device has to set it's own mask. I don't see a way around that. DT is > not a good option because it is discoverable in some cases. > > Isn't the question here how do we handle restrictions added by the > bus? It seems while this series adds support for handling dma-ranges > to set the default case, it also needs to handle when the driver sets > the mask. Is this not simply a matter of having dma_set_mask also > parse dma-ranges (or reuse a saved bus mask)? > >> I think this approach is much less useful for platform devices than it is >> for PCI devices, where we don't explicitly describe the "ranges" for each >> device. Are you thinking of off-chip or on-chip DMA masters here? If >> on-chip, I don't think it's likely that we would end up with different >> versions of the chip that have the same device on there but not the >> same DMA capabilities. > > Sounds like a challenge to h/w designers. :) > > I'm mainly thinking about the midway case where all masters are 32-bit > except AHCI which is 64-bit. The core libahci sets the mask based on > the capabilities register regardless of PCI or platform bus. Requiring > this to be setup by the platform or in the DT seems like a step > backwards. A slightly more complicated case would be something like > midway, but with RAM starting at say 2GB and DMA masters have the same > view as the cpu (i.e. no offset). Then the platform does need to set > the mask to (2^31 -1). > mask has to be (2^32 -1). I'd like to add my 5 cents here :) First of all this solution is pretended to be generic, so by default it can't handle all possible cases (corner cases) - only widely used. Arch or driver can always fix up it through bus notifier or driver's probe. Second, it works only for DT-boot case when devices instantiated from DT in generic way. So, if any bus creates devices in its own way - proper DMA configuration of such devices will be private problem of this bus. I've tried to formulate possible use cases which can be and can't be covered by this approach: [1] 32 bit SoC: - no DMA range limitation: DMA range has to be absent or be equal to RAM (default DMA configuration will be applied) - DMA range present and It's defined inside RAM (no address translation needed): DMA range can be present and depending on its size dma_mask will be equal or less than DMA_MASK(32). (default DMA configuration will be applied - if DMA range isn't present) - DMA range present and It's defined outside of RAM (address translation needed): DMA range has to be present and depending on its size dma_mask will be equal or less than DMA_MASK(32); and dma_pfn_offset will be calculated. Compatibility issues: - Old DT without DMA properties defined and DMA range is defined outside of RAM: arch or drivers code has to provide proper address translation routines. [2] 64 bit SOC (only 64 bit masters): - no DMA range limitation: DMA range has to be present and equal to RAM - DMA range present and It's defined inside RAM (no address translation needed): DMA range has to be present and depending on its size dma_mask will be equal or less than DMA_MASK(64). - DMA range present and It's defined outside of RAM (address translation needed): DMA range has to be present and depending on its size dma_mask will be equal or less than DMA_MASK(64); and dma_pfn_offset will be
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Wednesday 12 March 2014 15:19:48 Grygorii Strashko wrote:
On 03/12/2014 02:15 AM, Rob Herring wrote:
On Sun, Mar 9, 2014 at 12:39 AM, Arnd Bergmann a...@arndb.de wrote:
On Saturday 08 March 2014, Rob Herring wrote:
On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann a...@arndb.de wrote:
a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32)
on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on
devices
that would like to do more than that, or call e.g.
dma_set_mask(DMA_MASK(28))
for devices that can do less than 32 bit, as given in the argument. This
approach would be most consistent with the way PCI works, but it doesn't
really work well for the case where the mask is less than 32-bit and the
device driver doesn't know that.
b) Never have to call dma_set_mask() for platform devices and assume
that the
platform code sets it up correctly. This would probably be the simpler
solution, and I can't think of any downsides at the moment.
I don't think we want this. In the case of setting up 64-bit masters,
it is typically the device that knows if it can do 64-bit DMA either
thru a capabilities register or compatible value. That device specific
knowledge should really stay within the device's driver.
So you think we should still set a 64-bit mask in the ranges property
for devices that can only do 32-bit and let the driver figure it out?
No, I think option a makes more sense.
Ok.
BTW, don't you mean dma-ranges?
Yes, sorry about the confusion.
A device has it's own mask which is typically going to be 32 or
64-bit. The core code may not know what the device supports, so the
device has to set it's own mask. I don't see a way around that. DT is
not a good option because it is discoverable in some cases.
What is it? Do you mean the mask of the device is discoverable,
or something else?
Isn't the question here how do we handle restrictions added by the
bus? It seems while this series adds support for handling dma-ranges
to set the default case, it also needs to handle when the driver sets
the mask. Is this not simply a matter of having dma_set_mask also
parse dma-ranges (or reuse a saved bus mask)?
With the current proposed implementation, the device also has to set
a dma-ranges property to get any DMA support, we don't just take
the mask of the parent bus. This is important because the translation
does not have to be 1:1 but there can be an offset between the device
and the parent. I'd prefer to leave this explicit.
I think this approach is much less useful for platform devices than it is
for PCI devices, where we don't explicitly describe the ranges for each
device. Are you thinking of off-chip or on-chip DMA masters here? If
on-chip, I don't think it's likely that we would end up with different
versions of the chip that have the same device on there but not the
same DMA capabilities.
Sounds like a challenge to h/w designers. :)
I'm mainly thinking about the midway case where all masters are 32-bit
except AHCI which is 64-bit. The core libahci sets the mask based on
the capabilities register regardless of PCI or platform bus. Requiring
this to be setup by the platform or in the DT seems like a step
backwards. A slightly more complicated case would be something like
midway, but with RAM starting at say 2GB and DMA masters have the same
view as the cpu (i.e. no offset). Then the platform does need to set
the mask to (2^31 -1).
So how would you show this in DT? I'd assume that some of the other
devices on midway have drivers that may also try to set a 64-bit mask,
like EHCI for instance does. Is AHCI the only one that sits on a 64-bit
wide bus, like this?
axi {
#address-cells = 2;
#size-cells = 2;
dma-ranges = 0 0 0 0 0x100 0; /* max phys address space */
ahci {
dma-ranges = 0 0 0 0 0x100 0;
...
};
ahb {
#address-cells = 1;
#size-cells = 1;
dma-ranges = 0 0 0 0x1 0; /* only 4GB */
ehci {
dma-ranges = 0 0 0x;
/* side-question: is that the right way
to describe 4GB length? it seems missing
a byte */
};
};
};
BTW, I hope I understood you right that you wouldn't actually trust the
capabilities register by itself but only allow setting the 64-bit mask
in the arch code if the DT doesn't have any information about the
bus being incapable of doing this, right?
mask has to be (2^32 -1).
Right.
I'd like to add my 5 cents here :)
First of all this solution is pretended to be generic, so by default it
can't handle all possible cases
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
Hi Rob, Arnd, On 03/12/2014 02:15 AM, Rob Herring wrote: On Sun, Mar 9, 2014 at 12:39 AM, Arnd Bergmann a...@arndb.de wrote: On Saturday 08 March 2014, Rob Herring wrote: On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann a...@arndb.de wrote: a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32) on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices that would like to do more than that, or call e.g. dma_set_mask(DMA_MASK(28)) for devices that can do less than 32 bit, as given in the argument. This approach would be most consistent with the way PCI works, but it doesn't really work well for the case where the mask is less than 32-bit and the device driver doesn't know that. b) Never have to call dma_set_mask() for platform devices and assume that the platform code sets it up correctly. This would probably be the simpler solution, and I can't think of any downsides at the moment. I don't think we want this. In the case of setting up 64-bit masters, it is typically the device that knows if it can do 64-bit DMA either thru a capabilities register or compatible value. That device specific knowledge should really stay within the device's driver. So you think we should still set a 64-bit mask in the ranges property for devices that can only do 32-bit and let the driver figure it out? No, I think option a makes more sense. BTW, don't you mean dma-ranges? A device has it's own mask which is typically going to be 32 or 64-bit. The core code may not know what the device supports, so the device has to set it's own mask. I don't see a way around that. DT is not a good option because it is discoverable in some cases. Isn't the question here how do we handle restrictions added by the bus? It seems while this series adds support for handling dma-ranges to set the default case, it also needs to handle when the driver sets the mask. Is this not simply a matter of having dma_set_mask also parse dma-ranges (or reuse a saved bus mask)? I think this approach is much less useful for platform devices than it is for PCI devices, where we don't explicitly describe the ranges for each device. Are you thinking of off-chip or on-chip DMA masters here? If on-chip, I don't think it's likely that we would end up with different versions of the chip that have the same device on there but not the same DMA capabilities. Sounds like a challenge to h/w designers. :) I'm mainly thinking about the midway case where all masters are 32-bit except AHCI which is 64-bit. The core libahci sets the mask based on the capabilities register regardless of PCI or platform bus. Requiring this to be setup by the platform or in the DT seems like a step backwards. A slightly more complicated case would be something like midway, but with RAM starting at say 2GB and DMA masters have the same view as the cpu (i.e. no offset). Then the platform does need to set the mask to (2^31 -1). mask has to be (2^32 -1). I'd like to add my 5 cents here :) First of all this solution is pretended to be generic, so by default it can't handle all possible cases (corner cases) - only widely used. Arch or driver can always fix up it through bus notifier or driver's probe. Second, it works only for DT-boot case when devices instantiated from DT in generic way. So, if any bus creates devices in its own way - proper DMA configuration of such devices will be private problem of this bus. I've tried to formulate possible use cases which can be and can't be covered by this approach: [1] 32 bit SoC: - no DMA range limitation: DMA range has to be absent or be equal to RAM (default DMA configuration will be applied) - DMA range present and It's defined inside RAM (no address translation needed): DMA range can be present and depending on its size dma_mask will be equal or less than DMA_MASK(32). (default DMA configuration will be applied - if DMA range isn't present) - DMA range present and It's defined outside of RAM (address translation needed): DMA range has to be present and depending on its size dma_mask will be equal or less than DMA_MASK(32); and dma_pfn_offset will be calculated. Compatibility issues: - Old DT without DMA properties defined and DMA range is defined outside of RAM: arch or drivers code has to provide proper address translation routines. [2] 64 bit SOC (only 64 bit masters): - no DMA range limitation: DMA range has to be present and equal to RAM - DMA range present and It's defined inside RAM (no address translation needed): DMA range has to be present and depending on its size dma_mask will be equal or less than DMA_MASK(64). - DMA range present and It's defined outside of RAM (address translation needed): DMA range has to be present and depending on its size dma_mask will be equal or less than DMA_MASK(64); and dma_pfn_offset will be calculated. Compatibility issues: - Old DT without DMA properties defined: Drivers may still
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Sun, Mar 9, 2014 at 12:39 AM, Arnd Bergmann wrote: > On Saturday 08 March 2014, Rob Herring wrote: >> On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann wrote: > >> > a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32) >> > on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices >> > that would like to do more than that, or call e.g. >> > dma_set_mask(DMA_MASK(28)) >> > for devices that can do less than 32 bit, as given in the argument. This >> > approach would be most consistent with the way PCI works, but it doesn't >> > really work well for the case where the mask is less than 32-bit and the >> > device driver doesn't know that. >> > >> > b) Never have to call dma_set_mask() for platform devices and assume that >> > the >> > platform code sets it up correctly. This would probably be the simpler >> > solution, and I can't think of any downsides at the moment. >> >> I don't think we want this. In the case of setting up 64-bit masters, >> it is typically the device that knows if it can do 64-bit DMA either >> thru a capabilities register or compatible value. That device specific >> knowledge should really stay within the device's driver. > > So you think we should still set a 64-bit mask in the "ranges" property > for devices that can only do 32-bit and let the driver figure it out? No, I think option a makes more sense. BTW, don't you mean dma-ranges? A device has it's own mask which is typically going to be 32 or 64-bit. The core code may not know what the device supports, so the device has to set it's own mask. I don't see a way around that. DT is not a good option because it is discoverable in some cases. Isn't the question here how do we handle restrictions added by the bus? It seems while this series adds support for handling dma-ranges to set the default case, it also needs to handle when the driver sets the mask. Is this not simply a matter of having dma_set_mask also parse dma-ranges (or reuse a saved bus mask)? > I think this approach is much less useful for platform devices than it is > for PCI devices, where we don't explicitly describe the "ranges" for each > device. Are you thinking of off-chip or on-chip DMA masters here? If > on-chip, I don't think it's likely that we would end up with different > versions of the chip that have the same device on there but not the > same DMA capabilities. Sounds like a challenge to h/w designers. :) I'm mainly thinking about the midway case where all masters are 32-bit except AHCI which is 64-bit. The core libahci sets the mask based on the capabilities register regardless of PCI or platform bus. Requiring this to be setup by the platform or in the DT seems like a step backwards. A slightly more complicated case would be something like midway, but with RAM starting at say 2GB and DMA masters have the same view as the cpu (i.e. no offset). Then the platform does need to set the mask to (2^31 -1). Rob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Saturday 08 March 2014, Rob Herring wrote: > On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann wrote: > > a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32) > > on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices > > that would like to do more than that, or call e.g. > > dma_set_mask(DMA_MASK(28)) > > for devices that can do less than 32 bit, as given in the argument. This > > approach would be most consistent with the way PCI works, but it doesn't > > really work well for the case where the mask is less than 32-bit and the > > device driver doesn't know that. > > > > b) Never have to call dma_set_mask() for platform devices and assume that > > the > > platform code sets it up correctly. This would probably be the simpler > > solution, and I can't think of any downsides at the moment. > > I don't think we want this. In the case of setting up 64-bit masters, > it is typically the device that knows if it can do 64-bit DMA either > thru a capabilities register or compatible value. That device specific > knowledge should really stay within the device's driver. So you think we should still set a 64-bit mask in the "ranges" property for devices that can only do 32-bit and let the driver figure it out? I think this approach is much less useful for platform devices than it is for PCI devices, where we don't explicitly describe the "ranges" for each device. Are you thinking of off-chip or on-chip DMA masters here? If on-chip, I don't think it's likely that we would end up with different versions of the chip that have the same device on there but not the same DMA capabilities. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Saturday 08 March 2014, Rob Herring wrote: On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann a...@arndb.de wrote: a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32) on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices that would like to do more than that, or call e.g. dma_set_mask(DMA_MASK(28)) for devices that can do less than 32 bit, as given in the argument. This approach would be most consistent with the way PCI works, but it doesn't really work well for the case where the mask is less than 32-bit and the device driver doesn't know that. b) Never have to call dma_set_mask() for platform devices and assume that the platform code sets it up correctly. This would probably be the simpler solution, and I can't think of any downsides at the moment. I don't think we want this. In the case of setting up 64-bit masters, it is typically the device that knows if it can do 64-bit DMA either thru a capabilities register or compatible value. That device specific knowledge should really stay within the device's driver. So you think we should still set a 64-bit mask in the ranges property for devices that can only do 32-bit and let the driver figure it out? I think this approach is much less useful for platform devices than it is for PCI devices, where we don't explicitly describe the ranges for each device. Are you thinking of off-chip or on-chip DMA masters here? If on-chip, I don't think it's likely that we would end up with different versions of the chip that have the same device on there but not the same DMA capabilities. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Sun, Mar 9, 2014 at 12:39 AM, Arnd Bergmann a...@arndb.de wrote: On Saturday 08 March 2014, Rob Herring wrote: On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann a...@arndb.de wrote: a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32) on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices that would like to do more than that, or call e.g. dma_set_mask(DMA_MASK(28)) for devices that can do less than 32 bit, as given in the argument. This approach would be most consistent with the way PCI works, but it doesn't really work well for the case where the mask is less than 32-bit and the device driver doesn't know that. b) Never have to call dma_set_mask() for platform devices and assume that the platform code sets it up correctly. This would probably be the simpler solution, and I can't think of any downsides at the moment. I don't think we want this. In the case of setting up 64-bit masters, it is typically the device that knows if it can do 64-bit DMA either thru a capabilities register or compatible value. That device specific knowledge should really stay within the device's driver. So you think we should still set a 64-bit mask in the ranges property for devices that can only do 32-bit and let the driver figure it out? No, I think option a makes more sense. BTW, don't you mean dma-ranges? A device has it's own mask which is typically going to be 32 or 64-bit. The core code may not know what the device supports, so the device has to set it's own mask. I don't see a way around that. DT is not a good option because it is discoverable in some cases. Isn't the question here how do we handle restrictions added by the bus? It seems while this series adds support for handling dma-ranges to set the default case, it also needs to handle when the driver sets the mask. Is this not simply a matter of having dma_set_mask also parse dma-ranges (or reuse a saved bus mask)? I think this approach is much less useful for platform devices than it is for PCI devices, where we don't explicitly describe the ranges for each device. Are you thinking of off-chip or on-chip DMA masters here? If on-chip, I don't think it's likely that we would end up with different versions of the chip that have the same device on there but not the same DMA capabilities. Sounds like a challenge to h/w designers. :) I'm mainly thinking about the midway case where all masters are 32-bit except AHCI which is 64-bit. The core libahci sets the mask based on the capabilities register regardless of PCI or platform bus. Requiring this to be setup by the platform or in the DT seems like a step backwards. A slightly more complicated case would be something like midway, but with RAM starting at say 2GB and DMA masters have the same view as the cpu (i.e. no offset). Then the platform does need to set the mask to (2^31 -1). Rob -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann wrote:
> On Friday 07 March 2014, Santosh Shilimkar wrote:
>> >> +
>> >> + ret = dma_set_mask(dev, dma_mask);
>> >> + if (ret < 0) {
>> >> + dev_err(dev, "failed to set DMA mask %pad\n", _mask);
>> >> + dev->dma_mask = NULL;
>> >> + return;
>> >> + }
>> >> +
>> >> + dev_dbg(dev, "dma_pfn_offset(%#08lx) dma_mask(%pad)\n",
>> >> + dev->dma_pfn_offset, dev->dma_mask);
>> >> +
>> >> + ret = dma_set_coherent_mask(dev, dma_mask);
>> >
>> > I think these 2 calls belong in the drivers, not here.
>> >
>> I also had same initial thought but Arnd mentioned that its a
>> shared responsibility between ARCH and drivers. Driver which
>> could be common between arches not always have the correct
>> mask information and it can change based on which arch it
>> is running.
>>
>> With some discussion back and forth, we thought updating
>> the dma_mask while the device getting created, would be
>> better place since we can find the arch capability at
>> this centralise code and update it.
>>
>> Ofcourse its bit debatable as the question you asked is
>> bit obvious as well. I let Arnd give his view here.
>
> If we set the mask *here*, we probably don't want to call 'dma_set_mask', but
> write to the mask directly, or we could call dma_coerce_mask_and_coherent(),
> which is really for overriding the mask pointer and value at once in cases
> where you absolutely know what it should be.
>
> We do need to decide what interface we want to use in platform device drivers,
> and I'm hoping that Russell has some idea which one he prefers:
>
> a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32)
> on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices
> that would like to do more than that, or call e.g. dma_set_mask(DMA_MASK(28))
> for devices that can do less than 32 bit, as given in the argument. This
> approach would be most consistent with the way PCI works, but it doesn't
> really work well for the case where the mask is less than 32-bit and the
> device driver doesn't know that.
>
> b) Never have to call dma_set_mask() for platform devices and assume that the
> platform code sets it up correctly. This would probably be the simpler
> solution, and I can't think of any downsides at the moment.
I don't think we want this. In the case of setting up 64-bit masters,
it is typically the device that knows if it can do 64-bit DMA either
thru a capabilities register or compatible value. That device specific
knowledge should really stay within the device's driver.
Rob
>
> In either case we probably want to call something like dt_dma_configure()
> from dma_set_mask() again to make sure that we stay within the limits
> imposed by the bus structure.
>
> Arnd
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Fri, Mar 7, 2014 at 10:02 AM, Arnd Bergmann a...@arndb.de wrote:
On Friday 07 March 2014, Santosh Shilimkar wrote:
+
+ ret = dma_set_mask(dev, dma_mask);
+ if (ret 0) {
+ dev_err(dev, failed to set DMA mask %pad\n, dma_mask);
+ dev-dma_mask = NULL;
+ return;
+ }
+
+ dev_dbg(dev, dma_pfn_offset(%#08lx) dma_mask(%pad)\n,
+ dev-dma_pfn_offset, dev-dma_mask);
+
+ ret = dma_set_coherent_mask(dev, dma_mask);
I think these 2 calls belong in the drivers, not here.
I also had same initial thought but Arnd mentioned that its a
shared responsibility between ARCH and drivers. Driver which
could be common between arches not always have the correct
mask information and it can change based on which arch it
is running.
With some discussion back and forth, we thought updating
the dma_mask while the device getting created, would be
better place since we can find the arch capability at
this centralise code and update it.
Ofcourse its bit debatable as the question you asked is
bit obvious as well. I let Arnd give his view here.
If we set the mask *here*, we probably don't want to call 'dma_set_mask', but
write to the mask directly, or we could call dma_coerce_mask_and_coherent(),
which is really for overriding the mask pointer and value at once in cases
where you absolutely know what it should be.
We do need to decide what interface we want to use in platform device drivers,
and I'm hoping that Russell has some idea which one he prefers:
a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32)
on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices
that would like to do more than that, or call e.g. dma_set_mask(DMA_MASK(28))
for devices that can do less than 32 bit, as given in the argument. This
approach would be most consistent with the way PCI works, but it doesn't
really work well for the case where the mask is less than 32-bit and the
device driver doesn't know that.
b) Never have to call dma_set_mask() for platform devices and assume that the
platform code sets it up correctly. This would probably be the simpler
solution, and I can't think of any downsides at the moment.
I don't think we want this. In the case of setting up 64-bit masters,
it is typically the device that knows if it can do 64-bit DMA either
thru a capabilities register or compatible value. That device specific
knowledge should really stay within the device's driver.
Rob
In either case we probably want to call something like dt_dma_configure()
from dma_set_mask() again to make sure that we stay within the limits
imposed by the bus structure.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Friday 07 March 2014, Santosh Shilimkar wrote:
> >> +
> >> + ret = dma_set_mask(dev, dma_mask);
> >> + if (ret < 0) {
> >> + dev_err(dev, "failed to set DMA mask %pad\n", _mask);
> >> + dev->dma_mask = NULL;
> >> + return;
> >> + }
> >> +
> >> + dev_dbg(dev, "dma_pfn_offset(%#08lx) dma_mask(%pad)\n",
> >> + dev->dma_pfn_offset, dev->dma_mask);
> >> +
> >> + ret = dma_set_coherent_mask(dev, dma_mask);
> >
> > I think these 2 calls belong in the drivers, not here.
> >
> I also had same initial thought but Arnd mentioned that its a
> shared responsibility between ARCH and drivers. Driver which
> could be common between arches not always have the correct
> mask information and it can change based on which arch it
> is running.
>
> With some discussion back and forth, we thought updating
> the dma_mask while the device getting created, would be
> better place since we can find the arch capability at
> this centralise code and update it.
>
> Ofcourse its bit debatable as the question you asked is
> bit obvious as well. I let Arnd give his view here.
If we set the mask *here*, we probably don't want to call 'dma_set_mask', but
write to the mask directly, or we could call dma_coerce_mask_and_coherent(),
which is really for overriding the mask pointer and value at once in cases
where you absolutely know what it should be.
We do need to decide what interface we want to use in platform device drivers,
and I'm hoping that Russell has some idea which one he prefers:
a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32)
on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices
that would like to do more than that, or call e.g. dma_set_mask(DMA_MASK(28))
for devices that can do less than 32 bit, as given in the argument. This
approach would be most consistent with the way PCI works, but it doesn't
really work well for the case where the mask is less than 32-bit and the
device driver doesn't know that.
b) Never have to call dma_set_mask() for platform devices and assume that the
platform code sets it up correctly. This would probably be the simpler
solution, and I can't think of any downsides at the moment.
In either case we probably want to call something like dt_dma_configure()
from dma_set_mask() again to make sure that we stay within the limits
imposed by the bus structure.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Friday 07 March 2014, Santosh Shilimkar wrote:
+
+ ret = dma_set_mask(dev, dma_mask);
+ if (ret 0) {
+ dev_err(dev, failed to set DMA mask %pad\n, dma_mask);
+ dev-dma_mask = NULL;
+ return;
+ }
+
+ dev_dbg(dev, dma_pfn_offset(%#08lx) dma_mask(%pad)\n,
+ dev-dma_pfn_offset, dev-dma_mask);
+
+ ret = dma_set_coherent_mask(dev, dma_mask);
I think these 2 calls belong in the drivers, not here.
I also had same initial thought but Arnd mentioned that its a
shared responsibility between ARCH and drivers. Driver which
could be common between arches not always have the correct
mask information and it can change based on which arch it
is running.
With some discussion back and forth, we thought updating
the dma_mask while the device getting created, would be
better place since we can find the arch capability at
this centralise code and update it.
Ofcourse its bit debatable as the question you asked is
bit obvious as well. I let Arnd give his view here.
If we set the mask *here*, we probably don't want to call 'dma_set_mask', but
write to the mask directly, or we could call dma_coerce_mask_and_coherent(),
which is really for overriding the mask pointer and value at once in cases
where you absolutely know what it should be.
We do need to decide what interface we want to use in platform device drivers,
and I'm hoping that Russell has some idea which one he prefers:
a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32)
on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices
that would like to do more than that, or call e.g. dma_set_mask(DMA_MASK(28))
for devices that can do less than 32 bit, as given in the argument. This
approach would be most consistent with the way PCI works, but it doesn't
really work well for the case where the mask is less than 32-bit and the
device driver doesn't know that.
b) Never have to call dma_set_mask() for platform devices and assume that the
platform code sets it up correctly. This would probably be the simpler
solution, and I can't think of any downsides at the moment.
In either case we probably want to call something like dt_dma_configure()
from dma_set_mask() again to make sure that we stay within the limits
imposed by the bus structure.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Friday 07 March 2014 11:49 AM, Rob Herring wrote:
> On Thu, Mar 6, 2014 at 3:19 AM, Santosh Shilimkar
> wrote:
>> From: Grygorii Strashko
>>
>> Retrieve DMA configuration from DT and setup platform device's DMA
>> parameters.
>>
>> The DMA configuration in DT has to be specified using "dma-ranges"
>> property if supported. The DMA configuration applied
>> by dt_dma_configure() as following:
>> - call of_get_dma_range() and get supported DMA range info
>>(dma_addr, cpu_addr, dma_size);
>> - if "not found" then fill dma_mask as DMA_BIT_MASK(32)
>>(this is default behaviour);
>> - if "failed" then clean up dma_mask (DMA not supported)
>> - if ok then update devices DMA configuration:
>> set dma_mask to (dma_addr + dma_size - 1)
>> set dma_pfn_offset to PFN_DOWN(cpu_addr - dma_addr)
>>
>> Cc: Greg Kroah-Hartman
>> Cc: Russell King
>> Cc: Arnd Bergmann
>> Cc: Olof Johansson
>> Cc: Grant Likely
>> Cc: Rob Herring
>> Cc: Catalin Marinas
>> Cc: Linus Walleij
>> Signed-off-by: Grygorii Strashko
>> Signed-off-by: Santosh Shilimkar
>> ---
>> drivers/of/platform.c | 75
>> +++--
>> 1 file changed, 72 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index bd080db..53bb12f 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -187,6 +187,77 @@ struct platform_device *of_device_alloc(struct
>> device_node *np,
>> EXPORT_SYMBOL(of_device_alloc);
>>
>> /**
>> + * dt_dma_configure - apply default DMA configuration from dt
[..]
>> +
>> + ret = dma_set_mask(dev, dma_mask);
>> + if (ret < 0) {
>> + dev_err(dev, "failed to set DMA mask %pad\n", _mask);
>> + dev->dma_mask = NULL;
>> + return;
>> + }
>> +
>> + dev_dbg(dev, "dma_pfn_offset(%#08lx) dma_mask(%pad)\n",
>> + dev->dma_pfn_offset, dev->dma_mask);
>> +
>> + ret = dma_set_coherent_mask(dev, dma_mask);
>
> I think these 2 calls belong in the drivers, not here.
>
I also had same initial thought but Arnd mentioned that its a
shared responsibility between ARCH and drivers. Driver which
could be common between arches not always have the correct
mask information and it can change based on which arch it
is running.
With some discussion back and forth, we thought updating
the dma_mask while the device getting created, would be
better place since we can find the arch capability at
this centralise code and update it.
Ofcourse its bit debatable as the question you asked is
bit obvious as well. I let Arnd give his view here.
Regards,
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Thu, Mar 6, 2014 at 3:19 AM, Santosh Shilimkar
wrote:
> From: Grygorii Strashko
>
> Retrieve DMA configuration from DT and setup platform device's DMA
> parameters.
>
> The DMA configuration in DT has to be specified using "dma-ranges"
> property if supported. The DMA configuration applied
> by dt_dma_configure() as following:
> - call of_get_dma_range() and get supported DMA range info
>(dma_addr, cpu_addr, dma_size);
> - if "not found" then fill dma_mask as DMA_BIT_MASK(32)
>(this is default behaviour);
> - if "failed" then clean up dma_mask (DMA not supported)
> - if ok then update devices DMA configuration:
> set dma_mask to (dma_addr + dma_size - 1)
> set dma_pfn_offset to PFN_DOWN(cpu_addr - dma_addr)
>
> Cc: Greg Kroah-Hartman
> Cc: Russell King
> Cc: Arnd Bergmann
> Cc: Olof Johansson
> Cc: Grant Likely
> Cc: Rob Herring
> Cc: Catalin Marinas
> Cc: Linus Walleij
> Signed-off-by: Grygorii Strashko
> Signed-off-by: Santosh Shilimkar
> ---
> drivers/of/platform.c | 75
> +++--
> 1 file changed, 72 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index bd080db..53bb12f 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -187,6 +187,77 @@ struct platform_device *of_device_alloc(struct
> device_node *np,
> EXPORT_SYMBOL(of_device_alloc);
>
> /**
> + * dt_dma_configure - apply default DMA configuration from dt
s/dt_/of_/
> + * @dev: Device to apply DMA configuration
> + *
> + * Try to get devices's DMA configuration from DT and apply it.
> + * The DMA configuration is represented in DT by "dma-ranges" property.
> + * It configures:
> + * dma_pfn_offset, dma_mask and coherent_dma_mask.
> + *
> + * In case if DMA configuration can't be acquired from DT the default one is
> + * applied:
> + * dma_pfn_offset = 0,
> + * dma_mask = DMA_BIT_MASK(32)
This is really set to _dma_mask.
> + * coherent_dma_mask = DMA_BIT_MASK(32).
> +
> + * In case if platform code need to use own DMA configuration - it can use
> + * Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event to fix up
> + * DMA configuration.
> + */
> +static void dt_dma_configure(struct device *dev)
s/dt_/of_/
> +{
> + dma_addr_t dma_addr;
> + phys_addr_t paddr, size;
> + dma_addr_t dma_mask;
> + int ret;
> +
> + /*
> +* if dma-ranges property doesn't exist - use 32 bits DMA mask
> +* by default and don't set skip archdata.dma_pfn_offset
> +*/
> + ret = of_dma_get_range(dev->of_node, _addr, , );
> + if (ret == -ENODEV) {
> + dev->coherent_dma_mask = DMA_BIT_MASK(32);
> + if (!dev->dma_mask)
> + dev->dma_mask = >coherent_dma_mask;
> + return;
> + }
> +
> + /* if failed - disable DMA for device */
> + if (ret < 0) {
> + dev_err(dev, "failed to configure DMA\n");
> + return;
> + }
> +
> + /* DMA ranges found. Calculate and set dma_pfn_offset */
> + dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
> +
> + /* Configure DMA mask */
> + dev->dma_mask = >coherent_dma_mask;
> + if (!dev->dma_mask)
This condition is impossible.
> + return;
> +
> + dma_mask = dma_addr + size - 1;
> +
> + ret = dma_set_mask(dev, dma_mask);
> + if (ret < 0) {
> + dev_err(dev, "failed to set DMA mask %pad\n", _mask);
> + dev->dma_mask = NULL;
> + return;
> + }
> +
> + dev_dbg(dev, "dma_pfn_offset(%#08lx) dma_mask(%pad)\n",
> + dev->dma_pfn_offset, dev->dma_mask);
> +
> + ret = dma_set_coherent_mask(dev, dma_mask);
I think these 2 calls belong in the drivers, not here.
> + if (ret < 0) {
> + dev_err(dev, "failed to set coherent DMA mask %pad\n",
> + _mask);
> + }
> +}
> +
> +/**
> * 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 +285,7 @@ static struct platform_device
> *of_platform_device_create_pdata(
> #if defined(CONFIG_MICROBLAZE)
> dev->archdata.dma_mask = 0xUL;
> #endif
> - dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> - if (!dev->dev.dma_mask)
> - dev->dev.dma_mask = >dev.coherent_dma_mask;
> + dt_dma_configure(>dev);
> dev->dev.bus = _bus_type;
> dev->dev.platform_data = platform_data;
>
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Thu, Mar 6, 2014 at 3:19 AM, Santosh Shilimkar
santosh.shilim...@ti.com wrote:
From: Grygorii Strashko grygorii.stras...@ti.com
Retrieve DMA configuration from DT and setup platform device's DMA
parameters.
The DMA configuration in DT has to be specified using dma-ranges
property if supported. The DMA configuration applied
by dt_dma_configure() as following:
- call of_get_dma_range() and get supported DMA range info
(dma_addr, cpu_addr, dma_size);
- if not found then fill dma_mask as DMA_BIT_MASK(32)
(this is default behaviour);
- if failed then clean up dma_mask (DMA not supported)
- if ok then update devices DMA configuration:
set dma_mask to (dma_addr + dma_size - 1)
set dma_pfn_offset to PFN_DOWN(cpu_addr - dma_addr)
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: Russell King li...@arm.linux.org.uk
Cc: Arnd Bergmann a...@arndb.de
Cc: Olof Johansson o...@lixom.net
Cc: Grant Likely grant.lik...@linaro.org
Cc: Rob Herring robh...@kernel.org
Cc: Catalin Marinas catalin.mari...@arm.com
Cc: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
drivers/of/platform.c | 75
+++--
1 file changed, 72 insertions(+), 3 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index bd080db..53bb12f 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -187,6 +187,77 @@ struct platform_device *of_device_alloc(struct
device_node *np,
EXPORT_SYMBOL(of_device_alloc);
/**
+ * dt_dma_configure - apply default DMA configuration from dt
s/dt_/of_/
+ * @dev: Device to apply DMA configuration
+ *
+ * Try to get devices's DMA configuration from DT and apply it.
+ * The DMA configuration is represented in DT by dma-ranges property.
+ * It configures:
+ * dma_pfn_offset, dma_mask and coherent_dma_mask.
+ *
+ * In case if DMA configuration can't be acquired from DT the default one is
+ * applied:
+ * dma_pfn_offset = 0,
+ * dma_mask = DMA_BIT_MASK(32)
This is really set to coherent_dma_mask.
+ * coherent_dma_mask = DMA_BIT_MASK(32).
+
+ * In case if platform code need to use own DMA configuration - it can use
+ * Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event to fix up
+ * DMA configuration.
+ */
+static void dt_dma_configure(struct device *dev)
s/dt_/of_/
+{
+ dma_addr_t dma_addr;
+ phys_addr_t paddr, size;
+ dma_addr_t dma_mask;
+ int ret;
+
+ /*
+* if dma-ranges property doesn't exist - use 32 bits DMA mask
+* by default and don't set skip archdata.dma_pfn_offset
+*/
+ ret = of_dma_get_range(dev-of_node, dma_addr, paddr, size);
+ if (ret == -ENODEV) {
+ dev-coherent_dma_mask = DMA_BIT_MASK(32);
+ if (!dev-dma_mask)
+ dev-dma_mask = dev-coherent_dma_mask;
+ return;
+ }
+
+ /* if failed - disable DMA for device */
+ if (ret 0) {
+ dev_err(dev, failed to configure DMA\n);
+ return;
+ }
+
+ /* DMA ranges found. Calculate and set dma_pfn_offset */
+ dev-dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
+
+ /* Configure DMA mask */
+ dev-dma_mask = dev-coherent_dma_mask;
+ if (!dev-dma_mask)
This condition is impossible.
+ return;
+
+ dma_mask = dma_addr + size - 1;
+
+ ret = dma_set_mask(dev, dma_mask);
+ if (ret 0) {
+ dev_err(dev, failed to set DMA mask %pad\n, dma_mask);
+ dev-dma_mask = NULL;
+ return;
+ }
+
+ dev_dbg(dev, dma_pfn_offset(%#08lx) dma_mask(%pad)\n,
+ dev-dma_pfn_offset, dev-dma_mask);
+
+ ret = dma_set_coherent_mask(dev, dma_mask);
I think these 2 calls belong in the drivers, not here.
+ if (ret 0) {
+ dev_err(dev, failed to set coherent DMA mask %pad\n,
+ dma_mask);
+ }
+}
+
+/**
* 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 +285,7 @@ static struct platform_device
*of_platform_device_create_pdata(
#if defined(CONFIG_MICROBLAZE)
dev-archdata.dma_mask = 0xUL;
#endif
- dev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
- if (!dev-dev.dma_mask)
- dev-dev.dma_mask = dev-dev.coherent_dma_mask;
+ dt_dma_configure(dev-dev);
dev-dev.bus = platform_bus_type;
dev-dev.platform_data = platform_data;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH 4/7] of: configure the platform device dma_mask and dma_pfn_offset
On Friday 07 March 2014 11:49 AM, Rob Herring wrote:
On Thu, Mar 6, 2014 at 3:19 AM, Santosh Shilimkar
santosh.shilim...@ti.com wrote:
From: Grygorii Strashko grygorii.stras...@ti.com
Retrieve DMA configuration from DT and setup platform device's DMA
parameters.
The DMA configuration in DT has to be specified using dma-ranges
property if supported. The DMA configuration applied
by dt_dma_configure() as following:
- call of_get_dma_range() and get supported DMA range info
(dma_addr, cpu_addr, dma_size);
- if not found then fill dma_mask as DMA_BIT_MASK(32)
(this is default behaviour);
- if failed then clean up dma_mask (DMA not supported)
- if ok then update devices DMA configuration:
set dma_mask to (dma_addr + dma_size - 1)
set dma_pfn_offset to PFN_DOWN(cpu_addr - dma_addr)
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: Russell King li...@arm.linux.org.uk
Cc: Arnd Bergmann a...@arndb.de
Cc: Olof Johansson o...@lixom.net
Cc: Grant Likely grant.lik...@linaro.org
Cc: Rob Herring robh...@kernel.org
Cc: Catalin Marinas catalin.mari...@arm.com
Cc: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
drivers/of/platform.c | 75
+++--
1 file changed, 72 insertions(+), 3 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index bd080db..53bb12f 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -187,6 +187,77 @@ struct platform_device *of_device_alloc(struct
device_node *np,
EXPORT_SYMBOL(of_device_alloc);
/**
+ * dt_dma_configure - apply default DMA configuration from dt
[..]
+
+ ret = dma_set_mask(dev, dma_mask);
+ if (ret 0) {
+ dev_err(dev, failed to set DMA mask %pad\n, dma_mask);
+ dev-dma_mask = NULL;
+ return;
+ }
+
+ dev_dbg(dev, dma_pfn_offset(%#08lx) dma_mask(%pad)\n,
+ dev-dma_pfn_offset, dev-dma_mask);
+
+ ret = dma_set_coherent_mask(dev, dma_mask);
I think these 2 calls belong in the drivers, not here.
I also had same initial thought but Arnd mentioned that its a
shared responsibility between ARCH and drivers. Driver which
could be common between arches not always have the correct
mask information and it can change based on which arch it
is running.
With some discussion back and forth, we thought updating
the dma_mask while the device getting created, would be
better place since we can find the arch capability at
this centralise code and update it.
Ofcourse its bit debatable as the question you asked is
bit obvious as well. I let Arnd give his view here.
Regards,
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
