From patchwork Mon Jan 21 18:25:45 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Sealey X-Patchwork-Id: 2013681 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id C7F9E3FDD2 for ; Mon, 21 Jan 2013 18:28:29 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TxM4K-00054x-Un; Mon, 21 Jan 2013 18:26:05 +0000 Received: from mail-ob0-f170.google.com ([209.85.214.170]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TxM4E-0004zm-DK for linux-arm-kernel@lists.infradead.org; Mon, 21 Jan 2013 18:26:00 +0000 Received: by mail-ob0-f170.google.com with SMTP id wp18so6278235obc.29 for ; Mon, 21 Jan 2013 10:25:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer :x-gm-message-state; bh=ZjzSDDhxuqBFWpjx+rfb2kV5rr/s21xvpOtHFa9mDzs=; b=HhewBkWhd3H02hgYd3EPJm5dlg/8WtWh6ORclTQfk1ZAmhBFyTWIimyOfq+PA06kdl hY7P65Xk1ENLC2Nluh6gqyYLD9756Fi/aYEQyTJp+U0EGgtI1jWy/QRQqZKR4ZX9zCbw XG/ZXklWkunbfpYlFa8sdMmthpra5TOCTuptQY71KY3oJCwi8xWjrfBnn8bAII6i0COR ZAbrfC9kLVA9gWM6ooQOe7rA9Df9uOeKT4WvgDfl6/Nf1+JM3PADqrpjKDym0c/NFaNx fFE/heXcArq819yEEcnutoxRv7KXhzs1bmT6DtrxO4ZN0SwWL5xYjvQmGGjE8B2/p8iT YIfA== X-Received: by 10.60.32.200 with SMTP id l8mr15123879oei.43.1358792755292; Mon, 21 Jan 2013 10:25:55 -0800 (PST) Received: from queequeg.genesi-usa.com ([199.193.222.22]) by mx.google.com with ESMTPS id s8sm6776326obm.19.2013.01.21.10.25.53 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 21 Jan 2013 10:25:54 -0800 (PST) From: Matt Sealey To: linux-kernel@vger.kernel.org Subject: [PATCH] mc13892: sanity check num_regulators parsed vs. registered Date: Mon, 21 Jan 2013 12:25:45 -0600 Message-Id: <1358792745-1737-1-git-send-email-matt@genesi-usa.com> X-Mailer: git-send-email 1.7.10.4 X-Gm-Message-State: ALoCoQnrllGFj/evRloYC4SmqFC4AXu5P86UNxD/ZifdB53iN0JZs4lFjp7vj+2hSWfP20yQenk/ X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130121_132558_528098_369FEAAD X-CRM114-Status: GOOD ( 23.15 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.214.170 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Fabio Estevam , Matt Sealey , Mark Brown , linux-arm-kernel@lists.infradead.org, Shawn Guo X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Imagine a situation where a device tree has a few regulators in an appropriate node: regulators { sw1 { .. }; vvideo { .. }; : vfake { .. }; vtypo { .. }; }; In the above example, the node name "vfake" is an attempt to match a regulator name inside the driver which just so happens to not exist. The node name "vtypo" represents an accidental typographical error in a regulator name which may have been introduced to a device tree. In these cases, the number of regulators the mc13892 driver thinks it has does not match the number of regulators it parsed and registered. Since it will go over this array based on this number, it will actually re-register regulator "0" (which happens to be SW1) over and over again until it reaches the number, resulting in messages on the kernel log such as these: SW1: at 1100 mV VVIDEO: at 2775mV : SW1: at 1100 mV SW1: at 1100 mV .. up to that number of "mismatched" regulators. Nobody using DT can/will consume these regulators, so it should not be possible for it to cause any real regulator problems or driver breakages, but it is an easy thing to miss in a kernel log and is an immediate indication of a problem with the device tree authoring. This patch effectively sanity checks the number of counted children of the regulators node vs. the number that actually matched driver names, and sets the appropriate num_regulators value. It also gives a little warning for device tree authors that they MAY have screwed something up, such that this patch does not hide the device tree authoring problem. Signed-off-by: Matt Sealey Tested-by: Steev Klimaszewski Cc: Shawn Guo Cc: Fabio Estevam --- drivers/regulator/mc13892-regulator.c | 39 ++++++++++++++++++++++++++-- drivers/regulator/mc13xxx-regulator-core.c | 10 +++++-- drivers/regulator/mc13xxx.h | 4 +-- 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/drivers/regulator/mc13892-regulator.c b/drivers/regulator/mc13892-regulator.c index e4f2197..7c86040 100644 --- a/drivers/regulator/mc13892-regulator.c +++ b/drivers/regulator/mc13892-regulator.c @@ -535,15 +535,18 @@ static int mc13892_regulator_probe(struct platform_device *pdev) struct mc13xxx_regulator_init_data *mc13xxx_data; struct regulator_config config = { }; int i, ret; - int num_regulators = 0; + int num_regulators = 0, num_parsed; u32 val; num_regulators = mc13xxx_get_num_regulators_dt(pdev); + if (num_regulators <= 0 && pdata) num_regulators = pdata->num_regulators; if (num_regulators <= 0) return -EINVAL; + num_parsed = num_regulators; + priv = devm_kzalloc(&pdev->dev, sizeof(*priv) + num_regulators * sizeof(priv->regulators[0]), GFP_KERNEL); @@ -586,7 +589,39 @@ static int mc13892_regulator_probe(struct platform_device *pdev) = mc13892_vcam_get_mode; mc13xxx_data = mc13xxx_parse_regulators_dt(pdev, mc13892_regulators, - ARRAY_SIZE(mc13892_regulators)); + ARRAY_SIZE(mc13892_regulators), + &num_parsed); + + /* + * Perform a little sanity check on the regulator tree - if we found + * a number of regulators from mc13xxx_get_num_regulators_dt and + * then parsed a smaller number in mc13xxx_parse_regulators_dt then + * there is a regulator defined in the regulators node which has + * not matched any usable regulator in the driver. In this case, + * there is one missing and what will happen is the first regulator + * will get registered again. + * + * Fix this by basically making our number of registerable regulators + * equal to the number of regulators we parsed. We are allocating + * too much memory for priv, but this is unavoidable at this point. + * + * As an example of how this can happen, try making a typo in your + * regulators node (vviohi {} instead of viohi {}) so that the name + * does not match.. + * + * The check will basically pass for platform data (non-DT) because + * mc13xxx_parse_regulators_dt for !CONFIG_OF will not touch num_parsed. + * + */ + if (num_parsed != num_regulators) { + dev_warn(&pdev->dev, + "parsed %d != regulators %d - check your device tree!\n", + num_parsed, num_regulators); + + num_regulators = num_parsed; + priv->num_regulators = num_regulators; + } + for (i = 0; i < num_regulators; i++) { struct regulator_init_data *init_data; struct regulator_desc *desc; diff --git a/drivers/regulator/mc13xxx-regulator-core.c b/drivers/regulator/mc13xxx-regulator-core.c index 4ed89c6..2ecf1d8 100644 --- a/drivers/regulator/mc13xxx-regulator-core.c +++ b/drivers/regulator/mc13xxx-regulator-core.c @@ -181,12 +181,14 @@ EXPORT_SYMBOL_GPL(mc13xxx_get_num_regulators_dt); struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt( struct platform_device *pdev, struct mc13xxx_regulator *regulators, - int num_regulators) + int num_regulators, int *num_parsed) { struct mc13xxx_regulator_priv *priv = platform_get_drvdata(pdev); struct mc13xxx_regulator_init_data *data, *p; struct device_node *parent, *child; - int i; + int i, parsed = 0; + + *num_parsed = 0; of_node_get(pdev->dev.parent->of_node); parent = of_find_node_by_name(pdev->dev.parent->of_node, "regulators"); @@ -203,16 +205,20 @@ struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt( for (i = 0; i < num_regulators; i++) { if (!of_node_cmp(child->name, regulators[i].desc.name)) { + p->id = i; p->init_data = of_get_regulator_init_data( &pdev->dev, child); p->node = child; p++; + + parsed++; break; } } } + *num_parsed = parsed; return data; } EXPORT_SYMBOL_GPL(mc13xxx_parse_regulators_dt); diff --git a/drivers/regulator/mc13xxx.h b/drivers/regulator/mc13xxx.h index 06c8903..007f833 100644 --- a/drivers/regulator/mc13xxx.h +++ b/drivers/regulator/mc13xxx.h @@ -39,7 +39,7 @@ extern int mc13xxx_fixed_regulator_set_voltage(struct regulator_dev *rdev, extern int mc13xxx_get_num_regulators_dt(struct platform_device *pdev); extern struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt( struct platform_device *pdev, struct mc13xxx_regulator *regulators, - int num_regulators); + int num_regulators, int *num_parsed); #else static inline int mc13xxx_get_num_regulators_dt(struct platform_device *pdev) { @@ -48,7 +48,7 @@ static inline int mc13xxx_get_num_regulators_dt(struct platform_device *pdev) static inline struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt( struct platform_device *pdev, struct mc13xxx_regulator *regulators, - int num_regulators) + int num_regulators, int *num_parsed) { return NULL; }