lkml.org 
[lkml]   [2016]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
    2016-11-21 17:33 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
    > 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.

    Best regards,
    Bartosz Golaszewski

    \
     
     \ /
      Last update: 2016-11-21 17:49    [W:4.502 / U:0.188 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site