From patchwork Sat Jul 20 05:43:01 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naveen Krishna Chatradhi X-Patchwork-Id: 2830782 Return-Path: X-Original-To: patchwork-linux-samsung-soc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 8049F9F3EB for ; Sat, 20 Jul 2013 05:43:44 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 995B92015E for ; Sat, 20 Jul 2013 05:43:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0083B20145 for ; Sat, 20 Jul 2013 05:43:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753672Ab3GTFn2 (ORCPT ); Sat, 20 Jul 2013 01:43:28 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:28298 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753554Ab3GTFnF (ORCPT ); Sat, 20 Jul 2013 01:43:05 -0400 Received: from epcpsbgr3.samsung.com (u143.gpu120.samsung.co.kr [203.254.230.143]) by mailout1.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MQ700K28ZVPU180@mailout1.samsung.com> for linux-samsung-soc@vger.kernel.org; Sat, 20 Jul 2013 14:43:02 +0900 (KST) Received: from epcpsbgx2.samsung.com ( [172.20.52.123]) by epcpsbgr3.samsung.com (EPCPMTA) with SMTP id 50.B7.03969.6632AE15; Sat, 20 Jul 2013 14:43:02 +0900 (KST) X-AuditID: cbfee68f-b7f436d000000f81-f6-51ea2366c68d Received: from epextmailer03 ( [203.254.219.153]) by epcpsbgx2.samsung.com (EPCPMTA) with SMTP id AB.3E.07473.5632AE15; Sat, 20 Jul 2013 14:43:01 +0900 (KST) Date: Sat, 20 Jul 2013 05:43:01 +0000 (GMT) From: NAVEEN KRISHNA CHATRADHI Subject: Re: [PATCH] of: provide of_platform_unpopulate() To: Sebastian Andrzej Siewior , Rob Herring , Grant Likely Cc: "linux-omap@vger.kernel.org" , "linux-samsung-soc@vger.kernel.org" , "devicetree-discuss@lists.ozlabs.org" , "linux-kernel@vger.kernel.org" , Tony Lindgren , Doug Anderson , Vivek Gautam , Kukjin Kim , Kishon Vijay Abraham I , Roger Quadros , George Cherian , Felipe Balbi Reply-to: ch.naveen@samsung.com MIME-version: 1.0 X-MTR: 20130720053751716@ch.naveen Msgkey: 20130720053751716@ch.naveen X-EPLocale: en_US.windows-1252 X-Priority: 3 X-EPWebmail-Msg-Type: personal X-EPWebmail-Reply-Demand: 0 X-EPApproval-Locale: X-EPHeader: ML X-EPTrCode: X-EPTrName: X-MLAttribute: X-RootMTR: 20130720053751716@ch.naveen X-ParentMTR: X-ArchiveUser: X-CPGSPASS: N Content-transfer-encoding: base64 Content-type: text/plain; charset=windows-1252 MIME-version: 1.0 Message-id: <2333396.179421374298979822.JavaMail.weblogic@epml22> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupnleLIzCtJLcpLzFFi42JZI2JSrZum/CrQYOUHNosZ5/cxOTB6fN4k F8AYxWWTkpqTWZZapG+XwJVx8N02toIDmRVTph1jaWB8kNbFyMkhJKAi8XfHdFYQW0LAROLr +X3MELaYxIV769m6GLmAapYySnx4MoMdpmj+0wPMEIn5jBK/VsxlAUmwCKhKzJ89A8xmEzCX eHfiB9hUYQFLidZNk1hAGkQE+hgl7p1tZQdxmAVusUh8XXOOCeIOeYkZEzvBunkFBCVOznwC ZHMArVOSePOoGsTkFVCW+P7bEeIIOYklUy8zQdi8EjPan7LAxKd9XQP1gbTE+VkbGGG+Wfz9 MVScX+LY7R1MENN5JZ7cD4YZs3vzFzYIW0Bi6pmDUK3qEr+WLoNq5ZNYs/AtC8yYXaeWM8P0 3t8yF+wcZgFFiSndD9khbAOJI4vmsKJ6CsR2lHi/fSLzBEblWUhSs5C0z0LSjqxmASPLKkbR 1ILkguKk9CJjveLE3OLSvHS95PzcTYzAxHD637P+HYx3D1gfYkwGRslEZinR5HxgYskriTc0 NjOyMDUxNTYytzQjTVhJnFetxTpQSCA9sSQ1OzW1ILUovqg0J7X4ECMTB6dUA+OK7ROWXZVt jNgdobaEKT36wbWMrUwFDfZsP/urN/2QWyLxdke6VE1v8O5nRuZ/YnIjth6787hqduFNBQM/ nWdJb3P9er5cVxWLu124TfNIouChS/UX+A5e8Nt07FryxIxWzqzCV4ejRL/OmXfpcKPIBgPr T+Frp517+eDo9cDgZ4GLpeysJ0QpsRRnJBpqMRcVJwIAqAcN+yIDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpgk+LIzCtJLcpLzFFi42I5/e/2TN1U5VeBBo+/WFvMOL+PyYHR4/Mm uQDGqDSbjNTElNQihdS85PyUzLx0WyXv4HjneFMzA0NdQ0sLcyWFvMTcVFslF58AXbfMHKCh SgpliTmlQKGAxOJiJX07m6L80pJUhYz84hJbpWhDcyM9IwM9UyM9Q9NYK0MDAyNToJqEtIyD 77axFRzIrJgy7RhLA+ODtC5GTg4hARWJvzums4LYEgImEvOfHmCGsMUkLtxbz9bFyAVUM59R 4teKuSwgCRYBVYn5s2eA2WwC5hLvTvwAaxYWsJRo3TSJBaRBRKCPUeLe2VZ2EIdZ4BaLxNc1 55gg1slLzJjYCdbNKyAocXLmEyCbA2idksSbR9UgJq+AssT3344QR8hJLJl6mQnC5pWY0f6U BSY+7esaqEOlJc7P2sAIc/Ti74+h4vwSx27vYIKYzivx5H4wzJjdm7+wQdgCElPPHIRqVZf4 tXQZVCufxJqFb1lgxuw6tZwZpvf+lrlg5zALKEpM6X7IDmEbSBxZNIcV1VMgtqPE++0TmScw ys1CkpqFpH0WknZkNQsYWVYxiqYWJBcUJ6VXGOkVJ+YWl+al6yXn525iBKenZ4t2MP47b32I UYCDUYmH98KKl4FCrIllxZW5hxglOJiVRHjZ7wCFeFMSK6tSi/Lji0pzUosPMSYDI3Ais5Ro cj4wdeaVxBsam5ibGptaGBiam5uRJqwkzvus1TpQSCA9sSQ1OzW1ILUIZgsTB6dUA2PFpnWr I2S+TAovFb4vOIOH00ezQt6YUeTJ9obJxfp78kQCIwNT+825gxekfnznlvWC92/T7n7L4zvY NuQbJdse2LLZ+67yQX1jZpfeVKvlDk7iP69qLO3yWLn5v6JZtKlM41eW5c3R0xo33HVYpKm4 VVdl9Rcvz6ztqe4is25P2r64vaHqiRJLcUaioRZzUXEiAK0czI2TAwAA DLP-Filter: Pass X-CFilter-Loop: Reflected Sender: linux-samsung-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-samsung-soc@vger.kernel.org X-Spam-Status: No, score=-5.8 required=5.0 tests=BASE64_LENGTH_79_INF, BAYES_00, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hello Sebastian, I just did one more testing. In case of iio/adc/exynos_adc.c there is a bug in the remove path. If I fix the bug in the driver, with below patch --- a/drivers/iio/adc/exynos_adc.c +++ b/drivers/iio/adc/exynos_adc.c @@ -375,14 +375,14 @@ static int exynos_adc_remove(struct platform_device *pdev) struct iio_dev *indio_dev = platform_get_drvdata(pdev); struct exynos_adc *info = iio_priv(indio_dev); - device_for_each_child(&pdev->dev, NULL, - exynos_adc_remove_devices); regulator_disable(info->vdd); clk_disable_unprepare(info->clk); writel(0, info->enable_reg); iio_device_unregister(indio_dev); free_irq(info->irq, info); iio_device_free(indio_dev); + device_for_each_child(&pdev->dev, NULL, + exynos_adc_remove_devices); Even without your fix, I could configure it as a module and the rmmod, insmod are working fine. (no crash) Regards, Naveen ------- Original Message ------- Sender : Sebastian Andrzej Siewior Date : Jul 19, 2013 23:44 (GMT+05:30) Title : [PATCH] of: provide of_platform_unpopulate() So I called of_platform_populate() on a device to get each child device probed and on rmmod and I need to reverse its doing. After a quick grep I did what others did as well and rmmod ended in: | Unable to handle kernel NULL pointer dereference at virtual address 00000018 | PC is at release_resource+0x18/0x80 | Process rmmod (pid: 2005, stack limit = 0xedc30238) | [] (release_resource+0x18/0x80) from [] (platform_device_del+0x78/0xac) | [] (platform_device_del+0x78/0xac) from [] (platform_device_unregister+0xc/0x18) The problem is that platform_device_del() "releases" each ressource in its tree. This does not work on platform_devices created by OF becuase they were never added via insert_resource(). As a consequence old->parent in __release_resource() is NULL and we explode while accessing ->child. So I either I do something completly wrong _or_ nobody here tested the rmmod path of their driver. This patch provides a common function to unregister / remove devices which added to the system via of_platform_populate(). While this works now on my test case I have not tested any of the driver I modify here so feedback is greatly appreciated. Cc: Tony Lindgren Cc: Doug Anderson Cc: Vivek Gautam Cc: Naveen Krishna Chatradhi Cc: Kukjin Kim Cc: Kishon Vijay Abraham I Cc: Roger Quadros Cc: George Cherian Cc: Felipe Balbi Signed-off-by: Sebastian Andrzej Siewior --- drivers/bus/omap-ocp2scp.c | 13 ++----------- drivers/iio/adc/exynos_adc.c | 15 ++------------- drivers/mfd/omap-usb-host.c | 9 +-------- drivers/of/platform.c | 22 ++++++++++++++++++++++ drivers/usb/dwc3/dwc3-exynos.c | 11 +---------- drivers/usb/dwc3/dwc3-omap.c | 12 +----------- include/linux/of_platform.h | 4 ++++ 7 files changed, 33 insertions(+), 53 deletions(-) diff --git a/drivers/bus/omap-ocp2scp.c b/drivers/bus/omap-ocp2scp.c index 5511f98..510bb9e 100644 --- a/drivers/bus/omap-ocp2scp.c +++ b/drivers/bus/omap-ocp2scp.c @@ -23,15 +23,6 @@ #include #include -static int ocp2scp_remove_devices(struct device *dev, void *c) -{ - struct platform_device *pdev = to_platform_device(dev); - - platform_device_unregister(pdev); - - return 0; -} - static int omap_ocp2scp_probe(struct platform_device *pdev) { int ret; @@ -51,7 +42,7 @@ static int omap_ocp2scp_probe(struct platform_device *pdev) return 0; err0: - device_for_each_child(&pdev->dev, NULL, ocp2scp_remove_devices); + of_platform_unpopulate(&pdev->dev); return ret; } @@ -59,7 +50,7 @@ static int omap_ocp2scp_probe(struct platform_device *pdev) static int omap_ocp2scp_remove(struct platform_device *pdev) { pm_runtime_disable(&pdev->dev); - device_for_each_child(&pdev->dev, NULL, ocp2scp_remove_devices); + of_platform_unpopulate(&pdev->dev); return 0; } diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c index 9809fc9..10248e1 100644 --- a/drivers/iio/adc/exynos_adc.c +++ b/drivers/iio/adc/exynos_adc.c @@ -216,15 +216,6 @@ static const struct iio_chan_spec exynos_adc_iio_channels[] = { ADC_CHANNEL(9, "adc9"), }; -static int exynos_adc_remove_devices(struct device *dev, void *c) -{ - struct platform_device *pdev = to_platform_device(dev); - - platform_device_unregister(pdev); - - return 0; -} - static void exynos_adc_hw_init(struct exynos_adc *info) { u32 con1, con2; @@ -357,8 +348,7 @@ static int exynos_adc_probe(struct platform_device *pdev) return 0; err_of_populate: - device_for_each_child(&pdev->dev, NULL, - exynos_adc_remove_devices); + of_platform_unpopulate(&pdev->dev); regulator_disable(info->vdd); clk_disable_unprepare(info->clk); err_iio_dev: @@ -375,8 +365,7 @@ static int exynos_adc_remove(struct platform_device *pdev) struct iio_dev *indio_dev = platform_get_drvdata(pdev); struct exynos_adc *info = iio_priv(indio_dev); - device_for_each_child(&pdev->dev, NULL, - exynos_adc_remove_devices); + of_platform_unpopulate(&pdev->dev); regulator_disable(info->vdd); clk_disable_unprepare(info->clk); writel(0, info->enable_reg); diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c index 759fae3..bb26cea 100644 --- a/drivers/mfd/omap-usb-host.c +++ b/drivers/mfd/omap-usb-host.c @@ -832,13 +832,6 @@ static int usbhs_omap_probe(struct platform_device *pdev) return ret; } -static int usbhs_omap_remove_child(struct device *dev, void *data) -{ - dev_info(dev, "unregistering "); - platform_device_unregister(to_platform_device(dev)); - return 0; -} - /** * usbhs_omap_remove - shutdown processing for UHH & TLL HCDs * @pdev: USB Host Controller being removed @@ -871,7 +864,7 @@ static int usbhs_omap_remove(struct platform_device *pdev) pm_runtime_disable(&pdev->dev); /* remove children */ - device_for_each_child(&pdev->dev, NULL, usbhs_omap_remove_child); + of_platform_unpopulate(&pdev->dev); return 0; } diff --git a/drivers/of/platform.c b/drivers/of/platform.c index e0a6514..9cbb0c3 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -473,4 +473,26 @@ int of_platform_populate(struct device_node *root, return rc; } EXPORT_SYMBOL_GPL(of_platform_populate); + +static int of_remove_populated_child(struct device *dev, void *d) +{ + struct platform_device *pdev = to_platform_device(dev); + + of_device_unregister(pdev); + return 0; +} +/** + * of_platform_unpopulate() - Remove populated devices. + * @parent: parent of the populated devices. + * + * The reverse of of_platform_populate() and can only be used a parent was + * specified while invoking the former. + */ +void of_platform_unpopulate(struct device *parent) +{ + if (WARN_ON(!parent)) + return; + device_for_each_child(parent, NULL, of_remove_populated_child); +} +EXPORT_SYMBOL_GPL(of_platform_unpopulate); #endif /* CONFIG_OF_ADDRESS */ diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index 8ce9d7f..2bf5664 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -86,15 +86,6 @@ static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos) return ret; } -static int dwc3_exynos_remove_child(struct device *dev, void *unused) -{ - struct platform_device *pdev = to_platform_device(dev); - - platform_device_unregister(pdev); - - return 0; -} - static int dwc3_exynos_probe(struct platform_device *pdev) { struct dwc3_exynos *exynos; @@ -164,7 +155,7 @@ static int dwc3_exynos_remove(struct platform_device *pdev) { struct dwc3_exynos *exynos = platform_get_drvdata(pdev); - device_for_each_child(&pdev->dev, NULL, dwc3_exynos_remove_child); + of_platform_unpopulate(&pdev->dev); platform_device_unregister(exynos->usb2_phy); platform_device_unregister(exynos->usb3_phy); diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 077f110..8688613 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -327,15 +327,6 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap) return IRQ_HANDLED; } -static int dwc3_omap_remove_core(struct device *dev, void *c) -{ - struct platform_device *pdev = to_platform_device(dev); - - platform_device_unregister(pdev); - - return 0; -} - static void dwc3_omap_enable_irqs(struct dwc3_omap *omap) { u32 reg; @@ -529,8 +520,7 @@ static int dwc3_omap_remove(struct platform_device *pdev) dwc3_omap_disable_irqs(omap); pm_runtime_put_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); - device_for_each_child(&pdev->dev, NULL, dwc3_omap_remove_core); - + of_platform_unpopulate(&pdev->dev); return 0; } diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h index 05cb4a9..e354f9c 100644 --- a/include/linux/of_platform.h +++ b/include/linux/of_platform.h @@ -72,6 +72,7 @@ extern int of_platform_populate(struct device_node *root, const struct of_device_id *matches, const struct of_dev_auxdata *lookup, struct device *parent); +extern void of_platform_unpopulate(struct device *parent); #else static inline int of_platform_populate(struct device_node *root, const struct of_device_id *matches, @@ -80,6 +81,9 @@ static inline int of_platform_populate(struct device_node *root, { return -ENODEV; } +static inline void of_platform_unpopulate(struct device *parent) +{ +} #endif #endif /* _LINUX_OF_PLATFORM_H */