On Wed, Aug 27, 2014 at 09:07:15AM -0400, Ashwin Chaugule wrote:
> On 27 August 2014 06:27, Mark Brown <broonie@xxxxxxxxxx> 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?
Attachment:
signature.asc
Description: Digital signature
