On Wed, Aug 27, 2014 at 09:07:15AM -0400, Ashwin Chaugule wrote:
On 27 August 2014 06:27, Mark Brown broonie@kernel.org wrote:
On Tue, Aug 26, 2014 at 03:35:36PM -0400, Ashwin Chaugule wrote:
+static struct mbox_controller * +mbox_find_pcc_controller(char *name) +{
struct mbox_controller *mbox;
list_for_each_entry(mbox, &mbox_cons, node) {
if (mbox->name)
if (!strcmp(mbox->name, name))
return mbox;
}
return NULL;
+}
This doesn't look particularly PCC specific?
Call this mbox_find_controller_by_name() instead?
That certainly looks like what it's doing. Probably also make the name that gets passed in const while you're at it.
/* Sanity check */
if (!mbox || !mbox->dev || !mbox->ops || !mbox->num_chans)
/*
* PCC clients and controllers are currently not backed by
* platform device structures.
*/
+#ifndef CONFIG_PCC
if (!mbox->dev)
return -EINVAL;
+#endif
It seems better to make this consistent - either enforce it all the time or don't enforce it.
So this is where it got really messy. We're trying to create a
The messiness is orthogonal to my comment here - either it's legal to request a mailbox without a device or it isn't, it shouldn't depend on a random kernel configuration option for a particular mailbox driver which it is.
"device" out of something that isn't. The PCCT, which is used as a mailbox controller here, is a table and not a peripheral device. To treat this as a device (without faking it by manually putting together a struct device), would require adding a DSDT entry which is really a wrong place for it. Are there examples today where drivers manually create a struct driver and struct device and match them internally? (i.e. w/o using the generic driver subsystem)
Arguably that's what things like cpufreq end up doing, though people tend to just shove a device into DT. Are you sure there isn't any device at all in ACPI that you could hang this off, looking at my desktop I see rather a lot of apparently synthetic ACPI devices with names starting LNX including for example LNXSYSTM:00?
The main reason why I thought this Mailbox framework looked useful (after you pointed me to it) for PCC was due to its async notification features. But thats easy and small enough to add to the PCC driver itself. We can also add a generic controller lookup mechanism in the PCC driver for anyone who doesn't want to use ACPI. I think thats a much cleaner way to handle PCC support. Adding PCC as a generic mailbox controller is turning out to be more messier that we'd originally imagined.
If PCC is described by ACPI tables how would non-ACPI users be able to use it?