Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965204AbbLSShH (ORCPT ); Sat, 19 Dec 2015 13:37:07 -0500 Received: from mail-yk0-f170.google.com ([209.85.160.170]:36604 "EHLO mail-yk0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754042AbbLSShC convert rfc822-to-8bit (ORCPT ); Sat, 19 Dec 2015 13:37:02 -0500 MIME-Version: 1.0 In-Reply-To: <1450371684-23415-8-git-send-email-okaya@codeaurora.org> References: <1450371684-23415-1-git-send-email-okaya@codeaurora.org> <1450371684-23415-8-git-send-email-okaya@codeaurora.org> Date: Sat, 19 Dec 2015 20:37:00 +0200 Message-ID: Subject: Re: [PATCH V10 7/7] dma: qcom_hidma: add support for object hierarchy From: Andy Shevchenko To: Sinan Kaya Cc: dmaengine , Mark Rutland , Timur Tabi , devicetree , cov@codeaurora.org, Vinod Koul , jcm@redhat.com, Andy Gross , Arnd Bergmann , linux-arm-msm@vger.kernel.org, linux-arm Mailing List , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5514 Lines: 171 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@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 > +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 in probe of parent node driver? > +#endif > + platform_driver_register(&hidma_mgmt_driver); > + > + return 0; > +} > +module_init(hidma_mgmt_init); -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/