On Wed, Aug 10, 2011 at 05:31:28PM -0600, Kenneth Heitke wrote:
> SLIMbus (Serial Low Power Interchip Media Bus) is a specification
> developed by MIPI (Mobile Industry Processor Interface) alliance.
Please CC me on any future spins of this patch.
> Signed-off-by: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>
You ought to sign off the patch as well as you're part of the chain for
it getting into the kernel.
> +#ifdef CONFIG_PM_SLEEP
> +static int slim_legacy_suspend(struct device *dev, pm_message_t mesg)
> +{
> + struct slim_device *slim_dev = NULL;
Why have legacy stuff in a newly implemented subsystem?
> +static int slim_drv_probe(struct device *dev)
> +{
> + const struct slim_driver *sdrv = to_slim_driver(dev->driver);
> +
> + if (sdrv->probe)
> + return sdrv->probe(to_slim_device(dev));
> + return -ENODEV;
> +}
Why all the -ENODEVs if there's no function? I'd expect that if there's
nothing to do it should be possible to omit functions from drivers.
> +int slim_driver_register(struct slim_driver *drv)
> +{
> + drv->driver.bus = &slimbus_type;
> + if (drv->probe)
> + drv->driver.probe = slim_drv_probe;
Modifying the driver structure seems icky, why is this needed? Other
bus types appear to manage without.
> +static u16 slim_slicecodefromsize(u32 req)
> +{
> + u8 codetosize[8] = {1, 2, 3, 4, 6, 8, 12, 16};
> + if (req >= 8)
> + return 0;
ARRAY_SIZE()? I guess the table should be static too.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
