Re: [PATCH v4 08/11] mtd: brcmnand: add BCM63138 support

From: Arnd Bergmann
Date: Wed May 13 2015 - 06:49:15 EST


On Tuesday 12 May 2015 17:53:41 Brian Norris wrote:
> +static bool bcm63138_nand_intc_ack(struct brcmnand_soc *soc)
> +{
> + struct bcm63138_nand_soc_priv *priv = soc->priv;
> + void __iomem *mmio = priv->base + BCM63138_NAND_INT_STATUS;
> + u32 val = brcmnand_readl(mmio);
> +
> + if (val & BCM63138_CTLRDY) {
> + brcmnand_writel(val & ~BCM63138_CTLRDY, mmio);
> + return true;
> + }
> +
> + return false;
> +}
...
> +static int bcm63138_nand_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct bcm63138_nand_soc_priv *priv;
> + struct brcmnand_soc *soc;
> + struct resource *res;
> +
> + soc = devm_kzalloc(dev, sizeof(*soc), GFP_KERNEL);
> + if (!soc)
> + return -ENOMEM;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +

This is a slightly unconventional method of doing the abstraction.
For consistency with a lot of other drivers, I'd do it like this:

struct bcm63138_controller {
void __iomem *base;
brcmnand_controller parent;
};

static bool bcm63138_nand_intc_ack(struct brcmnand_controller *parent)
{
struct bcm63138_controller *controller;
controller = container_of(parent, struct brcmnand_controller, parent);

...
}

static int bcm63138_nand_probe(...)
{
struct bcm63138_controller *controller;

controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
...
return brcmnand_probe(pdev, &controller->parent);
}

This also simplifies the probe() functions and means less pointer chasing.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/