Message ID | 1430857666-18877-2-git-send-email-eric@anholt.net (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Den 05.05.2015 22:27, skrev Eric Anholt: > From: Lubomir Rintel <lkundrak@v3.sk> > > This mailbox driver provides a single mailbox channel to write 32-bit > values to the VPU and get a 32-bit response. The Raspberry Pi > firmware uses this mailbox channel to implement firmware calls, while > Roku 2 (despite being derived from the same firmware tree) doesn't. > > The driver was originally submitted by Lubomir, based on the > out-of-tree 2708 mailbox driver. Eric Anholt fixed it up for > upstreaming, with the major functional change being that it now has no > notion of multiple channels (since that is a firmware-dependent > concept) and instead the raspberrypi-firmware driver will do that > bit-twiddling in its own messages. ... > +static struct platform_driver bcm2835_mbox_driver = { > + .driver = { > + .name = "bcm2835-mbox", > + .owner = THIS_MODULE, > + .of_match_table = bcm2835_mbox_of_match, > + }, > + .probe = bcm2835_mbox_probe, > + .remove = bcm2835_mbox_remove, > +}; > +module_platform_driver(bcm2835_mbox_driver); I have tested this driver and the firmware driver booting directly from the VideoCore bootloader (no uboot). The mailbox driver loads too late to turn on USB power: [ 0.754258] platform soc:firmware: Driver raspberrypi-firmware requests probe deferral [ 2.041136] dwc2 20980000.usb: 256 invalid for host_nperio_tx_fifo_size. Check HW configuration. [ 2.050058] dwc2 20980000.usb: 512 invalid for host_perio_tx_fifo_size. Check HW configuration. [ 4.140894] dwc2 20980000.usb: dwc2_core_reset() HANG! Soft Reset GRSTCTL=80000001 [ 4.148583] dwc2 20980000.usb: dwc2_core_init(): Reset failed, aborting [ 4.155281] dwc2 20980000.usb: dwc2_hcd_init() FAILED, returning -16 [ 4.161720] dwc2: probe of 20980000.usb failed with error -16 [ 4.272027] bcm2835-mbox 2000b880.mailbox: mailbox enabled When I move mailbox module init to arch_initcall, USB power is turned on. [ 0.548481] bcm2835-mbox 2000b880.mailbox: mailbox enabled [ 1.733669] raspberrypi_firmware_set_power *** I have added this [ 2.242699] USB: Power-on latency exceeded, new value 509026000 ns [ 3.109748] dwc2 20980000.usb: DWC OTG Controller [ 3.114690] dwc2 20980000.usb: new USB bus registered, assigned bus number 1 [ 3.121890] dwc2 20980000.usb: irq 33, io mem 0x00000000 This silences the warning: struct raspberrypi_power_domain raspberrypi_power_domain_usb = { .base = { .power_on_latency_ns = 600000000, Noralf.
Noralf Trønnes <noralf@tronnes.org> writes: > Den 05.05.2015 22:27, skrev Eric Anholt: >> From: Lubomir Rintel <lkundrak@v3.sk> >> >> This mailbox driver provides a single mailbox channel to write 32-bit >> values to the VPU and get a 32-bit response. The Raspberry Pi >> firmware uses this mailbox channel to implement firmware calls, while >> Roku 2 (despite being derived from the same firmware tree) doesn't. >> >> The driver was originally submitted by Lubomir, based on the >> out-of-tree 2708 mailbox driver. Eric Anholt fixed it up for >> upstreaming, with the major functional change being that it now has no >> notion of multiple channels (since that is a firmware-dependent >> concept) and instead the raspberrypi-firmware driver will do that >> bit-twiddling in its own messages. > ... >> +static struct platform_driver bcm2835_mbox_driver = { >> + .driver = { >> + .name = "bcm2835-mbox", >> + .owner = THIS_MODULE, >> + .of_match_table = bcm2835_mbox_of_match, >> + }, >> + .probe = bcm2835_mbox_probe, >> + .remove = bcm2835_mbox_remove, >> +}; >> +module_platform_driver(bcm2835_mbox_driver); > > I have tested this driver and the firmware driver booting directly > from the VideoCore bootloader (no uboot). > The mailbox driver loads too late to turn on USB power: Yeah, I have a patch on my branches that returns -EPROBE_DEFER when trying to get a power domain and not finding the provider. It was rejected by the maintainers in favor of a proposed solution whose description I didn't quite follow. > This silences the warning: > struct raspberrypi_power_domain raspberrypi_power_domain_usb = { > .base = { > .power_on_latency_ns = 600000000, Oh, nice. Thanks!
On Thursday 07 May 2015, 12:54:20 wrote Eric Anholt: > Noralf Trønnes <noralf@tronnes.org> writes: > > > Den 05.05.2015 22:27, skrev Eric Anholt: > >> From: Lubomir Rintel <lkundrak@v3.sk> > >> > >> This mailbox driver provides a single mailbox channel to write 32-bit > >> values to the VPU and get a 32-bit response. The Raspberry Pi > >> firmware uses this mailbox channel to implement firmware calls, while > >> Roku 2 (despite being derived from the same firmware tree) doesn't. > >> > >> The driver was originally submitted by Lubomir, based on the > >> out-of-tree 2708 mailbox driver. Eric Anholt fixed it up for > >> upstreaming, with the major functional change being that it now has no > >> notion of multiple channels (since that is a firmware-dependent > >> concept) and instead the raspberrypi-firmware driver will do that > >> bit-twiddling in its own messages. > > ... > >> +static struct platform_driver bcm2835_mbox_driver = { > >> + .driver = { > >> + .name = "bcm2835-mbox", > >> + .owner = THIS_MODULE, > >> + .of_match_table = bcm2835_mbox_of_match, > >> + }, > >> + .probe = bcm2835_mbox_probe, > >> + .remove = bcm2835_mbox_remove, > >> +}; > >> +module_platform_driver(bcm2835_mbox_driver); > > > > I have tested this driver and the firmware driver booting directly > > from the VideoCore bootloader (no uboot). > > The mailbox driver loads too late to turn on USB power: > > Yeah, I have a patch on my branches that returns -EPROBE_DEFER when > trying to get a power domain and not finding the provider. It was > rejected by the maintainers in favor of a proposed solution whose > description I didn't quite follow. Do you have a link for this thread? > > This silences the warning: > > struct raspberrypi_power_domain raspberrypi_power_domain_usb = { > > .base = { > > .power_on_latency_ns = 600000000, > > Oh, nice. Thanks! Well, Using a timeout for dependencies seems odd to me. Regards, Alexander
Alexander Stein <alexanders83@web.de> writes: > On Thursday 07 May 2015, 12:54:20 wrote Eric Anholt: >> Noralf Trønnes <noralf@tronnes.org> writes: >> >> > Den 05.05.2015 22:27, skrev Eric Anholt: >> >> From: Lubomir Rintel <lkundrak@v3.sk> >> >> >> >> This mailbox driver provides a single mailbox channel to write 32-bit >> >> values to the VPU and get a 32-bit response. The Raspberry Pi >> >> firmware uses this mailbox channel to implement firmware calls, while >> >> Roku 2 (despite being derived from the same firmware tree) doesn't. >> >> >> >> The driver was originally submitted by Lubomir, based on the >> >> out-of-tree 2708 mailbox driver. Eric Anholt fixed it up for >> >> upstreaming, with the major functional change being that it now has no >> >> notion of multiple channels (since that is a firmware-dependent >> >> concept) and instead the raspberrypi-firmware driver will do that >> >> bit-twiddling in its own messages. >> > ... >> >> +static struct platform_driver bcm2835_mbox_driver = { >> >> + .driver = { >> >> + .name = "bcm2835-mbox", >> >> + .owner = THIS_MODULE, >> >> + .of_match_table = bcm2835_mbox_of_match, >> >> + }, >> >> + .probe = bcm2835_mbox_probe, >> >> + .remove = bcm2835_mbox_remove, >> >> +}; >> >> +module_platform_driver(bcm2835_mbox_driver); >> > >> > I have tested this driver and the firmware driver booting directly >> > from the VideoCore bootloader (no uboot). >> > The mailbox driver loads too late to turn on USB power: >> >> Yeah, I have a patch on my branches that returns -EPROBE_DEFER when >> trying to get a power domain and not finding the provider. It was >> rejected by the maintainers in favor of a proposed solution whose >> description I didn't quite follow. > > Do you have a link for this thread? https://lkml.org/lkml/2015/3/11/483
Den 08.05.2015 10:33, skrev Alexander Stein: > On Thursday 07 May 2015, 12:54:20 wrote Eric Anholt: >> Noralf Trønnes <noralf@tronnes.org> writes: >> >>> Den 05.05.2015 22:27, skrev Eric Anholt: >>>> From: Lubomir Rintel <lkundrak@v3.sk> >>>> >>>> This mailbox driver provides a single mailbox channel to write 32-bit >>>> values to the VPU and get a 32-bit response. The Raspberry Pi >>>> firmware uses this mailbox channel to implement firmware calls, while >>>> Roku 2 (despite being derived from the same firmware tree) doesn't. >>>> >>>> The driver was originally submitted by Lubomir, based on the >>>> out-of-tree 2708 mailbox driver. Eric Anholt fixed it up for >>>> upstreaming, with the major functional change being that it now has no >>>> notion of multiple channels (since that is a firmware-dependent >>>> concept) and instead the raspberrypi-firmware driver will do that >>>> bit-twiddling in its own messages. >>> ... >>>> +static struct platform_driver bcm2835_mbox_driver = { >>>> + .driver = { >>>> + .name = "bcm2835-mbox", >>>> + .owner = THIS_MODULE, >>>> + .of_match_table = bcm2835_mbox_of_match, >>>> + }, >>>> + .probe = bcm2835_mbox_probe, >>>> + .remove = bcm2835_mbox_remove, >>>> +}; >>>> +module_platform_driver(bcm2835_mbox_driver); >>> I have tested this driver and the firmware driver booting directly >>> from the VideoCore bootloader (no uboot). >>> The mailbox driver loads too late to turn on USB power: >> Yeah, I have a patch on my branches that returns -EPROBE_DEFER when >> trying to get a power domain and not finding the provider. It was >> rejected by the maintainers in favor of a proposed solution whose >> description I didn't quite follow. > Do you have a link for this thread? > >>> This silences the warning: >>> struct raspberrypi_power_domain raspberrypi_power_domain_usb = { >>> .base = { >>> .power_on_latency_ns = 600000000, >> Oh, nice. Thanks! > Well, Using a timeout for dependencies seems odd to me. I can only find one place where power_on_latency_ns is set, arch/arm/mach-imx/gpc.c: static struct pu_domain imx6q_pu_domain = { .base = { .name = "PU", .power_off = imx6q_pm_pu_power_off, .power_on = imx6q_pm_pu_power_on, .power_off_latency_ns = 25000, .power_on_latency_ns = 2000000, }, }; power_on_latency_ns is not set by the core, so it defaults to zero. So in the default case, genpd_power_on() will always issue a warning on the first power on. I would say that power_on_latency_ns is a characteristic of the power domain, like enable_time for regulators. Noralf.
On 05/08/2015 01:19 PM, Eric Anholt wrote: > Alexander Stein <alexanders83@web.de> writes: > >> On Thursday 07 May 2015, 12:54:20 wrote Eric Anholt: >>> Noralf Trønnes <noralf@tronnes.org> writes: >>> >>>> Den 05.05.2015 22:27, skrev Eric Anholt: >>>>> From: Lubomir Rintel <lkundrak@v3.sk> >>>>> >>>>> This mailbox driver provides a single mailbox channel to >>>>> write 32-bit values to the VPU and get a 32-bit response. >>>>> The Raspberry Pi firmware uses this mailbox channel to >>>>> implement firmware calls, while Roku 2 (despite being >>>>> derived from the same firmware tree) doesn't. >>>>> >>>>> The driver was originally submitted by Lubomir, based on >>>>> the out-of-tree 2708 mailbox driver. Eric Anholt fixed it >>>>> up for upstreaming, with the major functional change being >>>>> that it now has no notion of multiple channels (since that >>>>> is a firmware-dependent concept) and instead the >>>>> raspberrypi-firmware driver will do that bit-twiddling in >>>>> its own messages. >>>> ... >>>>> +static struct platform_driver bcm2835_mbox_driver = { + >>>>> .driver = { + .name = "bcm2835-mbox", + .owner = >>>>> THIS_MODULE, + .of_match_table = bcm2835_mbox_of_match, + >>>>> }, + .probe = bcm2835_mbox_probe, + .remove = >>>>> bcm2835_mbox_remove, +}; >>>>> +module_platform_driver(bcm2835_mbox_driver); >>>> >>>> I have tested this driver and the firmware driver booting >>>> directly from the VideoCore bootloader (no uboot). The >>>> mailbox driver loads too late to turn on USB power: >>> >>> Yeah, I have a patch on my branches that returns -EPROBE_DEFER >>> when trying to get a power domain and not finding the provider. >>> It was rejected by the maintainers in favor of a proposed >>> solution whose description I didn't quite follow. >> >> Do you have a link for this thread? > > https://lkml.org/lkml/2015/3/11/483 That's really odd; -EPROBE_DEFER was clearly invented exactly to handle dependencies just like this. Playing with initcall levels simply isn't scalable, and in the main people are actively working not to use them for dependencies like this; they're far too implicit. While the timeout mentioned earlier in the thread might work (I didn't really look at the details), again it's far too indirect/accidental to be a good solution.
Den 05.05.2015 22:27, skrev Eric Anholt: > From: Lubomir Rintel <lkundrak@v3.sk> > > This mailbox driver provides a single mailbox channel to write 32-bit > values to the VPU and get a 32-bit response. The Raspberry Pi > firmware uses this mailbox channel to implement firmware calls, while > Roku 2 (despite being derived from the same firmware tree) doesn't. > > The driver was originally submitted by Lubomir, based on the > out-of-tree 2708 mailbox driver. Eric Anholt fixed it up for > upstreaming, with the major functional change being that it now has no > notion of multiple channels (since that is a firmware-dependent > concept) and instead the raspberrypi-firmware driver will do that > bit-twiddling in its own messages. > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> > Signed-off-by: Craig McGeachie <slapdau@yahoo.com.au> > Signed-off-by: Eric Anholt <eric@anholt.net> > Cc: Jassi Brar <jassisinghbrar@gmail.com> > Acked-by: Stephen Warren <swarren@wwwdotorg.org> > --- [...] > diff --git a/drivers/mailbox/bcm2835-mailbox.c b/drivers/mailbox/bcm2835-mailbox.c [...] > +static bool bcm2835_last_tx_done(struct mbox_chan *link) > +{ > + struct bcm2835_mbox *mbox = bcm2835_link_mbox(link); > + bool ret; > + > + spin_lock(&mbox->lock); > + ret = !(readl(mbox->regs + MAIL0_STA) & ARM_MS_FULL); I don't know how the mailbox framework uses this function, but MAIL0_STA might be wrong here. Phil Elwell detected a bug in the downstream mailbox driver in the mbox_write function where it waits for space to be available in the FIFO. His observation: With the VC reader blocked and the ARM writing, MAIL0_STA reads empty permanently while MAIL1_STA goes from empty (0x40000000) to non-empty (0x00000001-0x00000007) to full (0x80000008). See: https://github.com/raspberrypi/linux/commit/3018d8a0996ad2340ba1b3f473f705ef285b01b5
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 84b0a2d..ab574da 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -60,4 +60,12 @@ config ALTERA_MBOX An implementation of the Altera Mailbox soft core. It is used to send message between processors. Say Y here if you want to use the Altera mailbox support. + +config BCM2835_MBOX + tristate "BCM2835 Mailbox" + depends on ARCH_BCM2835 + help + An implementation of the BCM2385 Mailbox. It is used to invoke + the services of the Videocore. Say Y here if you want to use the + BCM2835 Mailbox. endif diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index b18201e..8e6d822 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -11,3 +11,5 @@ obj-$(CONFIG_OMAP2PLUS_MBOX) += omap-mailbox.o obj-$(CONFIG_PCC) += pcc.o obj-$(CONFIG_ALTERA_MBOX) += mailbox-altera.o + +obj-$(CONFIG_BCM2835_MBOX) += bcm2835-mailbox.o diff --git a/drivers/mailbox/bcm2835-mailbox.c b/drivers/mailbox/bcm2835-mailbox.c new file mode 100644 index 0000000..16c3453 --- /dev/null +++ b/drivers/mailbox/bcm2835-mailbox.c @@ -0,0 +1,217 @@ +/* + * Copyright (C) 2010,2015 Broadcom + * Copyright (C) 2013-2014 Lubomir Rintel + * Copyright (C) 2013 Craig McGeachie + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This device provides a mechanism for writing to the mailboxes, + * that are shared between the ARM and the VideoCore processor + * + * Parts of the driver are based on: + * - arch/arm/mach-bcm2708/vcio.c file written by Gray Girling that was + * obtained from branch "rpi-3.6.y" of git://github.com/raspberrypi/ + * linux.git + * - drivers/mailbox/bcm2835-ipc.c by Lubomir Rintel at + * https://github.com/hackerspace/rpi-linux/blob/lr-raspberry-pi/drivers/ + * mailbox/bcm2835-ipc.c + * - documentation available on the following web site: + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface + */ + +#include <linux/device.h> +#include <linux/dma-mapping.h> +#include <linux/err.h> +#include <linux/interrupt.h> +#include <linux/irq.h> +#include <linux/kernel.h> +#include <linux/mailbox_controller.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/platform_device.h> +#include <linux/spinlock.h> + +/* Mailboxes */ +#define ARM_0_MAIL0 0x00 +#define ARM_0_MAIL1 0x20 + +/* + * Mailbox registers. We basically only support mailbox 0 & 1. We + * deliver to the VC in mailbox 1, it delivers to us in mailbox 0. See + * BCM2835-ARM-Peripherals.pdf section 1.3 for an explanation about + * the placement of memory barriers. + */ +#define MAIL0_RD (ARM_0_MAIL0 + 0x00) +#define MAIL0_POL (ARM_0_MAIL0 + 0x10) +#define MAIL0_STA (ARM_0_MAIL0 + 0x18) +#define MAIL0_CNF (ARM_0_MAIL0 + 0x1C) +#define MAIL1_WRT (ARM_0_MAIL1 + 0x00) + +/* Status register: FIFO state. */ +#define ARM_MS_FULL BIT(31) +#define ARM_MS_EMPTY BIT(30) + +/* Configuration register: Enable interrupts. */ +#define ARM_MC_IHAVEDATAIRQEN BIT(0) + +struct bcm2835_mbox { + void __iomem *regs; + spinlock_t lock; + struct mbox_controller controller; +}; + +static struct bcm2835_mbox *bcm2835_link_mbox(struct mbox_chan *link) +{ + return container_of(link->mbox, struct bcm2835_mbox, controller); +} + +static irqreturn_t bcm2835_mbox_irq(int irq, void *dev_id) +{ + struct bcm2835_mbox *mbox = dev_id; + struct device *dev = mbox->controller.dev; + struct mbox_chan *link = &mbox->controller.chans[0]; + + while (!(readl(mbox->regs + MAIL0_STA) & ARM_MS_EMPTY)) { + u32 msg = readl(mbox->regs + MAIL0_RD); + dev_dbg(dev, "Reply 0x%08X\n", msg); + mbox_chan_received_data(link, &msg); + } + return IRQ_HANDLED; +} + +static int bcm2835_send_data(struct mbox_chan *link, void *data) +{ + struct bcm2835_mbox *mbox = bcm2835_link_mbox(link); + int ret = 0; + u32 msg = *(u32 *)data; + + spin_lock(&mbox->lock); + writel(msg, mbox->regs + MAIL1_WRT); + dev_dbg(mbox->controller.dev, "Request 0x%08X\n", msg); + spin_unlock(&mbox->lock); + return ret; +} + +static int bcm2835_startup(struct mbox_chan *link) +{ + struct bcm2835_mbox *mbox = bcm2835_link_mbox(link); + + /* Enable the interrupt on data reception */ + writel(ARM_MC_IHAVEDATAIRQEN, mbox->regs + MAIL0_CNF); + + return 0; +} + +static void bcm2835_shutdown(struct mbox_chan *link) +{ + struct bcm2835_mbox *mbox = bcm2835_link_mbox(link); + + writel(0, mbox->regs + MAIL0_CNF); +} + +static bool bcm2835_last_tx_done(struct mbox_chan *link) +{ + struct bcm2835_mbox *mbox = bcm2835_link_mbox(link); + bool ret; + + spin_lock(&mbox->lock); + ret = !(readl(mbox->regs + MAIL0_STA) & ARM_MS_FULL); + spin_unlock(&mbox->lock); + return ret; +} + +static struct mbox_chan_ops bcm2835_mbox_chan_ops = { + .send_data = bcm2835_send_data, + .startup = bcm2835_startup, + .shutdown = bcm2835_shutdown, + .last_tx_done = bcm2835_last_tx_done +}; + +static struct mbox_chan *bcm2835_mbox_index_xlate(struct mbox_controller *mbox, + const struct of_phandle_args *sp) +{ + if (sp->args_count != 0) + return NULL; + + return &mbox->chans[0]; +} + +static int bcm2835_mbox_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + int ret = 0; + struct resource *iomem; + struct bcm2835_mbox *mbox; + + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL); + if (mbox == NULL) + return -ENOMEM; + spin_lock_init(&mbox->lock); + + ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0), + bcm2835_mbox_irq, 0, dev_name(dev), mbox); + if (ret) { + dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n", + ret); + return -ENODEV; + } + + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + mbox->regs = devm_ioremap_resource(&pdev->dev, iomem); + if (IS_ERR(mbox->regs)) { + ret = PTR_ERR(mbox->regs); + dev_err(&pdev->dev, "Failed to remap mailbox regs: %d\n", ret); + return ret; + } + + mbox->controller.txdone_poll = true; + mbox->controller.txpoll_period = 5; + mbox->controller.ops = &bcm2835_mbox_chan_ops; + mbox->controller.of_xlate = &bcm2835_mbox_index_xlate; + mbox->controller.dev = dev; + mbox->controller.num_chans = 1; + mbox->controller.chans = devm_kzalloc(dev, + sizeof(*mbox->controller.chans), GFP_KERNEL); + if (!mbox->controller.chans) + return -ENOMEM; + + ret = mbox_controller_register(&mbox->controller); + if (ret) + return ret; + + platform_set_drvdata(pdev, mbox); + dev_info(dev, "mailbox enabled\n"); + + return ret; +} + +static int bcm2835_mbox_remove(struct platform_device *pdev) +{ + struct bcm2835_mbox *mbox = platform_get_drvdata(pdev); + mbox_controller_unregister(&mbox->controller); + return 0; +} + +static const struct of_device_id bcm2835_mbox_of_match[] = { + { .compatible = "brcm,bcm2835-mbox", }, + {}, +}; +MODULE_DEVICE_TABLE(of, bcm2835_mbox_of_match); + +static struct platform_driver bcm2835_mbox_driver = { + .driver = { + .name = "bcm2835-mbox", + .owner = THIS_MODULE, + .of_match_table = bcm2835_mbox_of_match, + }, + .probe = bcm2835_mbox_probe, + .remove = bcm2835_mbox_remove, +}; +module_platform_driver(bcm2835_mbox_driver); + +MODULE_AUTHOR("Lubomir Rintel <lkundrak@v3.sk>"); +MODULE_DESCRIPTION("BCM2835 mailbox IPC driver"); +MODULE_LICENSE("GPL v2");