Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753382AbcKVG0M (ORCPT ); Tue, 22 Nov 2016 01:26:12 -0500 Received: from lelnx193.ext.ti.com ([198.47.27.77]:24708 "EHLO lelnx193.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752541AbcKVG0L (ORCPT ); Tue, 22 Nov 2016 01:26:11 -0500 Subject: Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver To: Frank Rowand , Bartosz Golaszewski , Kevin Hilman , Michael Turquette , Rob Herring , Mark Rutland , Peter Ujfalusi , Russell King References: <1477925138-23457-1-git-send-email-bgolaszewski@baylibre.com> <1477925138-23457-2-git-send-email-bgolaszewski@baylibre.com> <5833A2DA.40701@gmail.com> CC: LKML , arm-soc , linux-drm , linux-devicetree , Jyri Sarha , Tomi Valkeinen , David Airlie , Laurent Pinchart , Sudeep Holla From: Sekhar Nori Message-ID: <800171a8-2e2c-2afb-f96d-800a17eaa17a@ti.com> Date: Tue, 22 Nov 2016 11:55:21 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <5833A2DA.40701@gmail.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2607 Lines: 77 Hi Frank, On Tuesday 22 November 2016 07:13 AM, Frank Rowand wrote: > On 11/21/16 08:33, Sekhar Nori wrote: >> On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote: >>> +static int da8xx_ddrctl_probe(struct platform_device *pdev) >>> +{ >>> + const struct da8xx_ddrctl_config_knob *knob; >>> + const struct da8xx_ddrctl_setting *setting; >>> + struct device_node *node; >>> + struct resource *res; >>> + void __iomem *ddrctl; >>> + struct device *dev; >>> + u32 reg; >>> + >>> + dev = &pdev->dev; >>> + node = dev->of_node; >>> + >>> + setting = da8xx_ddrctl_get_board_settings(); >>> + if (!setting) { >>> + dev_err(dev, "no settings for board '%s'\n", >>> + of_flat_dt_get_machine_name()); >>> + return -EINVAL; >>> + } >> >> This causes a section mismatch because of_flat_dt_get_machine_name() >> has an __init annotation. I did not notice that before, sorry. >> >> It can be fixed with a patch like below: >> >> ---8<--- >> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c >> index a20e7bbbcbe0..9ca5aab3ac54 100644 >> --- a/drivers/memory/da8xx-ddrctl.c >> +++ b/drivers/memory/da8xx-ddrctl.c >> @@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting *da8xx_ddrctl_get_board_settings(void) >> return NULL; >> } >> >> +static const char* da8xx_ddrctl_get_machine_name(void) >> +{ >> + const char *str; >> + int ret; >> + >> + ret = of_property_read_string(of_root, "model", &str); >> + if (ret) >> + ret = of_property_read_string(of_root, "compatible", &str); >> + >> + return str; >> +} >> + >> static int da8xx_ddrctl_probe(struct platform_device *pdev) >> { >> const struct da8xx_ddrctl_config_knob *knob; >> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev) >> setting = da8xx_ddrctl_get_board_settings(); >> if (!setting) { >> dev_err(dev, "no settings for board '%s'\n", >> - of_flat_dt_get_machine_name()); > > da8xx_ddrctl_get_board_settings() tries to match based on the "compatible" > property in the root node. The "model" property in the root node has > nothing to do with the failure to match. So creating and then using > da8xx_ddrctl_get_machine_name() to potentially report model is not useful. > > It should be sufficient to simply report that no compatible matched. I agree with you on this. Even if model name is printed, you will have to go back and check the compatible anyway. But I think it will be useful to print the compatible instead of just reporting that nothing matched. Bartosz, if you agree too, could you send a fix patch just printing the compatible? Thanks, Sekhar