| Message ID | 1450371684-23415-8-git-send-email-okaya@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hi Sinan,
[auto build test ERROR on v4.4-rc5]
[also build test ERROR on next-20151218]
url: https://github.com/0day-ci/linux/commits/Sinan-Kaya/dma-add-Qualcomm-Technologies-HIDMA-driver/20151218-010637
config: sparc-allyesconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `hidma_mgmt_of_populate_channels':
>> hidma_mgmt.c:(.init.text+0x5cc8): undefined reference to `of_irq_parse_one'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Sinan,
[auto build test ERROR on v4.4-rc5]
[also build test ERROR on next-20151218]
url: https://github.com/0day-ci/linux/commits/Sinan-Kaya/dma-add-Qualcomm-Technologies-HIDMA-driver/20151218-010637
config: sparc64-allmodconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All errors (new ones prefixed by >>):
>> ERROR: "of_irq_parse_one" [drivers/dma/qcom/hdma_mgmt.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, Dec 17, 2015 at 7:01 PM, Sinan Kaya <okaya@codeaurora.org> wrote: > In order to create a relationship model between the channels and the > management object, we are adding support for object hierarchy to the > drivers. This patch simplifies the userspace application development. > We will not have to traverse different firmware paths based on device > tree or ACPI baed kernels. > > No matter what flavor of kernel is used, objects will be represented as > platform devices. > > The new layout is as follows: > > hidmam_10: hidma-mgmt@0x5A000000 { > compatible = "qcom,hidma-mgmt-1.0"; > ... > > hidma_10: hidma@0x5a010000 { > compatible = "qcom,hidma-1.0"; > ... > } > } > > The hidma_mgmt_init detects each instance of the hidma-mgmt-1.0 objects > in device tree and calls into the channel driver to create platform devices > for each child of the management object. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > +What: /sys/devices/platform/hidma-*/chid > + /sys/devices/platform/QCOM8061:*/chid > +Date: Dec 2015 > +KernelVersion: 4.4 > +Contact: "Sinan Kaya <okaya@cudeaurora.org>" > +Description: > + Contains the ID of the channel within the HIDMA instance. > + It is used to associate a given HIDMA channel with the > + priority and weight calls in the management interface. > -module_platform_driver(hidma_mgmt_driver); > +#ifdef CONFIG_OF > +static int object_counter; > + > +static int __init hidma_mgmt_of_populate_channels(struct device_node *np) > +{ > + struct platform_device *pdev_parent = of_find_device_by_node(np); > + struct platform_device_info pdevinfo; > + struct of_phandle_args out_irq; > + struct device_node *child; > + struct resource *res; > + const __be32 *cell; Always BE ? > + int ret = 0, size, i, num; > + u64 addr, addr_size; resource_size_t > + > + for_each_available_child_of_node(np, child) { > + int iter = 0; > + > + cell = of_get_property(child, "reg", &size); > + if (!cell) { > + ret = -EINVAL; > + goto out; > + } > + > + size /= (sizeof(*cell)); Redundant parens. I think it might be good to use common sense for operator precedence, here is a radical example of ignoring it. And this is not the first case. > + num = size / > + (of_n_addr_cells(child) + of_n_size_cells(child)) + 1; > + > + /* allocate a resource array */ > + res = kcalloc(num, sizeof(*res), GFP_KERNEL); > + if (!res) { > + ret = -ENOMEM; > + goto out; > + } > + > + /* read each reg value */ > + i = 0; > + while (i < size) { > + addr = of_read_number(&cell[i], > + of_n_addr_cells(child)); > + i += of_n_addr_cells(child); > + > + addr_size = of_read_number(&cell[i], > + of_n_size_cells(child)); > + i += of_n_size_cells(child); > + > + res[iter].start = addr; > + res[iter].end = res[iter].start + addr_size - 1; > + res[iter].flags = IORESOURCE_MEM; res->start = … … res++; > + iter++; > + } > + > + ret = of_irq_parse_one(child, 0, &out_irq); > + if (ret) > + goto out; > + > + res[iter].start = irq_create_of_mapping(&out_irq); > + res[iter].name = "hidma event irq"; > + res[iter].flags = IORESOURCE_IRQ; Ditto. > + > + pdevinfo.fwnode = &child->fwnode; > + pdevinfo.parent = pdev_parent ? &pdev_parent->dev : NULL; > + pdevinfo.name = child->name; > + pdevinfo.id = object_counter++; > + pdevinfo.res = res; > + pdevinfo.num_res = num; > + pdevinfo.data = NULL; > + pdevinfo.size_data = 0; > + pdevinfo.dma_mask = DMA_BIT_MASK(64); > + platform_device_register_full(&pdevinfo); > + > + kfree(res); > + res = NULL; > + } > +out: > + kfree(res); > + > + return ret; > +} > +#endif > + > +static int __init hidma_mgmt_init(void) > +{ > +#ifdef CONFIG_OF > + struct device_node *child; > + > + for (child = of_find_matching_node(NULL, hidma_mgmt_match); child; > + child = of_find_matching_node(child, hidma_mgmt_match)) { > + /* device tree based firmware here */ > + hidma_mgmt_of_populate_channels(child); > + of_node_put(child); > + } And why this is can't be done in probe of parent node driver? > +#endif > + platform_driver_register(&hidma_mgmt_driver); > + > + return 0; > +} > +module_init(hidma_mgmt_init);
> Hi Sinan, > > [auto build test ERROR on v4.4-rc5] > [also build test ERROR on next-20151218] > > url: > https://github.com/0day-ci/linux/commits/Sinan-Kaya/dma-add-Qualcomm-Technologies-HIDMA-driver/20151218-010637 > config: sparc64-allmodconfig (attached as .config) > reproduce: > wget > https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=sparc64 > > All errors (new ones prefixed by >>): > >>> ERROR: "of_irq_parse_one" [drivers/dma/qcom/hdma_mgmt.ko] undefined! > > --- > 0-DAY kernel test infrastructure Open Source Technology > Center > https://lists.01.org/pipermail/kbuild-all Intel > Corporation > I am confused about this. The function call is made only if OF is defined. The function that is being called is defined with extern symbol and resides in drivers/of/irq.c. how does the sparc arch define OF yet exclude drivers/of/irq.c -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 19, 2015 at 12:37 PM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: >> +static int __init hidma_mgmt_of_populate_channels(struct device_node *np) >> +{ >> + struct platform_device *pdev_parent = of_find_device_by_node(np); >> + struct platform_device_info pdevinfo; >> + struct of_phandle_args out_irq; >> + struct device_node *child; >> + struct resource *res; >> + const __be32 *cell; > > Always BE ? Unless things have changed, device tree properties are always in big-endian order, even on little-endian systems.
On 12/19/2015 1:37 PM, Andy Shevchenko wrote: > On Thu, Dec 17, 2015 at 7:01 PM, Sinan Kaya <okaya@codeaurora.org> wrote: >> In order to create a relationship model between the channels and the >> management object, we are adding support for object hierarchy to the >> drivers. This patch simplifies the userspace application development. >> We will not have to traverse different firmware paths based on device >> tree or ACPI baed kernels. >> >> No matter what flavor of kernel is used, objects will be represented as >> platform devices. >> >> The new layout is as follows: >> >> hidmam_10: hidma-mgmt@0x5A000000 { >> compatible = "qcom,hidma-mgmt-1.0"; >> ... >> >> hidma_10: hidma@0x5a010000 { >> compatible = "qcom,hidma-1.0"; >> ... >> } >> } >> >> The hidma_mgmt_init detects each instance of the hidma-mgmt-1.0 objects >> in device tree and calls into the channel driver to create platform devices >> for each child of the management object. >> >> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > >> +What: /sys/devices/platform/hidma-*/chid >> + /sys/devices/platform/QCOM8061:*/chid >> +Date: Dec 2015 >> +KernelVersion: 4.4 >> +Contact: "Sinan Kaya <okaya@cudeaurora.org>" >> +Description: >> + Contains the ID of the channel within the HIDMA instance. >> + It is used to associate a given HIDMA channel with the >> + priority and weight calls in the management interface. > >> -module_platform_driver(hidma_mgmt_driver); >> +#ifdef CONFIG_OF >> +static int object_counter; >> + >> +static int __init hidma_mgmt_of_populate_channels(struct device_node *np) >> +{ >> + struct platform_device *pdev_parent = of_find_device_by_node(np); >> + struct platform_device_info pdevinfo; >> + struct of_phandle_args out_irq; >> + struct device_node *child; >> + struct resource *res; >> + const __be32 *cell; > > Always BE ? Yes, as Timur indicated device tree is big endian all the time. > >> + int ret = 0, size, i, num; >> + u64 addr, addr_size; > > resource_size_t These values are read from the device tree blob using of_read_number function. of_read_number returns a u64. > >> + >> + for_each_available_child_of_node(np, child) { >> + int iter = 0; >> + >> + cell = of_get_property(child, "reg", &size); >> + if (!cell) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> + size /= (sizeof(*cell)); > > Redundant parens. I think it might be good to use common sense for > operator precedence, here is a radical example of ignoring it. And > this is not the first case. Removed. Note that I copied most of this function from another driver. > >> + num = size / >> + (of_n_addr_cells(child) + of_n_size_cells(child)) + 1; >> + >> + /* allocate a resource array */ >> + res = kcalloc(num, sizeof(*res), GFP_KERNEL); >> + if (!res) { >> + ret = -ENOMEM; >> + goto out; >> + } >> + >> + /* read each reg value */ >> + i = 0; >> + while (i < size) { >> + addr = of_read_number(&cell[i], >> + of_n_addr_cells(child)); >> + i += of_n_addr_cells(child); >> + >> + addr_size = of_read_number(&cell[i], >> + of_n_size_cells(child)); >> + i += of_n_size_cells(child); >> + >> + res[iter].start = addr; >> + res[iter].end = res[iter].start + addr_size - 1; >> + res[iter].flags = IORESOURCE_MEM; > > res->start = … > … > res++; ok > >> + iter++; >> + } >> + >> + ret = of_irq_parse_one(child, 0, &out_irq); >> + if (ret) >> + goto out; >> + >> + res[iter].start = irq_create_of_mapping(&out_irq); >> + res[iter].name = "hidma event irq"; >> + res[iter].flags = IORESOURCE_IRQ; > > Ditto. ok > >> + >> + pdevinfo.fwnode = &child->fwnode; >> + pdevinfo.parent = pdev_parent ? &pdev_parent->dev : NULL; >> + pdevinfo.name = child->name; >> + pdevinfo.id = object_counter++; >> + pdevinfo.res = res; >> + pdevinfo.num_res = num; >> + pdevinfo.data = NULL; >> + pdevinfo.size_data = 0; >> + pdevinfo.dma_mask = DMA_BIT_MASK(64); >> + platform_device_register_full(&pdevinfo); >> + >> + kfree(res); >> + res = NULL; >> + } >> +out: > >> + kfree(res); > > >> + >> + return ret; >> +} >> +#endif >> + >> +static int __init hidma_mgmt_init(void) >> +{ >> +#ifdef CONFIG_OF >> + struct device_node *child; >> + >> + for (child = of_find_matching_node(NULL, hidma_mgmt_match); child; >> + child = of_find_matching_node(child, hidma_mgmt_match)) { >> + /* device tree based firmware here */ >> + hidma_mgmt_of_populate_channels(child); >> + of_node_put(child); >> + } > > And why this is can't be done in probe of parent node driver? The populate function creates platform objects using platform_device_register_full function above. The hidma device driver later probes the object during object enumeration phase. I tried moving platform_device_register_full into the probe context. The objects got generated but hidma device driver didn't probe the object. I suppose we are required to create the objects before the probing starts. I copied this pattern from another driver. > >> +#endif >> + platform_driver_register(&hidma_mgmt_driver); >> + >> + return 0; >> +} >> +module_init(hidma_mgmt_init); >
diff --git a/Documentation/ABI/testing/sysfs-platform-hidma b/Documentation/ABI/testing/sysfs-platform-hidma new file mode 100644 index 0000000..d364415 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-platform-hidma @@ -0,0 +1,9 @@ +What: /sys/devices/platform/hidma-*/chid + /sys/devices/platform/QCOM8061:*/chid +Date: Dec 2015 +KernelVersion: 4.4 +Contact: "Sinan Kaya <okaya@cudeaurora.org>" +Description: + Contains the ID of the channel within the HIDMA instance. + It is used to associate a given HIDMA channel with the + priority and weight calls in the management interface. diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c index 833db9e..ac20bdb 100644 --- a/drivers/dma/qcom/hidma.c +++ b/drivers/dma/qcom/hidma.c @@ -549,6 +549,43 @@ static irqreturn_t hidma_chirq_handler(int chirq, void *arg) return hidma_ll_inthandler(chirq, lldev); } +static ssize_t hidma_show_values(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct platform_device *pdev = to_platform_device(dev); + struct hidma_dev *mdev = platform_get_drvdata(pdev); + + buf[0] = 0; + + if (strcmp(attr->attr.name, "chid") == 0) + sprintf(buf, "%d\n", mdev->chidx); + + return strlen(buf); +} + +static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name, + int mode) +{ + struct device_attribute *attrs; + char *name_copy; + + attrs = devm_kmalloc(dev->ddev.dev, sizeof(struct device_attribute), + GFP_KERNEL); + if (!attrs) + return -ENOMEM; + + name_copy = devm_kstrdup(dev->ddev.dev, name, GFP_KERNEL); + if (!name_copy) + return -ENOMEM; + + attrs->attr.name = name_copy; + attrs->attr.mode = mode; + attrs->show = hidma_show_values; + sysfs_attr_init(&attrs->attr); + + return device_create_file(dev->ddev.dev, attrs); +} + static int hidma_probe(struct platform_device *pdev) { struct hidma_dev *dmadev; @@ -682,6 +719,7 @@ static int hidma_probe(struct platform_device *pdev) dmadev->irq = chirq; tasklet_init(&dmadev->task, hidma_issue_task, (unsigned long)dmadev); hidma_debug_init(dmadev); + hidma_create_sysfs_entry(dmadev, "chid", S_IRUGO); dev_info(&pdev->dev, "HI-DMA engine driver registration complete\n"); platform_set_drvdata(pdev, dmadev); pm_runtime_mark_last_busy(dmadev->ddev.dev); @@ -730,7 +768,6 @@ static const struct of_device_id hidma_match[] = { {.compatible = "qcom,hidma-1.0",}, {}, }; - MODULE_DEVICE_TABLE(of, hidma_match); static struct platform_driver hidma_driver = { diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c index ef491b8..d387338 100644 --- a/drivers/dma/qcom/hidma_mgmt.c +++ b/drivers/dma/qcom/hidma_mgmt.c @@ -17,13 +17,14 @@ #include <linux/acpi.h> #include <linux/of.h> #include <linux/property.h> -#include <linux/interrupt.h> -#include <linux/platform_device.h> +#include <linux/of_irq.h> +#include <linux/of_platform.h> #include <linux/module.h> #include <linux/uaccess.h> #include <linux/slab.h> #include <linux/pm_runtime.h> #include <linux/bitops.h> +#include <linux/dma-mapping.h> #include "hidma_mgmt.h" @@ -298,5 +299,101 @@ static struct platform_driver hidma_mgmt_driver = { }, }; -module_platform_driver(hidma_mgmt_driver); +#ifdef CONFIG_OF +static int object_counter; + +static int __init hidma_mgmt_of_populate_channels(struct device_node *np) +{ + struct platform_device *pdev_parent = of_find_device_by_node(np); + struct platform_device_info pdevinfo; + struct of_phandle_args out_irq; + struct device_node *child; + struct resource *res; + const __be32 *cell; + int ret = 0, size, i, num; + u64 addr, addr_size; + + for_each_available_child_of_node(np, child) { + int iter = 0; + + cell = of_get_property(child, "reg", &size); + if (!cell) { + ret = -EINVAL; + goto out; + } + + size /= (sizeof(*cell)); + num = size / + (of_n_addr_cells(child) + of_n_size_cells(child)) + 1; + + /* allocate a resource array */ + res = kcalloc(num, sizeof(*res), GFP_KERNEL); + if (!res) { + ret = -ENOMEM; + goto out; + } + + /* read each reg value */ + i = 0; + while (i < size) { + addr = of_read_number(&cell[i], + of_n_addr_cells(child)); + i += of_n_addr_cells(child); + + addr_size = of_read_number(&cell[i], + of_n_size_cells(child)); + i += of_n_size_cells(child); + + res[iter].start = addr; + res[iter].end = res[iter].start + addr_size - 1; + res[iter].flags = IORESOURCE_MEM; + iter++; + } + + ret = of_irq_parse_one(child, 0, &out_irq); + if (ret) + goto out; + + res[iter].start = irq_create_of_mapping(&out_irq); + res[iter].name = "hidma event irq"; + res[iter].flags = IORESOURCE_IRQ; + + pdevinfo.fwnode = &child->fwnode; + pdevinfo.parent = pdev_parent ? &pdev_parent->dev : NULL; + pdevinfo.name = child->name; + pdevinfo.id = object_counter++; + pdevinfo.res = res; + pdevinfo.num_res = num; + pdevinfo.data = NULL; + pdevinfo.size_data = 0; + pdevinfo.dma_mask = DMA_BIT_MASK(64); + platform_device_register_full(&pdevinfo); + + kfree(res); + res = NULL; + } +out: + kfree(res); + + return ret; +} +#endif + +static int __init hidma_mgmt_init(void) +{ +#ifdef CONFIG_OF + struct device_node *child; + + for (child = of_find_matching_node(NULL, hidma_mgmt_match); child; + child = of_find_matching_node(child, hidma_mgmt_match)) { + /* device tree based firmware here */ + hidma_mgmt_of_populate_channels(child); + of_node_put(child); + } +#endif + platform_driver_register(&hidma_mgmt_driver); + + return 0; +} +module_init(hidma_mgmt_init); MODULE_LICENSE("GPL v2");
In order to create a relationship model between the channels and the management object, we are adding support for object hierarchy to the drivers. This patch simplifies the userspace application development. We will not have to traverse different firmware paths based on device tree or ACPI baed kernels. No matter what flavor of kernel is used, objects will be represented as platform devices. The new layout is as follows: hidmam_10: hidma-mgmt@0x5A000000 { compatible = "qcom,hidma-mgmt-1.0"; ... hidma_10: hidma@0x5a010000 { compatible = "qcom,hidma-1.0"; ... } } The hidma_mgmt_init detects each instance of the hidma-mgmt-1.0 objects in device tree and calls into the channel driver to create platform devices for each child of the management object. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- Documentation/ABI/testing/sysfs-platform-hidma | 9 +++ drivers/dma/qcom/hidma.c | 39 +++++++++- drivers/dma/qcom/hidma_mgmt.c | 103 ++++++++++++++++++++++++- 3 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-platform-hidma