Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934472AbbEMUY2 (ORCPT ); Wed, 13 May 2015 16:24:28 -0400 Received: from mail-pd0-f170.google.com ([209.85.192.170]:33322 "EHLO mail-pd0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753984AbbEMUYV (ORCPT ); Wed, 13 May 2015 16:24:21 -0400 Date: Wed, 13 May 2015 13:24:15 -0700 From: Brian Norris To: Arnd Bergmann Cc: linux-mtd@lists.infradead.org, Dmitry Torokhov , Anatol Pomazao , Ray Jui , Corneliu Doban , Jonathan Richardson , Scott Branden , Florian Fainelli , =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= , bcm-kernel-feedback-list@broadcom.com, Dan Ehrenberg , Gregory Fong , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Kevin Cernekee Subject: Re: [PATCH v4 08/11] mtd: brcmnand: add BCM63138 support Message-ID: <20150513202415.GI11598@ld-irv-0074> References: <1431478424-29230-1-git-send-email-computersforpeace@gmail.com> <7133995.DH7aRmliCR@wuerfel> <20150513194521.GF11598@ld-irv-0074> <5685584.0QXUdSHDpG@wuerfel> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5685584.0QXUdSHDpG@wuerfel> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2584 Lines: 83 On Wed, May 13, 2015 at 10:02:49PM +0200, Arnd Bergmann wrote: > On Wednesday 13 May 2015 12:45:21 Brian Norris wrote: > > I could still avoid one pointer chase and one extra memory allocation by > > embedding 'struct brcmnand_soc' in a 'struct bcm63138_nand_soc'. e.g.: > > > > struct bcm63138_nand_soc { > > void __iomem *base; > > struct brcmnand_soc soc; > > }; > > > > static bool bcm63138_nand_intc_ack(struct brcmnand_soc *soc) > > { > > struct bcm63138_nand_soc *priv; > > priv = container_of(soc, struct bcm63138_nand_soc, soc); > > > > ... > > } > > > > static int bcm63138_nand_probe(...) > > { > > struct bcm63138_nand_soc *priv; > > > > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > ... > > return brcmnand_probe(pdev, &priv->soc); > > } > > That would make struct brcmnand_soc an empty structure, right? No, it still contains the function pointers for our callbacks, which is the entire point. I guess it's more of a 'nand_soc_ops' structure than a 'nand_soc' pointer now though. > I think that's fine though, at least it avoids passing void pointers > and it avoids one of the two allocations you do. > > There is another variation of this model, which some drivers use: > > static int bcm63138_nand_probe(...) > { > struct bcm63138_nand_soc *priv; > struct brcmnand_controller *controller; > > controller = brcmnand_controller_alloc(dev, sizeof (*priv)); > > priv = brcmnand_controller_priv(controller); > > ... > > return brcmnand_register(controller); > } > > struct brcmnand_controller *brcmnand_controller_alloc(struct device *pdev, size_t extra) > { > struct brcmnand_controller *p = dev_kzalloc(dev, sizeof(*p) + extra); > > ... > > return p; > } > > void *brcmnand_controller_priv(brcmnand_controller *p) > { > /* extra data follows at the next byte after the controller structure */ > return p + 1; > } Ah, so this allows the driver to still be agnostic about the contents of brcmnand_controller. > Some subsystem maintainers prefer this model over the other one, up to you. I'll probably stick with mine. But thanks for the suggestion. I'll keep it in mind. I was actually thinking of imitating this model for other larger portions of drivers/mtd/nand/, to aid in bounding what drivers are expected to do vs. allowing the core subsystem to handle things. Brian -- 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/