of: add dma-mask binding
PATCH 1/1 of: add dma-mask binding
This will allow each device to specify its dma-mask
The microblaze architecture hook is keep temporary if no dma-mask is specified int the DT
--- drivers/of/platform.c | 26 +++++++++++++++++++++++++- 1 files changed, 25 insertions(+), 1 deletions(-)
drivers/of/platform.c +25 -1
@@ -121,6 +121,26 @@ void of_device_make_bus_id(struct device *dev)dev_set_name (dev,"%s.%d" , node->name, magic -1 ); } +static u64 *of_get_dma_mask (struct device_node *np) +{ +const __be32 *prop; +int len; +u64 *dma_mask; + + prop =of_get_property (np,"dma-mask" , &len); + +if (!prop) +return NULL ; + + dma_mask =kzalloc (sizeof (u64),GFP_KERNEL ); +if (!dma_mask) +return NULL ; + + *dma_mask =of_read_number (prop, len /4 ); + +return dma_mask; +} +/** * of_device_alloc - Allocate and initialize an of_device * @np: device node to assign to device
@@ -161,10 +181,14 @@ struct platform_device *of_device_alloc(struct device_node *np,WARN_ON (of_irq_to_resource_table (np, res, num_irq) != num_irq); } + dev->dev.dma_mask =of_get_dma_mask (np); dev->dev.of_node =of_node_get (np); +#if defined(CONFIG_MICROBLAZE ) - dev->dev.dma_mask = &dev->archdata.dma_mask; +if (!dev->dev.dma_mask) + dev->dev.dma_mask = &dev->archdata.dma_mask;#endif + dev->dev.parent = parent;if (bus_id)
Signature · 1 line
1.7.7
PATCH 1/1 of: add coherent dma-mask binding
This will allow each device to specify its coherent-dma-mask The default behavior the set it to DMA_BIT_MASK(32) if none specified is keeped.
--- drivers/of/platform.c | 15 ++++++++++++++- 1 files changed, 14 insertions(+), 1 deletions(-)
drivers/of/platform.c +14 -1
@@ -141,6 +141,19 @@ static u64* of_get_dma_mask(struct device_node *np)return dma_mask; } +static u64 of_get_coherent_dma_mask (struct device_node *np) +{ +const __be32 *prop; +int len; + + prop =of_get_property (np,"coherent-dma-mask" , &len); + +if (!prop) +return DMA_BIT_MASK (32 ); + +return of_read_number (prop, len /4 ); +} +/** * of_device_alloc - Allocate and initialize an of_device * @np: device node to assign to device
@@ -228,7 +241,7 @@ struct platform_device *of_platform_device_create_pdata(#if defined(CONFIG_MICROBLAZE ) dev->archdata.dma_mask =0xffffffffUL ;#endif - dev->dev.coherent_dma_mask =DMA_BIT_MASK (32 ); + dev->dev.coherent_dma_mask =of_get_coherent_dma_mask (np); dev->dev.bus = &platform_bus_type; dev->dev.platform_data = platform_data;
Signature · 1 line
1.7.7
PATCH 1/1 of: add dma-mask binding
In reply to Jean-Christophe PLAGNIOL-VILLARDQuoted 14 lines · Jean-Christophe PLAGNIOL-VILLARD
This will allow each device to specify its dma-mask
The microblaze architecture hook is keep temporary if no dma-mask is specified int the DTSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>--- drivers/of/platform.c | 26 +++++++++++++++++++++++++- 1 files changed, 25 insertions(+), 1 deletions(-)drivers/of/platform.c +0
-0
drivers/of/platform.c void of_device_make_bus_id(struct device *dev) · line 130
Quoted 10 lines
drivers/of/platform.c +7
-0@@ -121,6 +121,26 @@ void of_device_make_bus_id(struct device *dev)dev_set_name (dev,"%s.%d" , node->name, magic -1 ); } +static u64 *of_get_dma_mask (struct device_node *np) +{ +const __be32 *prop; +int len; +u64 *dma_mask; + + prop =of_get_property (np,"dma-mask" , &len);
This would need some documentation. There is already "dma-ranges" defined for OF which nay do what's needed.
drivers/of/platform.c void of_device_make_bus_id(struct device *dev) · line 135
Quoted 5 lines
drivers/of/platform.c +5
-0+ +if (!prop) +return NULL ; + + dma_mask =kzalloc (sizeof (u64),GFP_KERNEL );
This seems kind of wasteful for 1 u64.
Quoted 12 lines
drivers/of/platform.c +8
-0+if (!dma_mask) +return NULL ; + + *dma_mask =of_read_number (prop, len /4 ); + +return dma_mask; +} +/** * of_device_alloc - Allocate and initialize an of_device * @np: device node to assign to device
drivers/of/platform.c struct platform_device *of_device_alloc(struct device_node *np, · line 166
Quoted 8 lines
drivers/of/platform.c +2
-1@@ -161,10 +181,14 @@ struct platform_device *of_device_alloc(struct device_node *np,WARN_ON (of_irq_to_resource_table (np, res, num_irq) != num_irq); } + dev->dev.dma_mask =of_get_dma_mask (np); dev->dev.of_node =of_node_get (np); +#if defined(CONFIG_MICROBLAZE ) - dev->dev.dma_mask = &dev->archdata.dma_mask;
And doing it this way for didn't get a warm reception either:
http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-April/005285.html
Do you really need this to be something other than ~0UL and if so does it need to be per device or system wide?
You can solve this with bus notifiers in your at91 code. Here's an example which I did (but no longer need):
static u64 highbank_dma_mask = DMA_BIT_MASK(32);
static int highbank_platform_notifier(struct notifier_block *nb,
unsigned long event, void *__dev)
{
struct device *dev = __dev;
if (event != BUS_NOTIFY_ADD_DEVICE)
return NOTIFY_DONE;
if (of_device_is_compatible(dev->of_node, "calxeda,hb-nfc"))
dev->dma_mask = &highbank_dma_mask;
else
return NOTIFY_DONE;
return NOTIFY_OK;
}static struct notifier_block highbank_platform_nb = {
.notifier_call = highbank_platform_notifier,
};
int highbank_devices_init(void)
{
return bus_register_notifier(&platform_bus_type, &highbank_platform_nb);
}Rob
Quoted 7 lines
drivers/of/platform.c +3
-0+if (!dev->dev.dma_mask) + dev->dev.dma_mask = &dev->archdata.dma_mask;#endif + dev->dev.parent = parent;if (bus_id)
PATCH 1/1 of: add dma-mask binding
Quoted 18 lines · Rob Herring
Quoted 17 lines · Jean-Christophe PLAGNIOL-VILLARD
This will allow each device to specify its dma-mask
The microblaze architecture hook is keep temporary if no dma-mask is specified int the DTSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>--- drivers/of/platform.c | 26 +++++++++++++++++++++++++- 1 files changed, 25 insertions(+), 1 deletions(-)drivers/of/platform.c +0
-0@@ -121,6 +121,26 @@ void of_device_make_bus_id(struct device *dev)dev_set_name (dev,"%s.%d" , node->name, magic -1 ); }
Quoted 10 lines
drivers/of/platform.c void of_device_make_bus_id(struct device *dev) · line 130
Quoted 7 lines
drivers/of/platform.c +7
-0+static u64 *of_get_dma_mask (struct device_node *np) +{ +const __be32 *prop; +int len; +u64 *dma_mask; + + prop =of_get_property (np,"dma-mask" , &len);This would need some documentation. There is already "dma-ranges" defined for OF which nay do what's needed.
what is dma-ranges I see no doc about it but I don't think it could be used for the dma mask
Quoted 8 lines
drivers/of/platform.c void of_device_make_bus_id(struct device *dev) · line 135
Quoted 6 lines
drivers/of/platform.c +5
-0+ +if (!prop) +return NULL ; + + dma_mask =kzalloc (sizeof (u64),GFP_KERNEL );This seems kind of wasteful for 1 u64.
no choice dma_mask must be a pointer
Quoted 26 lines
Quoted 13 lines
drivers/of/platform.c +8
-0+if (!dma_mask) +return NULL ; + + *dma_mask =of_read_number (prop, len /4 ); + +return dma_mask; +} +/** * of_device_alloc - Allocate and initialize an of_device * @np: device node to assign to device drivers/of/platform.c struct platform_device *of_device_alloc(struct device_node *np, · line 166
Quoted 8 lines
drivers/of/platform.c +2
-1@@ -161,10 +181,14 @@ struct platform_device *of_device_alloc(struct device_node *np,WARN_ON (of_irq_to_resource_table (np, res, num_irq) != num_irq); } + dev->dev.dma_mask =of_get_dma_mask (np); dev->dev.of_node =of_node_get (np); +#if defined(CONFIG_MICROBLAZE ) - dev->dev.dma_mask = &dev->archdata.dma_mask;And doing it this way for didn't get a warm reception either:
http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-April/005285.html
Quoted 7 lines
Do you really need this to be something other than ~0UL and if so does it need to be per device or system wide?
You can solve this with bus notifiers in your at91 code. Here's an example which I did (but no longer need):
static u64
highbank_dma_mask=DMA_BIT_MASK(32);
IIRC on at91 all of them that need it have a DMA_BIT_MASK(32)
Quoted 11 lines
static int highbank_platform_notifier(struct
notifier_block*nb,
unsigned long event, void*__dev){ struct device *dev = __dev; if (event != BUS_NOTIFY_ADD_DEVICE) return NOTIFY_DONE; if (of_device_is_compatible(dev->of_node, "calxeda,hb-nfc")) dev->dma_mask = &highbank_dma_mask;
execpt as example today I need it on 2 node (OHCI & EHCI)
and if more device need I don't want to end up with more fixup
I really prefer to set it via DT
Best Regards,
J.
PATCH 1/1 of: add dma-mask binding
Quoted 29 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 18 lines · Rob Herring
Quoted 17 lines · Jean-Christophe PLAGNIOL-VILLARD
This will allow each device to specify its dma-mask
The microblaze architecture hook is keep temporary if no dma-mask is specified int the DTSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>--- drivers/of/platform.c | 26 +++++++++++++++++++++++++- 1 files changed, 25 insertions(+), 1 deletions(-)drivers/of/platform.c +0
-0@@ -121,6 +121,26 @@ void of_device_make_bus_id(struct device *dev)dev_set_name (dev,"%s.%d" , node->name, magic -1 ); }Quoted 10 lines
drivers/of/platform.c void of_device_make_bus_id(struct device *dev) · line 130
Quoted 7 lines
drivers/of/platform.c +7
-0+static u64 *of_get_dma_mask (struct device_node *np) +{ +const __be32 *prop; +int len; +u64 *dma_mask; + + prop =of_get_property (np,"dma-mask" , &len);This would need some documentation. There is already "dma-ranges" defined for OF which nay do what's needed.
Quoted 2 lines
what is dma-ranges I see no doc about it but I don't think it could be used for the dma mask
See the ePAPR spec. It's a PowerPC doc, but parts still apply.
https://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.1.pdf
There may also be something in OpenFirmware specs.
Quoted 51 lines
Quoted 8 lines
drivers/of/platform.c void of_device_make_bus_id(struct device *dev) · line 135
Quoted 6 lines
drivers/of/platform.c +5
-0+ +if (!prop) +return NULL ; + + dma_mask =kzalloc (sizeof (u64),GFP_KERNEL );This seems kind of wasteful for 1 u64.
no choice
dma_maskmust be a pointerQuoted 26 lines
Quoted 13 lines
drivers/of/platform.c +8
-0+if (!dma_mask) +return NULL ; + + *dma_mask =of_read_number (prop, len /4 ); + +return dma_mask; +} +/** * of_device_alloc - Allocate and initialize an of_device * @np: device node to assign to device drivers/of/platform.c struct platform_device *of_device_alloc(struct device_node *np, · line 166
Quoted 8 lines
drivers/of/platform.c +2
-1@@ -161,10 +181,14 @@ struct platform_device *of_device_alloc(struct device_node *np,WARN_ON (of_irq_to_resource_table (np, res, num_irq) != num_irq); } + dev->dev.dma_mask =of_get_dma_mask (np); dev->dev.of_node =of_node_get (np); +#if defined(CONFIG_MICROBLAZE ) - dev->dev.dma_mask = &dev->archdata.dma_mask;And doing it this way for didn't get a warm reception either:
http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-April/005285.html
Quoted 7 lines
Do you really need this to be something other than ~0UL and if so does it need to be per device or system wide?
You can solve this with bus notifiers in your at91 code. Here's an example which I did (but no longer need):
static u64
highbank_dma_mask=DMA_BIT_MASK(32);IIRC on at91 all of them that need it have a
DMA_BIT_MASK(32)Quoted 6 lines
static int highbank_platform_notifier(struct
notifier_block*nb,
unsigned long event, void*__dev){ struct device *dev = __dev;
Quoted 6 lines
Quoted 5 lines
if (event != BUS_NOTIFY_ADD_DEVICE) return NOTIFY_DONE; if (of_device_is_compatible(dev->of_node, "calxeda,hb-nfc")) dev->dma_mask = &highbank_dma_mask;execpt as example today I need it on 2 node (OHCI & EHCI)
I don't think there is any reason you can't point to a single value if a system wide setting fits. If not, using a per device value here is easily doable.
Quoted 4 lines
and if more device need I don't want to end up with more fixup
I really prefer to set it via DT
You don't have any h/w restrictions here and most ARM platforms don't, so really this doesn't need to be solved with DT for the default case.
Rob
PATCH 1/1 of: add dma-mask binding
In reply to Jean-Christophe PLAGNIOL-VILLARD[cc'ing Russell and Ben because my dma-mask-foo is weak]
Quoted 29 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 18 lines · Rob Herring
Quoted 17 lines · Jean-Christophe PLAGNIOL-VILLARD
This will allow each device to specify its dma-mask
The microblaze architecture hook is keep temporary if no dma-mask is specified int the DTSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>--- drivers/of/platform.c | 26 +++++++++++++++++++++++++- 1 files changed, 25 insertions(+), 1 deletions(-)drivers/of/platform.c +0
-0@@ -121,6 +121,26 @@ void of_device_make_bus_id(struct device *dev)dev_set_name (dev,"%s.%d" , node->name, magic -1 ); }Quoted 10 lines
drivers/of/platform.c void of_device_make_bus_id(struct device *dev) · line 130
Quoted 7 lines
drivers/of/platform.c +7
-0+static u64 *of_get_dma_mask (struct device_node *np) +{ +const __be32 *prop; +int len; +u64 *dma_mask; + + prop =of_get_property (np,"dma-mask" , &len);This would need some documentation. There is already "dma-ranges" defined for OF which nay do what's needed.
Quoted 2 lines
what is dma-ranges I see no doc about it but I don't think it could be used for the dma mask
As Rob says, it's documented in ePAPR.
Quoted 9 lines
Quoted 8 lines
drivers/of/platform.c void of_device_make_bus_id(struct device *dev) · line 135
Quoted 6 lines
drivers/of/platform.c +5
-0+ +if (!prop) +return NULL ; + + dma_mask =kzalloc (sizeof (u64),GFP_KERNEL );This seems kind of wasteful for 1 u64.
no choice
dma_maskmust be a pointer
We could merely add an empty dma_mask u64 to the containing struct platform_device and then assign the pointer to dev.dma_mask if it is needed. That would eliminate the malloc. Or we could by default use dev->dev.dma_mask = &dev->dev.coherent_dma_mask; Drivers or notifier code could override whatever we setup here as the default.
dma-ranges really is the property that should be used for this, but dma-ranges can describe a different set of permutations than dma_mask. The former can have any number of valid DMA windows, but dma_mask is based on per-address-line values. (The whole dma_mask approach appears insufficient to me, but I don't understand all the background details as to why it is implemented that way).
I'd like to avoid defining a new mechanism for describing the valid dma layout if at all possible. This could be done with a new function to fetch and parse each of the dma-ranges tuples and extract the intersect of the ranges. It will never be a perfect translation, particularly because RAM doesn't start at 0 on a lot of platforms.
So, let's call the base of ram X, and the top of ram X+S. Then let's assume that dma_mask is intended to filter out high address (instead of broken DMA low addresses) and that the DMA limitations are based on attached address lines, not arbitrary start-end addresses, so if we let number of address lines be 'A', then dma_mask = 2^A-1 (ideally). Individual bits could also be blanked out, but what that really means in a practical sense is that those address lines must be
0. There isn't a way to constrain address bits that must be set to 1.
Okay, so if we assume that linux will never choose a dma address below X (because there is no RAM there; except in the iommu case which can put dma addresses where it needs to regardless), then we only need to worry about the upper range for a device. We could derive a dma_mask by calling of_translate_address on the base address of each dma-ranges tuple and finding the one that starts at or below the base of memory. Then the default dma_mask can perhaps be calculated thusly (using 'Y' for top of the chosen dma-range tuple):
dma_mask = (2 ^ fls64(Y)) - 1;
Alternately, we could simply decide to do as you suggest and define a dma-mask property because that approach is sufficient for a lot of hardware and is therefore is a reasonable binding.
Thoughts?
Quoted 60 lines
Quoted 26 lines
Quoted 13 lines
drivers/of/platform.c +8
-0+if (!dma_mask) +return NULL ; + + *dma_mask =of_read_number (prop, len /4 ); + +return dma_mask; +} +/** * of_device_alloc - Allocate and initialize an of_device * @np: device node to assign to device drivers/of/platform.c struct platform_device *of_device_alloc(struct device_node *np, · line 166
Quoted 8 lines
drivers/of/platform.c +2
-1@@ -161,10 +181,14 @@ struct platform_device *of_device_alloc(struct device_node *np,WARN_ON (of_irq_to_resource_table (np, res, num_irq) != num_irq); } + dev->dev.dma_mask =of_get_dma_mask (np); dev->dev.of_node =of_node_get (np); +#if defined(CONFIG_MICROBLAZE ) - dev->dev.dma_mask = &dev->archdata.dma_mask;And doing it this way for didn't get a warm reception either:
http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-April/005285.html
Quoted 7 lines
Do you really need this to be something other than ~0UL and if so does it need to be per device or system wide?
You can solve this with bus notifiers in your at91 code. Here's an example which I did (but no longer need):
static u64
highbank_dma_mask=DMA_BIT_MASK(32);IIRC on at91 all of them that need it have a
DMA_BIT_MASK(32)Quoted 11 lines
static int highbank_platform_notifier(struct
notifier_block*nb,
unsigned long event, void*__dev){ struct device *dev = __dev; if (event != BUS_NOTIFY_ADD_DEVICE) return NOTIFY_DONE; if (of_device_is_compatible(dev->of_node, "calxeda,hb-nfc")) dev->dma_mask = &highbank_dma_mask;execpt as example today I need it on 2 node (OHCI & EHCI)
and if more device need I don't want to end up with more fixup
I really prefer to set it via DT
Best Regards,
J._______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 1 line
email sent from notmuch.vim plugin
PATCH 1/1 of: add dma-mask binding
Quoted 6 lines · Grant Likely
Quoted 4 lines
Quoted 2 lines
This would need some documentation. There is already "dma-ranges" defined for OF which nay do what's needed.
what is dma-ranges I see no doc about it but I don't think it could be used for the dma mask
As Rob says, it's documented in ePAPR.
dma-ranges is generally defined for bridges to indicate the dma window to system memory, it's not been by actual devices so far. It provides the opposite of "ranges", ie a translation from children to parent address space.
So it's not quite the same thing.
Now the fact that we are limited to power-of-2 masks is a Linux implementation detail. We might want to make the device-tree a bit more flexible and use something like dma-limit instead. However hardware limits tend to be in term of number of bits anyways so it's not a big issue.
I don't see a need to keep a "ranges" type of semantic. If we ever do that we can do a new binding for it or add a dma-base...
Quoted 3 lines
We could merely add an empty
dma_masku64 to the containing structplatform_deviceand then assign the pointer to dev.dma_mask if it is needed. That would eliminate the malloc.
Agreed.
Quoted 6 lines
Or we could by default
usedev->dev.dma_mask=&dev->dev.coherent_dma_mask; Drivers or notifier code could override whatever we setup here as the default.dma-ranges really is the property that should be used for this, but dma-ranges can describe a different set of permutations than
dma_mask.
I don't think so. As I said, dma-ranges is about a bridge/bus exposing DMA windows & translations. It has nothing to do with the device intrinsic DMA'ing capabilities.
Cheers,
Ben.