From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753260AbcKURr7 (ORCPT ); Mon, 21 Nov 2016 12:47:59 -0500 Received: from foss.arm.com ([217.140.101.70]:39052 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751688AbcKURr5 (ORCPT ); Mon, 21 Nov 2016 12:47:57 -0500 Subject: Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver To: Bartosz Golaszewski , Sekhar Nori References: <1477925138-23457-1-git-send-email-bgolaszewski@baylibre.com> <1477925138-23457-2-git-send-email-bgolaszewski@baylibre.com> Cc: Mark Rutland , linux-devicetree , Tomi Valkeinen , David Airlie , Kevin Hilman , Michael Turquette , Russell King , linux-drm , LKML , Rob Herring , Jyri Sarha , Frank Rowand , arm-soc , Laurent Pinchart From: Robin Murphy Message-ID: Date: Mon, 21 Nov 2016 17:47:52 +0000 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: 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 Hi Bartosz, Sekhar, On 21/11/16 16:48, Bartosz Golaszewski wrote: > 2016-11-21 17:33 GMT+01:00 Sekhar Nori : >> 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_machine_name()); >> return -EINVAL; >> } >> ---8<--- >> >> A similar fix is required for the other driver in this series (patch >> 2/5). I need some advise on whether I should introduce a common >> function to get the machine name post kernel boot-up (I cannot see an >> existing one). If yes, any advise on which file it should go into? >> > > Hi Sekhar, > > thanks for spotting that. > > I think we should introduce this function right away, rather than > having two static functions doing the same thing. If you don't mind, > I'll try to find a good spot for it and send a follow-up series fixing > the issue. As it happens, that was already proposed last week, for much the same reason: http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg111395.html Robin. > > Best regards, > Bartosz Golaszewski > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >