From patchwork Wed May 7 14:37:17 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pawel Moll X-Patchwork-Id: 29771 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-pa0-f72.google.com (mail-pa0-f72.google.com [209.85.220.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 2133320534 for ; Wed, 7 May 2014 14:37:31 +0000 (UTC) Received: by mail-pa0-f72.google.com with SMTP id rd3sf4742398pab.7 for ; Wed, 07 May 2014 07:37:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:subject:from:to:cc:date :in-reply-to:references:mime-version:sender:precedence:list-id :x-original-sender:x-original-authentication-results:mailing-list :list-post:list-help:list-archive:list-unsubscribe:content-type :content-transfer-encoding; bh=HkEaY6ij22wAz5W2WNqKZzbUusXRZhQj0O6t9acJ2Is=; b=kXpkwddc29ICpPkILbmy+xKyBWBXFaElCAZS2tVv+c170F631vwrrq0eMMF2C4wD6a +X7n3orZDASPhd4uORTeaKYL0L5C6vJljZAfA3QWxZ241gfrWBSAE3y2sLSfY231H4JB yDQUmKkhfTQiPu2oQBKuWJ25CeWzhy+N2GSuEAqt/RSPsmFsneOa/6X3mQkK6weEIV86 G2TS7WcGyWYp5jeKasmDpXP8oCdA4Ap7kkvSjG0pmoJ6EbX+i1HW0Sd9vZZ6+uUwFbpi WPH2SKyLWXagPmN67FvpltJ9uAKSwkW/F33cIoA6xTQKuNhpsYaTBIlP0onQ5ZeclXYF Cdow== X-Gm-Message-State: ALoCoQnYzBDDPt/3x532pjfkaoxqSC8+f0iN2RdZJhTFJJ5IgvJIVBOfWNbKdfICMaZr5TdmN1gT X-Received: by 10.66.102.36 with SMTP id fl4mr4429346pab.20.1399473451083; Wed, 07 May 2014 07:37:31 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.97.200 with SMTP id m66ls3288718qge.83.gmail; Wed, 07 May 2014 07:37:30 -0700 (PDT) X-Received: by 10.221.62.131 with SMTP id xa3mr39366104vcb.13.1399473450930; Wed, 07 May 2014 07:37:30 -0700 (PDT) Received: from mail-ve0-f182.google.com (mail-ve0-f182.google.com [209.85.128.182]) by mx.google.com with ESMTPS id kj3si2928416vdb.186.2014.05.07.07.37.30 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 07 May 2014 07:37:30 -0700 (PDT) Received-SPF: none (google.com: patch+caf_=patchwork-forward=linaro.org@linaro.org does not designate permitted sender hosts) client-ip=209.85.128.182; Received: by mail-ve0-f182.google.com with SMTP id sa20so1390902veb.13 for ; Wed, 07 May 2014 07:37:30 -0700 (PDT) X-Received: by 10.221.40.193 with SMTP id tr1mr7148975vcb.31.1399473450752; Wed, 07 May 2014 07:37:30 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.220.221.72 with SMTP id ib8csp314871vcb; Wed, 7 May 2014 07:37:30 -0700 (PDT) X-Received: by 10.66.122.1 with SMTP id lo1mr19945142pab.118.1399473449158; Wed, 07 May 2014 07:37:29 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id xx4si13940392pac.109.2014.05.07.07.37.28; Wed, 07 May 2014 07:37:28 -0700 (PDT) Received-SPF: none (google.com: devicetree-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755872AbaEGOh1 (ORCPT + 8 others); Wed, 7 May 2014 10:37:27 -0400 Received: from fw-tnat.austin.arm.com ([217.140.110.23]:24187 "EHLO collaborate-mta1.arm.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753202AbaEGOh1 (ORCPT ); Wed, 7 May 2014 10:37:27 -0400 Received: from [10.2.201.45] (hornet.cambridge.arm.com [10.2.201.45]) by collaborate-mta1.arm.com (Postfix) with ESMTP id CBED113F88D; Wed, 7 May 2014 09:37:15 -0500 (CDT) Message-ID: <1399473437.3706.25.camel@hornet> Subject: Re: [PATCH v2] of: Keep track of populated platform devices From: Pawel Moll To: Grant Likely Cc: Rob Herring , Rob Herring , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" Date: Wed, 07 May 2014 15:37:17 +0100 In-Reply-To: References: <1398858529.24255.3.camel@hornet> <1398866717-20268-1-git-send-email-pawel.moll@arm.com> <20140501092635.05011C409DA@trevor.secretlab.ca> X-Mailer: Evolution 3.10.4-0ubuntu1 Mime-Version: 1.0 Sender: devicetree-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: devicetree@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: pawel.moll@arm.com X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: patch+caf_=patchwork-forward=linaro.org@linaro.org does not designate permitted sender hosts) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , On Thu, 2014-05-01 at 10:43 +0100, Grant Likely wrote: > > That doesn't work in the case where drivers use of_platform_populate(). > > MFD devices in particular make use of it. If the driver does not get > > cleared on removal of the devices, then all MFD users will be broken on > > driver unbind/rebind. * > > > > We really need to have the inverse of of_platform_populate which will > > remove all child and child's child devices and clear all flags and such. > > Right now moany drivers are open-coding the removal. Agreed, but, unless I'm missing some fundamental issue, I believe we can solve the flag clearing problem in a generic way: --8<----------- 8<----------- > static int __of_platform_unpopulate_device(struct device *dev, void *c) > { > if (!dev->of_node || !of_node_get_flag(dev->of_node, OF_POPULATED) > return 0; > > // recursively remove the children too --- I'd like to find a way > to do this non-recursively > device_for_each_child(dev, NULL, __of_platform_unpopulate_device); As of_platform_populate() is recursive itself, I don't see much problem with this. > // Need to check if the device should be explicitly unbound from > it's driver before removing it. However, I think the driver core takes > care of this for us. Yep, removing either a driver or a device unbinds all existing pairs. > // Remove based on the bus type > switch (dev->bus) { > case &platform_bus_type: To my surprise gcc said "error: pointers are not permitted as case values". One learns every day ;-) > platform_device_remove(to_platform_device(dev)); > break; > case &amba_bus_type: > amba_device_remove(to_platform_device(dev)); > break; > } > > // Should check here if there are any other children to the > device. It is probably bad to remove a device that still has children. > Need to check what the driver core will do. > } The reference count will be still be greater than 0, so the parent won't be harmed. Nevertheless it's better to consciously handle such case. At least I tried. Pawel Acked-by: Grant Likely --- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c index 3cf61a1..0f489fb 100644 --- a/drivers/amba/bus.c +++ b/drivers/amba/bus.c @@ -17,6 +17,7 @@ #include #include #include +#include #include @@ -268,6 +269,7 @@ static void amba_device_release(struct device *dev) { struct amba_device *d = to_amba_device(dev); + of_device_node_put(dev); if (d->res.parent) release_resource(&d->res); kfree(d); diff --git a/include/linux/of_device.h b/include/linux/of_device.h index ef37021..fd14d46 100644 --- a/include/linux/of_device.h +++ b/include/linux/of_device.h @@ -41,6 +41,8 @@ extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env static inline void of_device_node_put(struct device *dev) { + if (dev->of_node) + of_node_clear_flag(dev->of_node, OF_POPULATED); of_node_put(dev->of_node); } --8<----------- This will work even if one manually unregisters a DT-originating device, because of_device_node_put() would be called in both amba and platform device (kobj) release path. By the way, the fact that today amba_device_release() doesn't do of_device_node_put() seems like a bug to me? (node's reference counter won't be decremented) > The function shouldn't be too difficult. I would expect it to look > something like this. You'll need to check the details. 8<----------- diff --git a/drivers/of/platform.c b/drivers/of/platform.c index dd4328c..b6b5a2b 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -493,4 +493,49 @@ int of_platform_populate(struct device_node *root, return rc; } EXPORT_SYMBOL_GPL(of_platform_populate); + +static int of_platform_device_destroy(struct device *dev, void *data) +{ + int *parents_children_left = data; + int my_children_left = 0; + + /* Do not touch devices not populated from the device tree */ + if (!dev->of_node || !of_node_check_flag(dev->of_node, OF_POPULATED)) { + *parents_children_left++; + return 0; + } + + device_for_each_child(dev, &my_children_left, + of_platform_device_destroy); + if (my_children_left) { + *parents_children_left++; + return 0; + } + + if (dev->bus == &platform_bus_type) + platform_device_unregister(to_platform_device(dev)); + else if (dev->bus == &amba_bustype) + amba_device_unregister(to_amba_device(dev)); + + return 0; +} + +/** + * of_platform_depopulate() - Remove devices populated from device tree + * @parent: device which childred will be removed + * + * Complementary to of_platform_populate(), this function removes children + * of the given device (and, recurrently, their children) that have been + * created from their respective device tree nodes (and only those, + * leaving others - eg. manually created - unharmed). */ + */ +int of_platform_depopulate(struct device *parent) +{ + int children_left = 0; + + device_for_each_child(dev, &children_left, of_platform_device_destroy); + + return children_left ? -EBUSY : 0; +} +EXPORT_SYMBOL_GPL(of_platform_depopulate); #endif /* CONFIG_OF_ADDRESS */ diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h index 05cb4a9..ef4f4ad 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); +static inline int of_platform_depopulate(struct device *parent); #else static inline int of_platform_populate(struct device_node *root, const struct of_device_id *matches, @@ -80,6 +81,10 @@ static inline int of_platform_populate(struct device_node *root, { return -ENODEV; } +static inline int of_platform_depopulate(struct device *parent) +{ + return -ENODEV; +} #endif #endif /* _LINUX_OF_PLATFORM_H */