linux-kernel - RE: [RESEND PATCH V2 1/2] input: misc: da9063: OnKey driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 29 Apr 2015 16:01:03 +0000
From:	"Opensource [Steve Twiss]" <stwiss.opensource@...semi.com>
To:	Lee Jones <lee.jones@...aro.org>,
	"Opensource [Steve Twiss]" <stwiss.opensource@...semi.com>
CC:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	DT <devicetree@...r.kernel.org>,
	David Dajun Chen <david.chen@...semi.com>,
	INPUT <linux-input@...r.kernel.org>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Mark Rutland <mark.rutland@....com>,
	"Pawel Moll" <pawel.moll@....com>,
	Rob Herring <robh+dt@...nel.org>,
	"Support Opensource" <Support.Opensource@...semi.com>
Subject: RE: [RESEND PATCH V2 1/2] input: misc: da9063: OnKey driver


On 28 April 2015 12:57 Lee Jones [mailto:lee.jones@...aro.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;
 }

Thanks for the review comments.
The next patch set for DA9063 will follow shortly.

Regards,
Steve