On Wed, 29 Apr 2015, Opensource [Steve Twiss] wrote: > > On 28 April 2015 12:57 Lee Jones [mailto:lee.jo...@linaro.org] wrote: > > > On Fri, 17 Apr 2015, S Twiss wrote: > > > > ++++++++++++++++++++++++++++++++++++++ > > > drivers/mfd/da9063-core.c | 55 +++++++++ > > > include/linux/mfd/da9063/pdata.h | 1 + > > > > This should be a seperate patch. > > > > Okay, done this now. Added a new PATCH 3/3 > > > > static struct resource da9063_onkey_resources[] = { > > > { > > > + .name = "ONKEY", > > > .start = DA9063_IRQ_ONKEY, > > > .end = DA9063_IRQ_ONKEY, > > > .flags = IORESOURCE_IRQ, > > > @@ -97,6 +98,7 @@ static const struct mfd_cell da9063_devs[] = { > > > .name = DA9063_DRVNAME_ONKEY, > > > .num_resources = > > ARRAY_SIZE(da9063_onkey_resources), > > > .resources = da9063_onkey_resources, > > > + .of_compatible = "dlg,da9063-onkey", > > > }, > > > { > > > .name = DA9063_DRVNAME_RTC, > > > > This is lowercase, so why does "ONKEY" have to be uppercase? > > > > No real reason why this is uppercase in favour of lowercase except it > is following the convention of the existing DA9063 driver code. > Currently the DA9063 uses uppercase for its naming, there are several > others components that use the same uppercase convention, e.g. the > RTC alarm and tick interrupt and the hardware LDO limit: > > > cat /proc/interrupts | grep 9063 > 384: 0 0 0 0 da9063-irq 0 ONKEY > 385: 0 2 0 0 da9063-irq 1 ALARM > 387: 0 30 0 0 da9063-irq 3 HWMON > 392: 0 0 0 0 da9063-irq 8 LDO_LIM > > I was going to leave this uppercase, but I can easily change it if > this is necessary. > > > > if (ret) > > > dev_err(da9063->dev, "Cannot add MFD cells\n"); > > > > > > + > > > > Tut tut! > > > > > return ret; > > > } > > I've changed that to remove the lazy fall-through on the error path. > It now has the following form: > > @@ -229,9 +229,10 @@ int da9063_device_init(struct da9063 *da9063, unsigned > int irq) > ret = mfd_add_devices(da9063->dev, -1, da9063_devs, > ARRAY_SIZE(da9063_devs), NULL, da9063->irq_base, > NULL); > - if (ret) > + if (ret) { > dev_err(da9063->dev, "Cannot add MFD cells\n"); > - > + return ret; > + } > > return ret; > }
Sorry, that's not what I meant. The fall-through is perfectly fine. I was tutting because you added an unrelated 'clean-up'. > Thanks for the review comments. > The next patch set for DA9063 will follow shortly. > > Regards, > Steve -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
