Re: [PATCH V10 7/7] dma: qcom_hidma: add support for object hierarchy
On 12/19/2015 1:37 PM, Andy Shevchenko wrote:
> On Thu, Dec 17, 2015 at 7:01 PM, Sinan Kaya 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@0x5A00 {
>> compatible = "qcom,hidma-mgmt-1.0";
>> ...
>>
>> hidma_10: hidma@0x5a01 {
>> 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
>
>
>> +What: /sys/devices/platform/hidma-*/chid
>> + /sys/devices/platform/QCOM8061:*/chid
>> +Date: Dec 2015
>> +KernelVersion: 4.4
>> +Contact: "Sinan Kaya "
>> +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;
Re: [PATCH V10 7/7] dma: qcom_hidma: add support for object hierarchy
On Sat, Dec 19, 2015 at 12:37 PM, Andy Shevchenko
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.
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
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 V10 7/7] dma: qcom_hidma: add support for object hierarchy
> 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 infrastructureOpen 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 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 V10 7/7] dma: qcom_hidma: add support for object hierarchy
On Thu, Dec 17, 2015 at 7:01 PM, Sinan Kaya 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@0x5A00 {
> compatible = "qcom,hidma-mgmt-1.0";
> ...
>
> hidma_10: hidma@0x5a01 {
> 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
> +What: /sys/devices/platform/hidma-*/chid
> + /sys/devices/platform/QCOM8061:*/chid
> +Date: Dec 2015
> +KernelVersion: 4.4
> +Contact: "Sinan Kaya "
> +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 i
Re: [PATCH V10 7/7] dma: qcom_hidma: add support for object hierarchy
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 infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH V10 7/7] dma: qcom_hidma: add support for object hierarchy
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 infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
