[PATCH 2/3] ARM: bcm2835: Add the Raspberry Pi firmware driver
eric at anholt.net
Wed Apr 29 10:51:30 PDT 2015
Stephen Warren <swarren at wwwdotorg.org> writes:
> On 04/27/2015 05:14 PM, Eric Anholt wrote:
>> This gives us a function for making mailbox property channel requests
>> of the firmware, and uses it to control the 3 power domains provided
>> by the firmware.
>> diff --git a/arch/arm/mach-bcm/raspberrypi-firmware.c b/arch/arm/mach-bcm/raspberrypi-firmware.c
>> + * Submits a set of concatenated tags to the VPU firmware through the
>> + * mailbox property interface.
>> + *
>> + * The buffer header and the ending tag are added by this function and
>> + * don't need to be supplied, just the actual tags for your operation.
>> + * See struct raspberrypi_firmware_property_tag_header for the per-tag structure.
>> + */
>> +int raspberrypi_firmware_property(void *data, size_t tag_size)
>> + size_t size = tag_size + 12;
>> + u32 *buf;
>> + dma_addr_t bus_addr;
>> + int ret = 0;
>> + if (!firmware)
>> + return -EPROBE_DEFER;
> I think it'd make more sense if the clients looked up the firmware
> driver via phandle at their probe time. This would mean:
> * No need for global "firmware", since clients could pass the firmware
> driver handle into this function.
> * Clients resolve deferred probe at their probe time. That way, they
> won't register themselves with subsystems asserting they can provide
> services, but find out they can't yet provide the service at that time.
The one client so far (vc4) was resolving deferred probe at its probe
time, but not taking a reference on the firmware driver. I figure I'll
have it do the phandle lookup and refcount -- do you still want the
struct platform_device passed in here? If we de-global firmware, it's
going to mean some faffing in the power domain side of things to find
the device again, it seems.
>> +static int raspberrypi_firmware_probe(struct platform_device *pdev)
> BTW if you wanted, a s/raspberrypi/rpi/ in all symbols and filenames
> seems reasonable. I'm not really bothered either way.
I lean slightly toward avoiding the abbreviation, but I could go either
way as well.
> I would have hoped that mbox_request_channel() returned -EPROBE_DEFER
> itself. It really should...
>> +static int raspberrypi_firmware_remove(struct platform_device *pdev)
>> + struct device *dev = &pdev->dev;
>> + of_genpd_del_provider(dev->of_node);
>> + mbox_free_channel(firmware->chan);
> firmware = NULL; ?
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 818 bytes
Desc: not available
More information about the linux-rpi-kernel