diff mbox series

[v6,2/4] firmware: raspberrypi: Introduce vl805 init routine

Message ID 20200324182812.20420-3-nsaenzjulienne@suse.de (mailing list archive)
State Superseded, archived
Headers show
Series USB: pci-quirks: Add Raspberry Pi 4 quirk | expand

Commit Message

Nicolas Saenz Julienne March 24, 2020, 6:28 p.m. UTC 
On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
loaded directly from an EEPROM or, if not present, by the SoC's
VideCore. The function informs VideCore that VL805 was just reset, or
requests for a probe defer.

Based on Tim Gover's downstream implementation.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

---
Changes since v4:
 - Inline function definition when RASPBERRYPI_FIRMWARE is not defined

Changes since v1:
 - Move include into .c file and add forward declaration to .h

 drivers/firmware/raspberrypi.c             | 38 ++++++++++++++++++++++
 include/soc/bcm2835/raspberrypi-firmware.h |  7 ++++
 2 files changed, 45 insertions(+)

Comments

Bjorn Helgaas April 1, 2020, 8:37 p.m. UTC | #1 
On Tue, Mar 24, 2020 at 07:28:10PM +0100, Nicolas Saenz Julienne wrote:
> On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
> loaded directly from an EEPROM or, if not present, by the SoC's
> VideCore. The function informs VideCore that VL805 was just reset, or
> requests for a probe defer.

Cover letter mentions both "VideCore" and "VideoCore".  I dunno which
is correct, but between the commit log and the comment, this patch
mentions "VideCore" four times.

> Based on Tim Gover's downstream implementation.

Maybe a URL?

> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> ---
> Changes since v4:
>  - Inline function definition when RASPBERRYPI_FIRMWARE is not defined
> 
> Changes since v1:
>  - Move include into .c file and add forward declaration to .h
> 
>  drivers/firmware/raspberrypi.c             | 38 ++++++++++++++++++++++
>  include/soc/bcm2835/raspberrypi-firmware.h |  7 ++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> index da26a584dca0..cbb495aff6a0 100644
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -12,6 +12,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/pci.h>
>  #include <soc/bcm2835/raspberrypi-firmware.h>
>  
>  #define MBOX_MSG(chan, data28)		(((data28) & ~0xf) | ((chan) & 0xf))
> @@ -286,6 +287,43 @@ struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node)
>  }
>  EXPORT_SYMBOL_GPL(rpi_firmware_get);
>  
> +/*
> + * On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
> + * loaded directly from an EEPROM or, if not present, by the SoC's VideCore.
> + * Inform VideCore that VL805 was just reset, or defer xhci's probe if not yet
> + * joinable trough the mailbox interface.

s/trough/through/

I don't see anything in this patch that looks like a mailbox
interface, but maybe that's just because I don't know anything about
Raspberry Pi.

> + */
> +int rpi_firmware_init_vl805(struct pci_dev *pdev)
> +{
> +	struct device_node *fw_np;
> +	struct rpi_firmware *fw;
> +	u32 dev_addr;
> +	int ret;
> +
> +	fw_np = of_find_compatible_node(NULL, NULL,
> +					"raspberrypi,bcm2835-firmware");
> +	if (!fw_np)
> +		return 0;
> +
> +	fw = rpi_firmware_get(fw_np);
> +	of_node_put(fw_np);
> +	if (!fw)
> +		return -EPROBE_DEFER;
> +
> +	dev_addr = pdev->bus->number << 20 | PCI_SLOT(pdev->devfn) << 15 |
> +		   PCI_FUNC(pdev->devfn) << 12;
> +
> +	ret = rpi_firmware_property(fw, RPI_FIRMWARE_NOTIFY_XHCI_RESET,
> +				    &dev_addr, sizeof(dev_addr));
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(&pdev->dev, "loaded Raspberry Pi's VL805 firmware\n");
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rpi_firmware_init_vl805);
> +
>  static const struct of_device_id rpi_firmware_of_match[] = {
>  	{ .compatible = "raspberrypi,bcm2835-firmware", },
>  	{},
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
> index cc9cdbc66403..3025aca3c358 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -10,6 +10,7 @@
>  #include <linux/of_device.h>
>  
>  struct rpi_firmware;
> +struct pci_dev;
>  
>  enum rpi_firmware_property_status {
>  	RPI_FIRMWARE_STATUS_REQUEST = 0,
> @@ -141,6 +142,7 @@ int rpi_firmware_property(struct rpi_firmware *fw,
>  int rpi_firmware_property_list(struct rpi_firmware *fw,
>  			       void *data, size_t tag_size);
>  struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
> +int rpi_firmware_init_vl805(struct pci_dev *pdev);
>  #else
>  static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag,
>  					void *data, size_t len)
> @@ -158,6 +160,11 @@ static inline struct rpi_firmware *rpi_firmware_get(struct device_node *firmware
>  {
>  	return NULL;
>  }
> +
> +static inline int rpi_firmware_init_vl805(struct pci_dev *pdev)
> +{
> +	return 0;
> +}
>  #endif
>  
>  #endif /* __SOC_RASPBERRY_FIRMWARE_H__ */
> -- 
> 2.25.1
>
Nicolas Saenz Julienne April 2, 2020, 11:32 a.m. UTC | #2 
Hi Bjorn,
thanks for taking time with this.

On Wed, 2020-04-01 at 15:37 -0500, Bjorn Helgaas wrote:
> On Tue, Mar 24, 2020 at 07:28:10PM +0100, Nicolas Saenz Julienne wrote:
> > On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
> > loaded directly from an EEPROM or, if not present, by the SoC's
> > VideCore. The function informs VideCore that VL805 was just reset, or
> > requests for a probe defer.
> 
> Cover letter mentions both "VideCore" and "VideoCore".  I dunno which
> is correct, but between the commit log and the comment, this patch
> mentions "VideCore" four times.

Ouch, sorry, it's VideoCore. I have an auto complete thing, wrote it once wrong
and polluted the whole patch.

> > Based on Tim Gover's downstream implementation.
> 
> Maybe a URL?

I was under the impression that adding links in the commit log that are likely
to be short-lived was frowned upon. That said I could've added it into the
cover letter. For reference here it is:

https://github.com/raspberrypi/linux/commit/9935b4c7e360b4494b4cb6e3ce797238a1ab78bd

> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > 
> > ---
> > Changes since v4:
> >  - Inline function definition when RASPBERRYPI_FIRMWARE is not defined
> > 
> > Changes since v1:
> >  - Move include into .c file and add forward declaration to .h
> > 
> >  drivers/firmware/raspberrypi.c             | 38 ++++++++++++++++++++++
> >  include/soc/bcm2835/raspberrypi-firmware.h |  7 ++++
> >  2 files changed, 45 insertions(+)
> > 
> > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> > index da26a584dca0..cbb495aff6a0 100644
> > --- a/drivers/firmware/raspberrypi.c
> > +++ b/drivers/firmware/raspberrypi.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/slab.h>
> > +#include <linux/pci.h>
> >  #include <soc/bcm2835/raspberrypi-firmware.h>
> >  
> >  #define MBOX_MSG(chan, data28)		(((data28) & ~0xf) | ((chan) &
> > 0xf))
> > @@ -286,6 +287,43 @@ struct rpi_firmware *rpi_firmware_get(struct
> > device_node *firmware_node)
> >  }
> >  EXPORT_SYMBOL_GPL(rpi_firmware_get);
> >  
> > +/*
> > + * On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
> > + * loaded directly from an EEPROM or, if not present, by the SoC's
> > VideCore.
> > + * Inform VideCore that VL805 was just reset, or defer xhci's probe if not
> > yet
> > + * joinable trough the mailbox interface.
> 
> s/trough/through/

Noted.

> I don't see anything in this patch that looks like a mailbox
> interface, but maybe that's just because I don't know anything about
> Raspberry Pi.

There are two layers to this. The bcm2835-mailbox interface, that is generic to
all SoC users and the Raspberry Pi firmware driver, which interacts with RPi's
custom VideoCore firmware trough the bcm2835-mailbox, and provides a light
level of abstraction. It's like that to keep a clear separation between what's
a SoC feature an what is RPi specific.

So with a call to rpi_firmware_get() you're supposed to get a handle to the
shared RPi firmware structure. As long as it's ready. To pass messages down the
mailbox, you call rpi_firmware_property(), which takes care of contention,
formating and DMA issues, before passing it into the actual mailbox interface
and beyond.

> > + */
> > +int rpi_firmware_init_vl805(struct pci_dev *pdev)
> > +{
> > +	struct device_node *fw_np;
> > +	struct rpi_firmware *fw;
> > +	u32 dev_addr;
> > +	int ret;
> > +
> > +	fw_np = of_find_compatible_node(NULL, NULL,
> > +					"raspberrypi,bcm2835-firmware");
> > +	if (!fw_np)
> > +		return 0;
> > +
> > +	fw = rpi_firmware_get(fw_np);
> > +	of_node_put(fw_np);
> > +	if (!fw)
> > +		return -EPROBE_DEFER;
> > +
> > +	dev_addr = pdev->bus->number << 20 | PCI_SLOT(pdev->devfn) << 15 |
> > +		   PCI_FUNC(pdev->devfn) << 12;
> > +
> > +	ret = rpi_firmware_property(fw, RPI_FIRMWARE_NOTIFY_XHCI_RESET,
> > +				    &dev_addr, sizeof(dev_addr));
> > +	if (ret)
> > +		return ret;
> > +
> > +	dev_dbg(&pdev->dev, "loaded Raspberry Pi's VL805 firmware\n");
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(rpi_firmware_init_vl805);
> > +
> >  static const struct of_device_id rpi_firmware_of_match[] = {
> >  	{ .compatible = "raspberrypi,bcm2835-firmware", },
> >  	{},

[...]

Regards,
Nicolas
Bjorn Helgaas April 2, 2020, 7:40 p.m. UTC | #3 
On Thu, Apr 02, 2020 at 01:32:35PM +0200, Nicolas Saenz Julienne wrote:
> On Wed, 2020-04-01 at 15:37 -0500, Bjorn Helgaas wrote:
> > On Tue, Mar 24, 2020 at 07:28:10PM +0100, Nicolas Saenz Julienne wrote:
> > > On the Raspberry Pi 4, after a PCI reset, VL805's firmware may
> > > either be loaded directly from an EEPROM or, if not present, by
> > > the SoC's VideCore. The function informs VideCore that VL805 was
> > > just reset, or requests for a probe defer.

Is VL805 the XHCI USB device?  A hint here would help non-RPi experts
know how this fits into the topology.

> > > Based on Tim Gover's downstream implementation.
> >
> > Maybe a URL?
> 
> I was under the impression that adding links in the commit log that
> are likely to be short-lived was frowned upon. That said I could've
> added it into the cover letter. For reference here it is:
> 
> https://github.com/raspberrypi/linux/commit/9935b4c7e360b4494b4cb6e3ce797238a1ab78bd

I think your impression is correct.  If this was posted to a mailing
list archived on lore.kernel.org, a link to the cover letter would be
ideal.

> To pass messages down the mailbox, you call rpi_firmware_property(),
> which takes care of contention, formating and DMA issues, before
> passing it into the actual mailbox interface and beyond.

OK.  The "rpi_firmware_property" name doesn't give much of a hint that
it is sending messages.  It sounds like it might be a lookup function.
But that's an existing thing, not something you're changing here.

> > > + */
> > > +int rpi_firmware_init_vl805(struct pci_dev *pdev)
> > > +{
> > > +	struct device_node *fw_np;
> > > +	struct rpi_firmware *fw;
> > > +	u32 dev_addr;
> > > +	int ret;
> > > +
> > > +	fw_np = of_find_compatible_node(NULL, NULL,
> > > +					"raspberrypi,bcm2835-firmware");
> > > +	if (!fw_np)
> > > +		return 0;
> > > +
> > > +	fw = rpi_firmware_get(fw_np);
> > > +	of_node_put(fw_np);
> > > +	if (!fw)
> > > +		return -EPROBE_DEFER;
> > > +
> > > +	dev_addr = pdev->bus->number << 20 | PCI_SLOT(pdev->devfn) << 15 |
> > > +		   PCI_FUNC(pdev->devfn) << 12;
> > > +
> > > +	ret = rpi_firmware_property(fw, RPI_FIRMWARE_NOTIFY_XHCI_RESET,
> > > +				    &dev_addr, sizeof(dev_addr));
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	dev_dbg(&pdev->dev, "loaded Raspberry Pi's VL805 firmware\n");
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rpi_firmware_init_vl805);
> > > +
> > >  static const struct of_device_id rpi_firmware_of_match[] = {
> > >  	{ .compatible = "raspberrypi,bcm2835-firmware", },
> > >  	{},
> 
> [...]
> 
> Regards,
> Nicolas
>
Nicolas Saenz Julienne April 4, 2020, 6:56 p.m. UTC | #4 
On Thu, 2020-04-02 at 14:40 -0500, Bjorn Helgaas wrote:
> On Thu, Apr 02, 2020 at 01:32:35PM +0200, Nicolas Saenz Julienne wrote:
> > On Wed, 2020-04-01 at 15:37 -0500, Bjorn Helgaas wrote:
> > > On Tue, Mar 24, 2020 at 07:28:10PM +0100, Nicolas Saenz Julienne wrote:
> > > > On the Raspberry Pi 4, after a PCI reset, VL805's firmware may
> > > > either be loaded directly from an EEPROM or, if not present, by
> > > > the SoC's VideCore. The function informs VideCore that VL805 was
> > > > just reset, or requests for a probe defer.
> 
> Is VL805 the XHCI USB device?  A hint here would help non-RPi experts
> know how this fits into the topology.

Yes, VL805 is the XHCI USB device. I'll keep it in mind for the next series.

Regards,
Nicolas
diff mbox series

Patch

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index da26a584dca0..cbb495aff6a0 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -12,6 +12,7 @@ 
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/pci.h>
 #include <soc/bcm2835/raspberrypi-firmware.h>
 
 #define MBOX_MSG(chan, data28)		(((data28) & ~0xf) | ((chan) & 0xf))
@@ -286,6 +287,43 @@  struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node)
 }
 EXPORT_SYMBOL_GPL(rpi_firmware_get);
 
+/*
+ * On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
+ * loaded directly from an EEPROM or, if not present, by the SoC's VideCore.
+ * Inform VideCore that VL805 was just reset, or defer xhci's probe if not yet
+ * joinable trough the mailbox interface.
+ */
+int rpi_firmware_init_vl805(struct pci_dev *pdev)
+{
+	struct device_node *fw_np;
+	struct rpi_firmware *fw;
+	u32 dev_addr;
+	int ret;
+
+	fw_np = of_find_compatible_node(NULL, NULL,
+					"raspberrypi,bcm2835-firmware");
+	if (!fw_np)
+		return 0;
+
+	fw = rpi_firmware_get(fw_np);
+	of_node_put(fw_np);
+	if (!fw)
+		return -EPROBE_DEFER;
+
+	dev_addr = pdev->bus->number << 20 | PCI_SLOT(pdev->devfn) << 15 |
+		   PCI_FUNC(pdev->devfn) << 12;
+
+	ret = rpi_firmware_property(fw, RPI_FIRMWARE_NOTIFY_XHCI_RESET,
+				    &dev_addr, sizeof(dev_addr));
+	if (ret)
+		return ret;
+
+	dev_dbg(&pdev->dev, "loaded Raspberry Pi's VL805 firmware\n");
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_init_vl805);
+
 static const struct of_device_id rpi_firmware_of_match[] = {
 	{ .compatible = "raspberrypi,bcm2835-firmware", },
 	{},
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index cc9cdbc66403..3025aca3c358 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -10,6 +10,7 @@ 
 #include <linux/of_device.h>
 
 struct rpi_firmware;
+struct pci_dev;
 
 enum rpi_firmware_property_status {
 	RPI_FIRMWARE_STATUS_REQUEST = 0,
@@ -141,6 +142,7 @@  int rpi_firmware_property(struct rpi_firmware *fw,
 int rpi_firmware_property_list(struct rpi_firmware *fw,
 			       void *data, size_t tag_size);
 struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
+int rpi_firmware_init_vl805(struct pci_dev *pdev);
 #else
 static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag,
 					void *data, size_t len)
@@ -158,6 +160,11 @@  static inline struct rpi_firmware *rpi_firmware_get(struct device_node *firmware
 {
 	return NULL;
 }
+
+static inline int rpi_firmware_init_vl805(struct pci_dev *pdev)
+{
+	return 0;
+}
 #endif
 
 #endif /* __SOC_RASPBERRY_FIRMWARE_H__ */