Hi Josh,

On Mon, 2013-10-28 at 13:12 -0500, Josh Cartwright wrote: 
> From: Kenneth Heitke <khei...@codeaurora.org>
> 
> System Power Management Interface (SPMI) is a specification
> developed by the MIPI (Mobile Industry Process Interface) Alliance
> optimized for the real time control of Power Management ICs (PMIC).
> 
> SPMI is a two-wire serial interface that supports up to 4 master
> devices and up to 16 logical slaves.
> 
> The framework supports message APIs, multiple busses (1 controller
> per bus) and multiple clients/slave devices per controller.
> 
> Signed-off-by: Kenneth Heitke <khei...@codeaurora.org>
> Signed-off-by: Michael Bohan <mbo...@codeaurora.org>
> Signed-off-by: Josh Cartwright <jo...@codeaurora.org>
> ---
>  drivers/Kconfig                 |   2 +
>  drivers/Makefile                |   1 +
>  drivers/spmi/Kconfig            |   9 +
>  drivers/spmi/Makefile           |   4 +
>  drivers/spmi/spmi.c             | 491 
> ++++++++++++++++++++++++++++++++++++++++
>  include/dt-bindings/spmi/spmi.h |  18 ++
>  include/linux/mod_devicetable.h |   8 +
>  include/linux/spmi.h            | 342 ++++++++++++++++++++++++++++
>  8 files changed, 875 insertions(+)
>  create mode 100644 drivers/spmi/Kconfig
>  create mode 100644 drivers/spmi/Makefile
>  create mode 100644 drivers/spmi/spmi.c
>  create mode 100644 include/dt-bindings/spmi/spmi.h
>  create mode 100644 include/linux/spmi.h
> 

<snip>

> +#ifdef CONFIG_PM_SLEEP
> +static int spmi_pm_suspend(struct device *dev)
> +{
> +     const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;

This is what pm_generic_suspend() do. Do we really need this wrapper?

> +
> +     if (pm)
> +             return pm_generic_suspend(dev);
> +     else
> +             return 0;
> +}
> +
> +static int spmi_pm_resume(struct device *dev)
> +{
> +     const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +
> +     if (pm)
> +             return pm_generic_resume(dev);
> +     else
> +             return 0;
> +}
> +#else
> +#define spmi_pm_suspend              NULL
> +#define spmi_pm_resume               NULL
> +#endif
> +
> +static const struct dev_pm_ops spmi_pm_ops = {
> +     .suspend        = spmi_pm_suspend,
> +     .resume         = spmi_pm_resume,
> +     SET_RUNTIME_PM_OPS(
> +             pm_generic_suspend,
> +             pm_generic_resume,
> +             pm_generic_runtime_idle
> +     )
> +};
> +

<snip>

> +
> +static int spmi_drv_probe(struct device *dev)
> +{
> +     const struct spmi_driver *sdrv = to_spmi_driver(dev->driver);
> +     int err = 0;
> +
> +     if (sdrv->probe)
> +             err = sdrv->probe(to_spmi_device(dev));
> +
> +     return err;
> +}
> +
> +static int spmi_drv_remove(struct device *dev)
> +{
> +     const struct spmi_driver *sdrv = to_spmi_driver(dev->driver);
> +     int err = 0;
> +
> +     if (sdrv->remove)
> +             err = sdrv->remove(to_spmi_device(dev));
> +
> +     return err;
> +}
> +
> +static void spmi_drv_shutdown(struct device *dev)
> +{
> +     const struct spmi_driver *sdrv = to_spmi_driver(dev->driver);
> +
> +     if (sdrv->shutdown)

If driver for device is not loaded this will cause kernel NULL
pointer dereference.

> +             sdrv->shutdown(to_spmi_device(dev));
> +}
> +
> +static struct bus_type spmi_bus_type = {
> +     .name           = "spmi",
> +     .match          = spmi_device_match,
> +     .pm             = &spmi_pm_ops,
> +     .probe          = spmi_drv_probe,
> +     .remove         = spmi_drv_remove,
> +     .shutdown       = spmi_drv_shutdown,
> +};
> +

<snip>

> +
> +static int of_spmi_register_devices(struct spmi_controller *ctrl)
> +{
> +     struct device_node *node;
> +     int err;
> +
> +     dev_dbg(&ctrl->dev, "of_spmi_register_devices\n");
> +
> +     if (!ctrl->dev.of_node)
> +             return -ENODEV;
> +
> +     dev_dbg(&ctrl->dev, "looping through children\n");
> +
> +     for_each_available_child_of_node(ctrl->dev.of_node, node) {
> +             struct spmi_device *sdev;
> +             u32 reg[2];
> +
> +             dev_dbg(&ctrl->dev, "adding child %s\n", node->full_name);
> +
> +             err = of_property_read_u32_array(node, "reg", reg, 2);
> +             if (err) {
> +                     dev_err(&ctrl->dev,
> +                             "node %s does not have 'reg' property\n",
> +                             node->full_name);
> +                     continue;

Shouldn't this be a fatal error?

> +             }
> +
> +             if (reg[1] != SPMI_USID) {
> +                     dev_err(&ctrl->dev,
> +                             "node %s contains unsupported 'reg' entry\n",
> +                             node->full_name);
> +                     continue;
> +             }
> +
> +             if (reg[0] > 0xF) {
> +                     dev_err(&ctrl->dev,
> +                             "invalid usid on node %s\n",
> +                             node->full_name);
> +                     continue;
> +             }
> +
> +             dev_dbg(&ctrl->dev, "read usid %02x\n", reg[0]);
> +
> +             sdev = spmi_device_alloc(ctrl);
> +             if (!sdev)
> +                     continue;
> +
> +             sdev->dev.of_node = node;
> +             sdev->usid = (u8) reg[0];
> +
> +             err = spmi_device_add(sdev);
> +             if (err) {
> +                     dev_err(&sdev->dev,
> +                             "failure adding device. status %d\n", err);
> +                     spmi_device_put(sdev);
> +                     continue;
> +             }
> +     }
> +
> +     return 0;
> +}
> +
> +int spmi_controller_add(struct spmi_controller *ctrl)
> +{
> +     int ret;
> +
> +     /* Can't register until after driver model init */
> +     if (WARN_ON(!spmi_bus_type.p))
> +             return -EAGAIN;
> +
> +     ret = device_add(&ctrl->dev);
> +     if (ret)
> +             return ret;
> +
> +     if (IS_ENABLED(CONFIG_OF))
> +             of_spmi_register_devices(ctrl);

Check for error code here?

> +
> +     dev_dbg(&ctrl->dev, "spmi-%d registered: dev:%p\n",
> +             ctrl->nr, &ctrl->dev);
> +
> +     return 0;
> +};
> +EXPORT_SYMBOL_GPL(spmi_controller_add);
> +


Regards,
Ivan

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to