Hi Chanwoo,

I love the idea and I now have a little mechanical issues in your code.

It looks getting mature fast.
However, I would like to suggest you to

allow not to compile devfreq-event.c and not include its compiled object
  if devfreq.c is required but devfreq-event.c is not required.
  (e.g., add CONFIG_PM_DEVFREQ_EVENT and let it be enabled when needed)
  just a little concern for lightweight devices.
    (this change might require a bit more work on the header as well)
  - Or do you think devfreq-event.c will become almost mandatory for
   most devfreq drivers?


[snip]

DEVFREQ is a generic DVFS mechanism (or subsystem).

Plus, I thought devfreq-event is considered to be a "framework"
for devfreq event class drivers. Am I mistaken?

[snip]

+struct devfreq_event_dev *devfreq_event_add_edev(struct device *dev,
+                                               struct devfreq_event_desc *desc)
+{
+       struct devfreq_event_dev *edev;
+       static atomic_t event_no = ATOMIC_INIT(0);
+       int ret;
+
+       if (!dev || !desc)
+               return ERR_PTR(-EINVAL);
+
+       if (!desc->name || !desc->ops)
+               return ERR_PTR(-EINVAL);
+
+       if (!desc->ops->set_event || !desc->ops->get_event)
+               return ERR_PTR(-EINVAL);
+
+       edev = devm_kzalloc(dev, sizeof(*edev), GFP_KERNEL);
+       if (!edev)
+               return ERR_PTR(-ENOMEM);
+
+       mutex_lock(&devfreq_event_list_lock);

You seem to lock that global lock too long. That lock is only required
while you operate the list. The data to be protected by this mutex is
devfreq_event_list. Until the new entry is added to the list, the new
entry is free from protection. (may be delayed right before list_add)

+       mutex_init(&edev->lock);
+       edev->desc = desc;
+       edev->dev.parent = dev;
+       edev->dev.class = devfreq_event_class;
+       edev->dev.release = devfreq_event_release_edev;
+
+       dev_set_name(&edev->dev, "event.%d", atomic_inc_return(&event_no) - 1);
+       ret = device_register(&edev->dev);
+       if (ret < 0) {
+               put_device(&edev->dev);
+               mutex_unlock(&devfreq_event_list_lock);
+               return ERR_PTR(ret);
+       }
+       dev_set_drvdata(&edev->dev, edev);
+
+       INIT_LIST_HEAD(&edev->node);
+       list_add(&edev->node, &devfreq_event_list);
+       mutex_unlock(&devfreq_event_list_lock);
+
+       return edev;
+}

[snip / reversed maybe.. sorry]

+/**
+ * devfreq_event_is_enabled() - Check whether devfreq-event dev is enabled or
+ *                             not.
+ * @edev       : the devfreq-event device
+ *
+ * Note that this function check whether devfreq-event dev is enabled or not.
+ * If return true, the devfreq-event dev is enabeld. If return false, the
+ * devfreq-event dev is disabled.
+ */
+bool devfreq_event_is_enabled(struct devfreq_event_dev *edev)
+{
+       bool enabled = false;
+
+       if (!edev || !edev->desc)
+               return enabled;
+
+       mutex_lock(&edev->lock);
+
+       if (edev->enable_count > 0)
+               enabled = true;
+
+       if (edev->desc->ops && edev->desc->ops->is_enabled)
+               enabled |= edev->desc->ops->is_enabled(edev);

What does it mean when enabled_count > 0 and ops->is_enabled() is false? or..
What does it mean when enabled_count = 0 and ops->is_enabled() is true?

If you do enable_count in the subsystem, why would we rely on
ops->is_enabled()? Are you assuming that a device MAY turn itself off
without any kernel control (ops->disable()) and it is still a correct
behabior?

+
+       mutex_unlock(&edev->lock);
+
+       return enabled;
+}
+EXPORT_SYMBOL_GPL(devfreq_event_is_enabled);

Cheers,
MyungJoo