Re: [PATCH v2] drivers/mailbox: Add Aspeed mailbox driver

From: Joel Stanley
Date: Wed Feb 08 2017 - 23:41:26 EST


On Wed, Feb 8, 2017 at 10:06 AM, Cyril Bur <cyrilbur@xxxxxxxxx> wrote:
> This provides access to the mbox registers on the ast2400 and ast2500
> SoCs.
>
> This driver allows arbitrary reads and writes to the 16 data registers as
> the other end may have configured the mbox hardware to provide an
> interrupt when a specific register gets written to.
>
> Signed-off-by: Cyril Bur <cyrilbur@xxxxxxxxx>
> ---
> V2:
> s/ASpeed/Aspeed/
> Reword Kconfig options
> Use tristate for config symbol
>
> drivers/mailbox/Kconfig | 8 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/aspeed-mbox.c | 334 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 344 insertions(+)
> create mode 100644 drivers/mailbox/aspeed-mbox.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index ceff415f201c..e24044d5c219 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -152,4 +152,12 @@ config BCM_PDC_MBOX
> Mailbox implementation for the Broadcom PDC ring manager,
> which provides access to various offload engines on Broadcom
> SoCs. Say Y here if you want to use the Broadcom PDC.
> +
> +config ASPEED_LPC_MBOX
> + depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
> + tristate "Aspeed LPC Mailbox Controller"
> + ---help---
> + Provides a driver for the MBOX registers found on Aspeed SOCs
> + (AST2400 and AST2500). This driver provides a device for aspeed
> + mbox registers

You repeat yourself.

Mention that this driver is for the the BMC side.

> diff --git a/drivers/mailbox/aspeed-mbox.c b/drivers/mailbox/aspeed-mbox.c

> +static int aspeed_mbox_probe(struct platform_device *pdev)
> +{
> + struct aspeed_mbox *mbox;
> + struct device *dev;
> + int rc;
> +
> + dev = &pdev->dev;
> +
> + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&pdev->dev, mbox);
> +
> + rc = of_property_read_u32(dev->of_node, "reg", &mbox->base);
> + if (rc) {
> + dev_err(dev, "Couldn't read reg device-tree property\n");
> + return rc;
> + }

I think we want to use of_address_to_resource here.

> +
> +static const struct of_device_id aspeed_mbox_match[] = {
> + { .compatible = "aspeed,ast2400-mbox" },
> + { .compatible = "aspeed,ast2500-mbox" },
> + { },

I didn't see the bindings in this series. Have they already been accepted?

Cheers,

Joel