From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934257AbcKVSWg (ORCPT ); Tue, 22 Nov 2016 13:22:36 -0500 Received: from mail-pg0-f68.google.com ([74.125.83.68]:33771 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933584AbcKVSWf (ORCPT ); Tue, 22 Nov 2016 13:22:35 -0500 Subject: Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver To: Sekhar Nori , 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> <800171a8-2e2c-2afb-f96d-800a17eaa17a@ti.com> Cc: LKML , arm-soc , linux-drm , linux-devicetree , Jyri Sarha , Tomi Valkeinen , David Airlie , Laurent Pinchart , Sudeep Holla From: Frank Rowand Message-ID: <58348CB8.7050304@gmail.com> Date: Tue, 22 Nov 2016 10:21:44 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <800171a8-2e2c-2afb-f96d-800a17eaa17a@ti.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 On 11/21/16 22:25, Sekhar Nori wrote: > 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? Please note that the compatible property might contain several strings, not just a single string. > > Thanks, > Sekhar >