Re: [PATCH 2/3] crypto: brcm: Add Broadcom SPU driver
Mark,
Thanks for the comments. Replies below.
Rob
On 12/6/2016 9:18 AM, Mark Rutland wrote:
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 = _ns2_types,
+ },
+ {
+ .compatible = "brcm,spum-nsp-crypto",
+ .data = _nsp_types,
+ },
+ {
+ .compatible = "brcm,spu2-crypto",
+ .data = _types,
+ },
+ {
+ .compatible = "brcm,spu2-v2-crypto",
+ .data = _v2_types,
+ },
These last two weren't in the binding document.
yes, I'll add them.
+ { /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, bcm_spu_dt_ids);
+
+static int spu_dt_read(struct platform_device *pdev)
+{
+ struct device *dev = >dev;
+ struct spu_hw *spu = _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?
You are correct. This is unnecessary. I will remove.
+ /* 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.
Ok, will remove.
+ 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(>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?
As I said in the previous email, I want one instance of the driver to
register crypto algos once with the crypto API and to distribute crypto
requests among all available SPU hw blocks.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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 = _ns2_types,
> + },
> + {
> + .compatible = "brcm,spum-nsp-crypto",
> + .data = _nsp_types,
> + },
> + {
> + .compatible = "brcm,spu2-crypto",
> + .data = _types,
> + },
> + {
> + .compatible = "brcm,spu2-v2-crypto",
> + .data = _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 = >dev;
> + struct spu_hw *spu = _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(>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.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] crypto: brcm: Add Broadcom SPU driver
Hi Rob, [auto build test ERROR on cryptodev/master] [also build test ERROR on v4.9-rc7 next-20161201] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Rob-Rice/crypto-brcm-DT-documentation-for-Broadcom-SPU-driver/20161202-010038 base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master config: arm64-defconfig (attached as .config) compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm64 All errors (new ones prefixed by >>): >> ERROR: "des_ekey" [drivers/crypto/bcm/bcm_crypto_spu.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
