Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932164AbbKWSi7 (ORCPT ); Mon, 23 Nov 2015 13:38:59 -0500 Received: from proxima.lp0.eu ([81.2.80.65]:42174 "EHLO proxima.lp0.eu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752723AbbKWSi4 (ORCPT ); Mon, 23 Nov 2015 13:38:56 -0500 Subject: Re: [PATCH (v4) 2/2] mtd: brcmnand: Add support for the BCM63268 To: Jonas Gorski References: <56506D55.3000907@simon.arlott.org.uk> <20151122215945.GA5930@rob-hp-laptop> <56523E85.905@simon.arlott.org.uk> <56523EFF.9050502@simon.arlott.org.uk> Cc: Rob Herring , "devicetree@vger.kernel.org" , Brian Norris , Linux Kernel Mailing List , David Woodhouse , MTD Maling List , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Florian Fainelli From: Simon Arlott Message-ID: <56535D15.7070307@simon.arlott.org.uk> Date: Mon, 23 Nov 2015 18:38:13 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2812 Lines: 76 On 23/11/15 15:42, Jonas Gorski wrote: > On Sun, Nov 22, 2015 at 11:17 PM, Simon Arlott wrote: >> + priv->clk = of_clk_get(dev->of_node, 0); > > > Why not use a named clock here? That way you can make use of > devm_clk_get and don't need to care about putting it. Ok. >> + >> +static struct platform_driver bcm63268_nand_driver = { >> + .probe = bcm63268_nand_probe, >> + .remove = brcmnand_remove, >> + .driver = { >> + .name = "bcm63268_nand", >> + .pm = &brcmnand_pm_ops, >> + .of_match_table = bcm63268_nand_of_match, >> + } >> +}; >> +module_platform_driver(bcm63268_nand_driver); >> + >> +MODULE_LICENSE("GPL"); >> +MODULE_AUTHOR("Simon Arlott"); >> +MODULE_DESCRIPTION("NAND driver for BCM63268"); >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c >> index 2c8f67f..7b4988f 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.c >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c >> @@ -2270,6 +2270,9 @@ int brcmnand_remove(struct platform_device *pdev) >> list_for_each_entry(host, &ctrl->host_list, node) >> nand_release(&host->mtd); >> >> + if (ctrl->soc && ctrl->soc->remove_device) >> + ctrl->soc->remove_device(ctrl->soc); >> + > > This is a bit weird, why don't you just use your own .remove callback > for the driver that like you did for .probe? then you won't need to > add a remove_device callback at all. I don't have access to the struct brcmnand_soc which is stored by brcmnand_probe() as the containing struct is defined in brcmnand.c. I could move that struct out to a header file and use it directly from a bcm63268_nand_remove() function but I'd have to move a few other defines too. >> dev_set_drvdata(&pdev->dev, NULL); >> >> return 0; >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.h b/drivers/mtd/nand/brcmnand/brcmnand.h >> index ef5eabb..5c5dc2d 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.h >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.h >> @@ -24,6 +24,7 @@ struct brcmnand_soc { >> bool (*ctlrdy_ack)(struct brcmnand_soc *soc); >> void (*ctlrdy_set_enabled)(struct brcmnand_soc *soc, bool en); >> void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare); >> + void (*remove_device)(struct brcmnand_soc *soc); >> }; >> >> static inline void brcmnand_soc_data_bus_prepare(struct brcmnand_soc *soc) > > > Jonas > -- Simon Arlott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/