linux-kernel - Re: [PATCH 2/3] crypto: brcm: Add Broadcom SPU driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161206141826.GC24177@leverpostej>
Date:   Tue, 6 Dec 2016 14:18:26 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Rob Rice <rob.rice@...adcom.com>
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>, linux-crypto@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Ray Jui <rjui@...adcom.com>,
        Scott Branden <sbranden@...adcom.com>,
        Jon Mason <jonmason@...adcom.com>,
        bcm-kernel-feedback-list@...adcom.com,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        linux-arm-kernel@...ts.infradead.org,
        Steve Lin <steven.lin1@...adcom.com>
Subject: Re: [PATCH 2/3] crypto: brcm: Add Broadcom SPU driver

On Wed, Nov 30, 2016 at 03:07:32PM -0500, Rob Rice wrote:
> +static const struct of_device_id bcm_spu_dt_ids[] = {
> +	{
> +		.compatible = "brcm,spum-crypto",
> +		.data = &spum_ns2_types,
> +	},
> +	{
> +		.compatible = "brcm,spum-nsp-crypto",
> +		.data = &spum_nsp_types,
> +	},
> +	{
> +		.compatible = "brcm,spu2-crypto",
> +		.data = &spu2_types,
> +	},
> +	{
> +		.compatible = "brcm,spu2-v2-crypto",
> +		.data = &spu2_v2_types,
> +	},

These last two weren't in the binding document.

> +	{ /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, bcm_spu_dt_ids);
> +
> +static int spu_dt_read(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct spu_hw *spu = &iproc_priv.spu;
> +	struct device_node *dn = pdev->dev.of_node;
> +	struct resource *spu_ctrl_regs;
> +	const struct of_device_id *match;
> +	struct spu_type_subtype *matched_spu_type;
> +	void __iomem *spu_reg_vbase[MAX_SPUS];
> +	int i;
> +	int err;
> +
> +	if (!of_device_is_available(dn)) {
> +		dev_crit(dev, "SPU device not available");
> +		return -ENODEV;
> +	}

How can this happen?

> +	/* Count number of mailbox channels */
> +	spu->num_chan = of_count_phandle_with_args(dn, "mboxes", "#mbox-cells");
> +	dev_dbg(dev, "Device has %d SPU channels", spu->num_chan);
> +
> +	match = of_match_device(of_match_ptr(bcm_spu_dt_ids), dev);
> +	matched_spu_type = (struct spu_type_subtype *)match->data;

This cast usn't necessary.

> +	spu->spu_type = matched_spu_type->type;
> +	spu->spu_subtype = matched_spu_type->subtype;
> +
> +	/* Read registers and count number of SPUs */
> +	i = 0;
> +	while ((i < MAX_SPUS) && ((spu_ctrl_regs =
> +		platform_get_resource(pdev, IORESOURCE_MEM, i)) != NULL)) {
> +		dev_dbg(dev,
> +			"SPU %d control register region res.start = %#x, res.end = %#x",
> +			i,
> +			(unsigned int)spu_ctrl_regs->start,
> +			(unsigned int)spu_ctrl_regs->end);
> +
> +		spu_reg_vbase[i] = devm_ioremap_resource(dev, spu_ctrl_regs);
> +		if (IS_ERR(spu_reg_vbase[i])) {
> +			err = PTR_ERR(spu_reg_vbase[i]);
> +			dev_err(&pdev->dev, "Failed to map registers: %d\n",
> +				err);
> +			spu_reg_vbase[i] = NULL;
> +			return err;
> +		}
> +		i++;
> +	}

These *really* sound like independent devices. There are no shared
registers, and each has its own mbox.

Why do we group them like this?

Thanks,
Mark.