lkml.org 
[lkml]   [2013]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v3 01/36] mtd: st_spi_fsm: Allocate resources and register with MTD framework
    On Fri, Nov 29, 2013 at 12:18:50PM +0000, Lee Jones wrote:
    > --- /dev/null
    > +++ b/drivers/mtd/devices/st_spi_fsm.c
    > @@ -0,0 +1,111 @@
    > +/*
    > + * st_spi_fsm.c Support for ST Serial Flash Controller
    > + *
    > + * Author: Angus Clark <angus.clark@st.com>
    > + *
    > + * Copyright (C) 2010-2013 STicroelectronics Limited
    Should be STMicrolectronics? :)

    > + *
    > + * JEDEC probe based on drivers/mtd/devices/m25p80.c
    > + *
    > + * This code is free software; you can redistribute it and/or modify
    > + * it under the terms of the GNU General Public License version 2 as
    > + * published by the Free Software Foundation.
    > + *
    > + */
    > +#include <linux/kernel.h>
    > +#include <linux/module.h>
    > +#include <linux/platform_device.h>
    > +#include <linux/mtd/mtd.h>
    > +#include <linux/sched.h>
    > +#include <linux/delay.h>
    > +#include <linux/io.h>
    > +#include <linux/of.h>
    > +
    > +#include <asm/io.h>
    > +
    > +#include "st_spi_fsm.h"

    I agree with Srinivas. Unless you think this header can be used in
    another file, it should probably be part of this .c file. It also lends
    itself to some strange usage of 'static' functions declared (but not
    defined) in the header. But I'll comment when I get to those patches.

    > +
    > +static int stfsm_probe(struct platform_device *pdev)
    > +{
    > + struct device_node *np = pdev->dev.of_node;
    > + struct resource *res;
    > + struct stfsm *fsm;
    > +
    > + if (!np) {
    > + dev_err(&pdev->dev, "No DT found\n");
    > + return -EINVAL;
    > + }
    > +
    > + fsm = devm_kzalloc(&pdev->dev, sizeof(*fsm), GFP_KERNEL);
    > + if (!fsm)
    > + return -ENOMEM;
    > +
    > + fsm->dev = &pdev->dev;
    > +
    > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
    > + if (!res) {
    > + dev_err(&pdev->dev, "Resource not found\n");
    > + return -ENODEV;
    > + }
    > +
    > + fsm->region = devm_request_mem_region(&pdev->dev, res->start,
    > + resource_size(res), pdev->name);
    > + if (!fsm->region) {
    > + dev_err(&pdev->dev,
    > + "Failed to reserve memory region [0x%08x-0x%08x]\n",

    Use the special %pr or %pR format specifier? See
    Documentation/printk-formats.txt

    > + res->start, res->end);
    > + return -EBUSY;
    > + }
    > +
    > + fsm->base = devm_ioremap_nocache(&pdev->dev,
    > + res->start, resource_size(res));
    > + if (!fsm->base) {
    > + dev_err(&pdev->dev, "Failed to ioremap [0x%08x]\n", res->start);

    Also %pr or %pR?

    > + return -EINVAL;
    > + }
    > +
    > + mutex_init(&fsm->lock);
    > +
    > + platform_set_drvdata(pdev, fsm);
    > +
    > + fsm->mtd.dev.parent = &pdev->dev;
    > + fsm->mtd.type = MTD_NORFLASH;
    > + fsm->mtd.writesize = 4;
    > + fsm->mtd.writebufsize = fsm->mtd.writesize;
    > + fsm->mtd.flags = MTD_CAP_NORFLASH;
    > +
    > + return mtd_device_parse_register(&fsm->mtd, NULL, NULL, NULL, 0);
    > +}
    > +
    > +static int stfsm_remove(struct platform_device *pdev)
    > +{
    > + struct stfsm *fsm = platform_get_drvdata(pdev);
    > + int err;
    > +
    > + err = mtd_device_unregister(&fsm->mtd);
    > + if (err)
    > + return err;
    > +
    > + return 0;

    Simplify to the following?

    return mtd_device_unregister(&fsm->mtd);

    > +}

    [...]

    Brian


    \
     
     \ /
      Last update: 2013-12-10 20:41    [W:4.248 / U:0.584 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site