Re: [PATCH 2/5 v2] Input: bu21013_ts - Move GPIO init and exit functions into the driver

2012-11-26 Thread Dmitry Torokhov
Hi Lee,

On Mon, Nov 26, 2012 at 12:01:43PM +, Lee Jones wrote:
>  /**
> + * bu21013_gpio_board_init() - configures the touch panel
> + * @reset_pin: reset pin number
> + *
> + * This function is used to configure the voltage and
> + * reset the touch panel controller.
> + */
> +static int bu21013_gpio_board_init(int reset_pin)
> +{
> + int retval = 0;
> +
> + retval = gpio_request_one(reset_pin, GPIOF_INIT_HIGH, "touchp_reset");

Also need to specify dircetion (even though it defaults to output).

> + if (retval) {
> + printk(KERN_ERR "Unable to request gpio reset_pin");
> + return retval;
> + }
> +
> + return retval;
> +}
> +
> +/**
> + * bu21013_gpio_board_exit() - deconfigures the touch panel controller
> + * @reset_pin: reset pin number
> + *
> + * This function is used to deconfigure the chip selection
> + * for touch panel controller.
> + */
> +static int bu21013_gpio_board_exit(int reset_pin)
> +{
> + int retval = 0;
> +
> + retval = gpio_direction_output(reset_pin, 0);
> + if (retval < 0) {
> + printk(KERN_ERR "%s: gpio direction failed\n",
> +__func__);
> + return retval;

You should not return here, as gpio has to be freed even if you unable
to toggle it.

> + }
> + gpio_set_value(reset_pin, 0);
> + gpio_free(reset_pin);
> +
> + return retval;
> +}
> +
> +/**
>   * bu21013_init_chip() - power on sequence for the bu21013 controller
>   * @data: device structure pointer
>   *
> @@ -449,6 +493,8 @@ static int __devinit bu21013_probe(struct i2c_client 
> *client,
>   return -EINVAL;
>   }
>  
> + pdata->irq = gpio_to_irq(pdata->touch_pin);
> +

No, you still can not do it since pdata is a const pointer. Does not
your compiler throw a warning here?

I already sent you a version of the patch that does not have these
issues, is there a particular reason why you needed to roll your own
instead of simply trying out the one I sent?

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 1/5] Input: bu21013_ts - Request a regulator that actually exists

2012-11-26 Thread Dmitry Torokhov
On Mon, Nov 26, 2012 at 12:16:18PM +, Lee Jones wrote:
> On Wed, 14 Nov 2012, Lee Jones wrote:
> 
> > Currently the BU21013 Touch Screen driver requests a regulator by the
> > name of 'V-TOUCH', which doesn't exist anywhere in the kernel. The
> > correct name, as referenced in platform regulator code is 'avdd'. Here,
> > when we request a regulator, we use the correct name instead.
> > 
> > Cc: Dmitry Torokhov 
> > Cc: linux-in...@vger.kernel.org
> > Acked-by: Arnd Bergmann 
> > Acked-by: Linus Walleij 
> > Signed-off-by: Lee Jones 
> > ---
> >  drivers/input/touchscreen/bu21013_ts.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/input/touchscreen/bu21013_ts.c 
> > b/drivers/input/touchscreen/bu21013_ts.c
> > index 5c487d2..2fae682 100644
> > --- a/drivers/input/touchscreen/bu21013_ts.c
> > +++ b/drivers/input/touchscreen/bu21013_ts.c
> > @@ -461,7 +461,7 @@ static int __devinit bu21013_probe(struct i2c_client 
> > *client,
> > bu21013_data->chip = pdata;
> > bu21013_data->client = client;
> >  
> > -   bu21013_data->regulator = regulator_get(&client->dev, "V-TOUCH");
> > +   bu21013_data->regulator = regulator_get(&client->dev, "avdd");
> > if (IS_ERR(bu21013_data->regulator)) {
> > dev_err(&client->dev, "regulator_get failed\n");
> > error = PTR_ERR(bu21013_data->regulator);
> > -- 
> > 1.7.9.5
> 
> Did you see this one also Dmitry?

Yes, I have it, I am waiting for the other 2 patches in series to
settle.

-- 
Dmitry
--
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/


Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support

2012-11-26 Thread Dmitry Torokhov
On Mon, Nov 26, 2012 at 09:57:45PM +0530, Viresh Kumar wrote:
> On 20 November 2012 14:17, Dmitry Torokhov  wrote:
> > No, not really, it just does not work well with devm_* patches that got
> > applied: on removal you unprepare clock as the very first operation and
> > then devm_* does the rest which is wrong order.
> >
> > I am looking at adding dem_* for clocks.
> 
> I haven't seen your clock patch in your tree. I am asking because i want to
> push this patch :)

Yeah, I think I am not going to rush it and let it take its normal
course instead of short-cutting through my tree. I am going to apply the
patcch below in the meantime.

Thanks.

-- 
Dmitry


Input: spear-keyboard - Add clk_{un}prepare() support

From: Vipul Kumar Samar 

clk_{un}prepare is mandatory for platforms using common clock framework.
Because for SPEAr we don't do anything in clk_{un}prepare() calls, just
call them once in probe/remove.

Signed-off-by: Vipul Kumar Samar 
Signed-off-by: Viresh Kumar 
Signed-off-by: Dmitry Torokhov 
---
 drivers/input/keyboard/spear-keyboard.c |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/input/keyboard/spear-keyboard.c 
b/drivers/input/keyboard/spear-keyboard.c
index d70093b..695d237 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -267,9 +267,14 @@ static int spear_kbd_probe(struct platform_device *pdev)
return error;
}
 
+   error = clk_prepare(kbd->clk);
+   if (error)
+   return error;
+
error = input_register_device(input_dev);
if (error) {
dev_err(&pdev->dev, "Unable to register keyboard device\n");
+   clk_unprepare(kbd->clk);
return error;
}
 
@@ -281,6 +286,11 @@ static int spear_kbd_probe(struct platform_device *pdev)
 
 static int spear_kbd_remove(struct platform_device *pdev)
 {
+   struct spear_kbd *kbd = platform_get_drvdata(pdev);
+
+   input_unregister_device(kbd->input);
+   clk_unprepare(kbd->clk);
+
device_init_wakeup(&pdev->dev, 0);
platform_set_drvdata(pdev, NULL);
 
--
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/


Re: [Pv-drivers] [PATCH 00/12] VMCI for Linux upstreaming

2012-11-26 Thread Dmitry Torokhov
On Monday, November 26, 2012 02:37:54 PM Greg KH wrote:
> On Wed, Nov 21, 2012 at 12:31:04PM -0800, George Zhang wrote:
> > * * *
> > This series of VMCI linux upstreaming patches include latest udpate from
> > VMware.
> > 
> > Summary of changes:
> > - Sparse clean.
> > - Checkpatch clean with one exception, a "complex macro" in
> > 
> >   which we can't add parentheses.
> > 
> > - Remove all runtime assertions.
> > - Fix device name, so that existing user clients work.
> > - Fix VMCI handle lookup.
> 
> Given that you failed to answer the questions I asked the last time you
> posted this series, and you did not make any of the changes I asked for,
> I can't accept this (nor should you expect me to.)
> 
> And people wonder why reviewers get so grumpy...
> 
> My trees are now closed for the 3.8 merge window, so feel free to try
> again after 3.8-rc1 is out, and you have answered, and addressed, the
> questions and comments I made.

Greg, there were 3 specific complaints from you:

1. "Given that this is a static function, there's no need for these
"asserts", right?  Please send a follow-on patch removing all BUG_ON()
calls from these files, it's not acceptable to crash a user's box from
a driver that is handling parameters you are feeding it."

2. "You obviously didn't run checkpatch on this file"

3. "This line causes sparse to complain.  The odds that userspace knows
what gcc is using for "bool" is pretty low."

Given the fact that the series addresses all 3 I fail to understand why
you would be grumpy.

Anyway, since there vsock has not been reviewed yet we are OK with
postponing this patch series till 3.9.

Thanks,
Dmitry

--
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/


Re: [Pv-drivers] [PATCH 00/12] VMCI for Linux upstreaming

2012-11-26 Thread Dmitry Torokhov
On Monday, November 26, 2012 03:23:57 PM Greg KH wrote:
> On Mon, Nov 26, 2012 at 03:01:04PM -0800, Dmitry Torokhov wrote:
> > On Monday, November 26, 2012 02:37:54 PM Greg KH wrote:
> > > On Wed, Nov 21, 2012 at 12:31:04PM -0800, George Zhang wrote:
> > > > * * *
> > > > This series of VMCI linux upstreaming patches include latest udpate
> > > > from
> > > > VMware.
> > > > 
> > > > Summary of changes:
> > > > - Sparse clean.
> > > > - Checkpatch clean with one exception, a "complex macro" in
> > > > 
> > > >   which we can't add parentheses.
> > > > 
> > > > - Remove all runtime assertions.
> > > > - Fix device name, so that existing user clients work.
> > > > - Fix VMCI handle lookup.
> > > 
> > > Given that you failed to answer the questions I asked the last time you
> > > posted this series, and you did not make any of the changes I asked for,
> > > I can't accept this (nor should you expect me to.)
> > > 
> > > And people wonder why reviewers get so grumpy...
> > > 
> > > My trees are now closed for the 3.8 merge window, so feel free to try
> > > again after 3.8-rc1 is out, and you have answered, and addressed, the
> > > questions and comments I made.
> > 
> > Greg, there were 3 specific complaints from you:
> > 
> > 1. "Given that this is a static function, there's no need for these
> > "asserts", right?  Please send a follow-on patch removing all BUG_ON()
> > calls from these files, it's not acceptable to crash a user's box from
> > a driver that is handling parameters you are feeding it."
> > 
> > 2. "You obviously didn't run checkpatch on this file"
> > 
> > 3. "This line causes sparse to complain.  The odds that userspace knows
> > what gcc is using for "bool" is pretty low."
> > 
> > Given the fact that the series addresses all 3 I fail to understand why
> > you would be grumpy.
> 
> You are ignoring my response to patch 12/12 for some reason (which
> repeated a bunch of the questions I had with that patch the last time it
> was posted.)  That is what I am referring to here.  None of those
> questions were addressed.

That one was explicitly acknowledged in
<20121030052234.gh32...@dtor-ws.eng.vmware.com> and fixed in series
posted on 11/01. Since it was fixed in earlier posting we did not
mention it again.

> 
> Also, how was I to know that those 3 comments above were addressed?
> When someone posts questions and comments, please respond to those
> comments.  Don't just not respond at all and post the whole series 2
> weeks later with things changed and a vague comment of "summary of
> changes" in the 00 message.  Otherwise I will assume that you never even
> saw my post.

I thought "Sparse clean" and "Checkpatch clean with one exception ..."
are concrete enough, but I am open to improving the messaging. What 
would you like us to say?

> 
> In other words, if someone takes the time to review and post comments,
> the least you can do is acknowledge those comments, right?

We did not want to litter mailing lists with "OK" responses, but will
do in the future.

Thanks,
Dmitry
--
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/


Re: [Pv-drivers] [PATCH 00/12] VMCI for Linux upstreaming

2012-11-26 Thread Dmitry Torokhov
On Monday, November 26, 2012 03:44:26 PM Greg KH wrote:
> On Mon, Nov 26, 2012 at 03:36:52PM -0800, Dmitry Torokhov wrote:
> > On Monday, November 26, 2012 03:23:57 PM Greg KH wrote:
> > > On Mon, Nov 26, 2012 at 03:01:04PM -0800, Dmitry Torokhov wrote:
> > > > On Monday, November 26, 2012 02:37:54 PM Greg KH wrote:
> > > > > On Wed, Nov 21, 2012 at 12:31:04PM -0800, George Zhang wrote:
> > > > > > * * *
> > > > > > This series of VMCI linux upstreaming patches include latest
> > > > > > udpate
> > > > > > from
> > > > > > VMware.
> > > > > > 
> > > > > > Summary of changes:
> > > > > > - Sparse clean.
> > > > > > - Checkpatch clean with one exception, a "complex macro" in
> > > > > > 
> > > > > >   which we can't add parentheses.
> > > > > > 
> > > > > > - Remove all runtime assertions.
> > > > > > - Fix device name, so that existing user clients work.
> > > > > > - Fix VMCI handle lookup.
> > > > > 
> > > > > Given that you failed to answer the questions I asked the last time
> > > > > you
> > > > > posted this series, and you did not make any of the changes I asked
> > > > > for,
> > > > > I can't accept this (nor should you expect me to.)
> > > > > 
> > > > > And people wonder why reviewers get so grumpy...
> > > > > 
> > > > > My trees are now closed for the 3.8 merge window, so feel free to
> > > > > try
> > > > > again after 3.8-rc1 is out, and you have answered, and addressed,
> > > > > the
> > > > > questions and comments I made.
> > > > 
> > > > Greg, there were 3 specific complaints from you:
> > > > 
> > > > 1. "Given that this is a static function, there's no need for these
> > > > "asserts", right?  Please send a follow-on patch removing all BUG_ON()
> > > > calls from these files, it's not acceptable to crash a user's box from
> > > > a driver that is handling parameters you are feeding it."
> > > > 
> > > > 2. "You obviously didn't run checkpatch on this file"
> > > > 
> > > > 3. "This line causes sparse to complain.  The odds that userspace
> > > > knows
> > > > what gcc is using for "bool" is pretty low."
> > > > 
> > > > Given the fact that the series addresses all 3 I fail to understand
> > > > why
> > > > you would be grumpy.
> > > 
> > > You are ignoring my response to patch 12/12 for some reason (which
> > > repeated a bunch of the questions I had with that patch the last time it
> > > was posted.)  That is what I am referring to here.  None of those
> > > questions were addressed.
> > 
> > That one was explicitly acknowledged in
> > <20121030052234.gh32...@dtor-ws.eng.vmware.com> and fixed in series
> > posted on 11/01. Since it was fixed in earlier posting we did not
> > mention it again.
> 
> I questioned it on November 15, in:
>   Message-ID: <20121116000118.ga8...@kroah.com>
> 
> Just ignoring that long response is acceptable?  Really?  I didn't ask
> enough questions in that review?  I see obvious comments in there that
> were _not_ addressed in the November 21st posting of that patch
> (typedefs for u32?  No c99 initializers?)

Hmm, neither I nor Google is aware of that msgid... So that would explain
why we have not addressed the comments that were in it ;)

Mind resending it, please? 

> 
> And why isn't George responding to my comments when I ask questions?
> 
> Also, please start numbering the submissions, this having to reference
> them by date is going to cause us all to get even more confused quicker.

OK, will do.

Thanks,
Dmitry

--
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/


Re: [PATCH 12/12] VMCI: Some header and config files.

2012-11-26 Thread Dmitry Torokhov
Hi Greg,

For some reason it still didn't go through to our corporate mail server
but I see it on LKML.

On Mon, Nov 26, 2012 at 04:03:04PM -0800, Greg KH wrote:
> On Wed, Nov 07, 2012 at 10:43:03AM -0800, George Zhang wrote:
> 
> > +static inline struct vmci_handle VMCI_MAKE_HANDLE(vmci_id cid, vmci_id rid)
> > +{
> > +   struct vmci_handle h;
> > +   h.context = cid;
> > +   h.resource = rid;
> > +   return h;
> > +}
> 
> You return a structure on the stack that just went away?  Yeah, I know
> it's an inline, but come on, that's not ok.

This is certainly OK even if it is not inline, we return the _value_,
not the pointer to the stacki memory. And yes, the structure is 64 bit
value so it is returned in registers.

> 
> > +#define VMCI_HANDLE_TO_CONTEXT_ID(_handle) ((_handle).context)
> > +#define VMCI_HANDLE_TO_RESOURCE_ID(_handle) ((_handle).resource)
> 
> It's longer to write the macro out than to just do .context or
> .resource, just use them instead.

We really want vmci_handle to be opaque where possible and use proper
accessors.

> 
> > +#define VMCI_HANDLE_EQUAL(_h1, _h2) ((_h1).context == (_h2).context && 
> > \
> > +(_h1).resource == (_h2).resource)
> 
> Inline function instead?  How often do you really need this?

OK, makes sense.

> 
> > +#define VMCI_INVALID_ID ~0
> > +static const struct vmci_handle VMCI_INVALID_HANDLE = { VMCI_INVALID_ID,
> > +   VMCI_INVALID_ID
> > +};
> 
> C99 initializers please.

OK.

> 
> > +
> > +#define VMCI_HANDLE_INVALID(_handle)   
> > \
> > +   VMCI_HANDLE_EQUAL((_handle), VMCI_INVALID_HANDLE)
> 
> Gotta love magic values :(

?

> 
> > +/*
> > + * The below defines can be used to send anonymous requests.
> > + * This also indicates that no response is expected.
> > + */
> > +#define VMCI_ANON_SRC_CONTEXT_ID   VMCI_INVALID_ID
> > +#define VMCI_ANON_SRC_RESOURCE_ID  VMCI_INVALID_ID
> > +#define VMCI_ANON_SRC_HANDLE   
> > vmci_make_handle(VMCI_ANON_SRC_CONTEXT_ID, \
> > +   VMCI_ANON_SRC_RESOURCE_ID)
> 
> So you just created an invalid message?

Anonymous one, yes.

> 
> > +/* The lowest 16 context ids are reserved for internal use. */
> > +#define VMCI_RESERVED_CID_LIMIT ((u32) 16)
> > +
> > +/*
> > + * Hypervisor context id, used for calling into hypervisor
> > + * supplied services from the VM.
> > + */
> > +#define VMCI_HYPERVISOR_CONTEXT_ID 0
> > +
> > +/*
> > + * Well-known context id, a logical context that contains a set of
> > + * well-known services. This context ID is now obsolete.
> > + */
> > +#define VMCI_WELL_KNOWN_CONTEXT_ID 1
> > +
> > +/*
> > + * Context ID used by host endpoints.
> > + */
> > +#define VMCI_HOST_CONTEXT_ID  2
> > +
> > +#define VMCI_CONTEXT_IS_VM(_cid) (VMCI_INVALID_ID != (_cid) && 
> > \
> > + (_cid) > VMCI_HOST_CONTEXT_ID)
> > +
> 
> Are you sure all of this stuff needs to be in a .h file that lives in
> include/linux/?

Probably not. We'll rebase to 3.7 and split as needed.

> 
> > +/*
> > + * Linux defines _IO* macros, but the core kernel code ignore the encoded
> > + * ioctl value. It is up to individual drivers to decode the value (for
> > + * example to look at the size of a structure to determine which version
> > + * of a specific command should be used) or not (which is what we
> > + * currently do, so right now the ioctl value for a given command is the
> > + * command itself).
> > + *
> > + * Hence, we just define the IOCTL_VMCI_foo values directly, with no
> > + * intermediate IOCTLCMD_ representation.
> > + */
> > +#define IOCTLCMD(_cmd) IOCTL_VMCI_ ## _cmd
> 
> I don't recall ever getting a valid answer for this (if you did, my
> appologies, can you repeat it).  What in the world are you talking about
> here?  Why is your driver somehow special from the thousands of other
> ones that use the in-kernel IO macros properly for an ioctl?

Hmm, not sure, we'll review ioctl definitions and fix as needed.

Thanks!

-- 
Dmitry
--
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/


Re: [Pv-drivers] [PATCH 12/12] VMCI: Some header and config files.

2012-11-26 Thread Dmitry Torokhov
On Monday, November 26, 2012 04:32:39 PM Greg KH wrote:
> On Mon, Nov 26, 2012 at 04:23:57PM -0800, Dmitry Torokhov wrote:
> > Hi Greg,
> > 
> > For some reason it still didn't go through to our corporate mail server
> > but I see it on LKML.
> 
> Good.
> 
> > On Mon, Nov 26, 2012 at 04:03:04PM -0800, Greg KH wrote:
> > > On Wed, Nov 07, 2012 at 10:43:03AM -0800, George Zhang wrote:
> > > > +static inline struct vmci_handle VMCI_MAKE_HANDLE(vmci_id cid,
> > > > vmci_id rid) +{
> > > > +   struct vmci_handle h;
> > > > +   h.context = cid;
> > > > +   h.resource = rid;
> > > > +   return h;
> > > > +}
> > > 
> > > You return a structure on the stack that just went away?  Yeah, I know
> > > it's an inline, but come on, that's not ok.
> > 
> > This is certainly OK even if it is not inline, we return the _value_,
> > not the pointer to the stacki memory. And yes, the structure is 64 bit
> > value so it is returned in registers.
> 
> Even on a 32bit processor? 

I thought it would, but it looks like it won't. Maybe we'll just switch it
to a macro with C99 style initializators to keep the same semantic but
avoid the question.

> Also, you already have another function that
> does this same thing, so having 2 functions in the same patch seems odd,
> right?

Yes, you can say that it is probably a bit excessive.

OK, now that we are on the same page we'll go and fix the issues.

Thanks,
Dmitry
--
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/


Re: [Pv-drivers] [PATCH 00/12] VMCI for Linux upstreaming

2012-11-26 Thread Dmitry Torokhov
On Mon, Nov 26, 2012 at 07:27:07PM -0500, Woody Suwalski wrote:
> Greg KH wrote:
> >On Mon, Nov 26, 2012 at 03:52:31PM -0800, Dmitry Torokhov wrote:
> >>
> >>Mind resending it, please?
> >Now resent.
> I see both versions of Greg's message - one from 15 Nov, one
> today's. On my Gmail account...
> So Greg did post it...
> 

Right, I also see it now in my personal LKML archive but for some
reason it didn't get delivered to my corporate mailbox. Weird.

Thanks,
Dmitry
--
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/


[PATCH v2] ARM: eukrea_mbimx27-baseboard - use stock get_pendown_state() in ads7846

2012-11-27 Thread Dmitry Torokhov
The default implementation matches exactly our custom one so we can switch
to using the default one. As a bonus the driver will take care of setting
GPIO line for us.

Signed-off-by: Dmitry Torokhov 
---

Adjusted to use IMX_GPIO_NR() macro.

 arch/arm/mach-imx/eukrea_mbimx27-baseboard.c |   19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/arch/arm/mach-imx/eukrea_mbimx27-baseboard.c 
b/arch/arm/mach-imx/eukrea_mbimx27-baseboard.c
index 98aef57..f361143 100644
--- a/arch/arm/mach-imx/eukrea_mbimx27-baseboard.c
+++ b/arch/arm/mach-imx/eukrea_mbimx27-baseboard.c
@@ -238,24 +238,8 @@ static const struct imxuart_platform_data uart_pdata 
__initconst = {
.flags = IMXUART_HAVE_RTSCTS,
 };
 
-#define ADS7846_PENDOWN (GPIO_PORTD | 25)
-
-static void __maybe_unused ads7846_dev_init(void)
-{
-   if (gpio_request(ADS7846_PENDOWN, "ADS7846 pendown") < 0) {
-   printk(KERN_ERR "can't get ads7846 pen down GPIO\n");
-   return;
-   }
-   gpio_direction_input(ADS7846_PENDOWN);
-}
-
-static int ads7846_get_pendown_state(void)
-{
-   return !gpio_get_value(ADS7846_PENDOWN);
-}
-
 static struct ads7846_platform_data ads7846_config __initdata = {
-   .get_pendown_state  = ads7846_get_pendown_state,
+   .gpio_pendown   = IMX_GPIO_NR(4, 25),
.keep_vref_on   = 1,
 };
 
@@ -323,7 +307,6 @@ void __init eukrea_mbimx27_baseboard_init(void)
|| defined(CONFIG_TOUCHSCREEN_ADS7846_MODULE)
/* ADS7846 Touchscreen controller init */
mxc_gpio_mode(GPIO_PORTD | 25 | GPIO_GPIO | GPIO_IN);
-   ads7846_dev_init();
 #endif
 
/* SPI_CS0 init */
--
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/


[PATCH 2/4] Input: RMI4 - move sensor driver and F01 handler into the core

2012-11-27 Thread Dmitry Torokhov
There is no point in having the sensor driver and F01 handler separate
from the RMI core since it is not useful without them and having them
all together simplifies initialization among other things.

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/rmi4/Kconfig  | 23 ++-
 drivers/input/rmi4/Makefile | 10 +++---
 drivers/input/rmi4/rmi_bus.c| 41 -
 drivers/input/rmi4/rmi_driver.c | 36 ++--
 drivers/input/rmi4/rmi_driver.h |  6 ++
 drivers/input/rmi4/rmi_f01.c| 22 +++---
 6 files changed, 68 insertions(+), 70 deletions(-)

diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index 41cbbee..d0c7b6e 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -1,8 +1,8 @@
 #
 # RMI4 configuration
 #
-config RMI4_BUS
-   bool "Synaptics RMI4 bus support"
+config RMI4_CORE
+   tristate "Synaptics RMI4 bus support"
help
  Say Y here if you want to support the Synaptics RMI4 bus.  This is
  required for all RMI4 device support.
@@ -13,7 +13,7 @@ config RMI4_BUS
 
 config RMI4_DEBUG
bool "RMI4 Debugging"
-   depends on RMI4_BUS
+   depends on RMI4_CORE
select DEBUG_FS
help
  Say Y here to enable debug feature in the RMI4 driver.
@@ -26,8 +26,8 @@ config RMI4_DEBUG
  If unsure, say N.
 
 config RMI4_I2C
-   bool "RMI4 I2C Support"
-   depends on RMI4_BUS && I2C
+   tristate "RMI4 I2C Support"
+   depends on RMI4_CORE && I2C
help
  Say Y here if you want to support RMI4 devices connected to an I2C
  bus.
@@ -36,20 +36,9 @@ config RMI4_I2C
 
  This feature is not currently available as a loadable module.
 
-config RMI4_GENERIC
-   bool "RMI4 Generic driver"
-   depends on RMI4_BUS
-   help
- Say Y here if you want to support generic RMI4 devices.
-
- This is pretty much required if you want to do anything useful with
- your RMI device.
-
- This feature is not currently available as a loadable module.
-
 config RMI4_F11
tristate "RMI4 Function 11 (2D pointing)"
-   depends on RMI4_BUS && RMI4_GENERIC
+   depends on RMI4_CORE
help
  Say Y here if you want to add support for RMI4 function 11.
 
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 8882c3d..5c6bad5 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -1,8 +1,12 @@
-obj-$(CONFIG_RMI4_BUS) += rmi_bus.o
-obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
-obj-$(CONFIG_RMI4_GENERIC) += rmi_driver.o rmi_f01.o
+obj-$(CONFIG_RMI4_CORE) += rmi_core.o
+rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
+
+# Function drivers
 obj-$(CONFIG_RMI4_F11) += rmi_f11.o
 
+# Transports
+obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
+
 ccflags-$(CONFIG_RMI4_DEBUG) += -DDEBUG
 
 ifeq ($(KERNELRELEASE),)
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index a912349..47cf0d5 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -206,6 +206,27 @@ static int __init rmi_bus_init(void)
 
mutex_init(&rmi_bus_mutex);
 
+   error = bus_register(&rmi_bus_type);
+   if (error) {
+   pr_err("%s: error registering the RMI bus: %d\n",
+   __func__, error);
+   return error;
+   }
+
+   error = rmi_register_f01_handler();
+   if (error) {
+   pr_err("%s: error registering the RMI F01 handler: %d\n",
+   __func__, error);
+   goto err_unregister_bus;
+   }
+
+   error = rmi_register_sensor_driver();
+   if (error) {
+   pr_err("%s: error registering the RMI sensor driver: %d\n",
+   __func__, error);
+   goto err_unregister_f01;
+   }
+
if (IS_ENABLED(CONFIG_RMI4_DEBUG)) {
rmi_debugfs_root = debugfs_create_dir(rmi_bus_type.name, NULL);
if (!rmi_debugfs_root)
@@ -218,24 +239,26 @@ static int __init rmi_bus_init(void)
}
}
 
-   error = bus_register(&rmi_bus_type);
-   if (error < 0) {
-   pr_err("%s: error registering the RMI bus: %d\n", __func__,
-  error);
-   return error;
-   }
-   pr_debug("%s: successfully registered RMI bus.\n", __func__);
-
return 0;
+
+err_unregister_f01:
+   rmi_unregister_f01_handler();
+err_unregister_bus:
+   bus_unregister(&rmi_bus_type);
+   return error;
 }
 
 static void __exit rmi_bus_exit(void)
 {
-   /* We should only ever get here if all drivers are unloaded, so
+   /*
+* We should only ever get here if all dri

[PATCH 3/4] Input: RMI4 - move function registration into the core

2012-11-27 Thread Dmitry Torokhov
This saves on boilerplate code that is needed in individual modules
implementing function handlers.

Also converted bool bitfields back to u8.

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/rmi4/rmi_bus.c| 189 ++
 drivers/input/rmi4/rmi_driver.c |   2 +-
 drivers/input/rmi4/rmi_driver.h |   6 +-
 drivers/input/rmi4/rmi_f01.c|  57 ++---
 drivers/input/rmi4/rmi_f11.c| 250 +---
 include/linux/rmi.h |   9 ++
 6 files changed, 271 insertions(+), 242 deletions(-)

diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index 47cf0d5..acbfd3d 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -73,51 +73,6 @@ static atomic_t physical_device_count = ATOMIC_INIT(0);
 static struct dentry *rmi_debugfs_root;
 #endif
 
-#ifdef CONFIG_PM
-static int rmi_bus_suspend(struct device *dev)
-{
-   struct device_driver *driver = dev->driver;
-   const struct dev_pm_ops *pm;
-
-   if (!driver)
-   return 0;
-
-   pm = driver->pm;
-   if (pm && pm->suspend)
-   return pm->suspend(dev);
-   if (driver->suspend)
-   return driver->suspend(dev, PMSG_SUSPEND);
-
-   return 0;
-}
-
-static int rmi_bus_resume(struct device *dev)
-{
-   struct device_driver *driver = dev->driver;
-   const struct dev_pm_ops *pm;
-
-   if (!driver)
-   return 0;
-
-   pm = driver->pm;
-   if (pm && pm->resume)
-   return pm->resume(dev);
-   if (driver->resume)
-   return driver->resume(dev);
-
-   return 0;
-}
-#endif
-
-static SIMPLE_DEV_PM_OPS(rmi_bus_pm_ops,
-rmi_bus_suspend, rmi_bus_resume);
-
-struct bus_type rmi_bus_type = {
-   .name   = "rmi",
-   .pm = &rmi_bus_pm_ops
-};
-EXPORT_SYMBOL_GPL(rmi_bus_type);
-
 static void release_rmidev_device(struct device *dev)
 {
device_unregister(dev);
@@ -184,6 +139,142 @@ void rmi_unregister_phys_device(struct rmi_phys_device 
*phys)
 EXPORT_SYMBOL(rmi_unregister_phys_device);
 
 /**
+ * rmi_register_function_handler - register a handler for an RMI function
+ * @handler: RMI handler that should be registered.
+ * @module: pointer to module that implements the handler
+ * @mod_name: name of the module implementing the handler
+ *
+ * This function performs additional setup of RMI function handler and
+ * registers it with the RMI core so that it can be bound to
+ * RMI function devices.
+ */
+int __rmi_register_function_handler(struct rmi_function_handler *handler,
+struct module *owner,
+const char *mod_name)
+{
+   int error;
+
+   handler->driver.bus = &rmi_bus_type;
+   handler->driver.owner = owner;
+   handler->driver.mod_name = mod_name;
+
+   error = driver_register(&handler->driver);
+   if (error) {
+   pr_err("driver_register() failed for %s, error: %d\n",
+   handler->driver.name, error);
+   return error;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(__rmi_register_function_handler);
+
+/**
+ * rmi_unregister_function_handler - unregister given RMI function handler
+ * @handler: RMI handler that should be unregistered.
+ *
+ * This function unregisters given function handler from RMI core which
+ * causes it to be unbound from the function devices.
+ */
+void rmi_unregister_function_handler(struct rmi_function_handler *handler)
+{
+   driver_unregister(&handler->driver);
+}
+EXPORT_SYMBOL(rmi_unregister_function_handler);
+
+
+static int rmi_function_match(struct device *dev, struct device_driver *drv)
+{
+   struct rmi_function_handler *handler;
+   struct rmi_function *fn;
+
+   if (dev->type != &rmi_function_type)
+   return 0;
+
+   if (drv == &rmi_sensor_driver.driver)
+   return 0;
+
+   fn = to_rmi_function(dev);
+   handler = to_rmi_function_handler(drv);
+
+   return fn->fd.function_number == handler->func;
+}
+
+static int rmi_function_probe(struct device *dev)
+{
+   struct rmi_function *fn = to_rmi_function(dev);
+   struct rmi_function_handler *handler =
+   to_rmi_function_handler(dev->driver);
+   int error;
+
+   if (handler->probe) {
+   error = handler->probe(fn);
+   return error;
+   }
+
+   return 0;
+}
+
+static int rmi_function_remove(struct device *dev)
+{
+   struct rmi_function *fn = to_rmi_function(dev);
+   struct rmi_function_handler *handler =
+   to_rmi_function_handler(dev->driver);
+
+   if (handler->remove)
+   handler->remove(fn);
+
+   return 0;
+}
+

[PATCH 4/4] Input: RMI4 - introduce rmi_module_driver() macro

2012-11-27 Thread Dmitry Torokhov
This also allows us to cut down on the boilerplate code in the function
handler modules.

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/rmi4/rmi_f11.c | 13 +
 include/linux/rmi.h  | 14 ++
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index dbb6060..8457ab4 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -2756,18 +2756,7 @@ static struct rmi_function_handler rmi_f11_handler = {
 #endif  /* defined(CONFIG_HAS_EARLYSUSPEND) */
 };
 
-static int __init rmi_f11_module_init(void)
-{
-   return rmi_register_function_handler(&rmi_f11_handler);
-}
-
-static void __exit rmi_f11_module_exit(void)
-{
-   rmi_unregister_function_handler(&rmi_f11_handler);
-}
-
-module_init(rmi_f11_module_init);
-module_exit(rmi_f11_module_exit);
+module_rmi_driver(rmi_f11_handler);
 
 MODULE_AUTHOR("Christopher Heiny http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH 05/06] input/rmi4: F01 - device control

2012-11-27 Thread Dmitry Torokhov
On Mon, Nov 26, 2012 at 02:31:27PM -0800, Christopher Heiny wrote:
> On 11/26/2012 01:40 AM, Dmitry Torokhov wrote:
> >Hi Christopher,
> >
> >On Fri, Nov 16, 2012 at 07:58:53PM -0800, Christopher Heiny wrote:
> >>RMI Function 01 implements basic device control and power management
> >>behaviors for the RMI4 sensor.
> >>
> >>rmi_f01.h exports definitions that we expect to be used by other 
> >>functionality
> >>in the future (such as firmware reflash).
> >
> >Please see my comments below.
> 
> Hi Dmitry,
> 
> Thanks for the feedback and the patch.  I've got just one question,
> included below, with a bunch of snipping).
> 
>   Chris
> 
> >
> >>
> >>
> >>Signed-off-by: Christopher Heiny 
> >>
> >>Cc: Dmitry Torokhov 
> >>Cc: Linus Walleij 
> >>Cc: Naveen Kumar Gaddipati 
> >>Cc: Joeri de Gram 
> >>
> >>
> >>---
> >>
> >>  drivers/input/rmi4/rmi_f01.c | 1348 
> >> ++
> >>  drivers/input/rmi4/rmi_f01.h |  160 +
> >>  2 files changed, 1508 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> >>new file mode 100644
> >>index 000..038266c
> >>--- /dev/null
> >>+++ b/drivers/input/rmi4/rmi_f01.c
> >>@@ -0,0 +1,1348 @@
> >>+/*
> >>+ * Copyright (c) 2011-2012 Synaptics Incorporated
> >>+ * Copyright (c) 2011 Unixphere
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify
> >>+ * it under the terms of the GNU General Public License as published by
> >>+ * the Free Software Foundation; either version 2 of the License, or
> >>+ * (at your option) any later version.
> 
> [snip]
> 
> >>+/**
> >>+ * @reset - set this bit to force a firmware reset of the sensor.
> >>+ */
> >>+struct f01_device_commands {
> >>+   bool reset:1;
> >>+   u8 reserved:7;
> >
> >When specifying bitwise fields please use u8, u16, etc only.
> 
> Um, OK.  Previously patch feedback suggested to use bool instead of
> u8 for single bit fields (see here:
> http://www.spinics.net/lists/linux-input/msg22198.html).  So I'm a
> little confused.  It's no big deal to change it back, but I'd like
> confirmation that it is really what we should do.

I believe that it is better to specify exact bitness of the base type of
the bitfield so you do not surprised by the alignment.

Thanks.

-- 
Dmitry
--
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/


[PATCH 2/2] Input: RMI4 - F11 - remove relreport attribute

2012-11-27 Thread Dmitry Torokhov
This data item does not seem to be used anywhere.

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/rmi4/rmi_f11.c | 37 -
 include/linux/rmi.h  |  1 -
 2 files changed, 38 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 717e2d8..da3fd2a 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -832,42 +832,6 @@ enum finger_state_values {
F11_RESERVED= 0x03
 };
 
-static ssize_t rmi_f11_relreport_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
-   struct rmi_function *fn = to_rmi_function(dev);
-   struct f11_data *data = fn->data;
-
-   return snprintf(buf, PAGE_SIZE, "%u\n",
-   data->sensors[0].axis_align.rel_report_enabled);
-}
-
-static ssize_t rmi_f11_relreport_store(struct device *dev,
-  struct device_attribute *attr,
-  const char *buf,
-  size_t count)
-{
-   struct rmi_function *fn = to_rmi_function(dev);
-   struct f11_data *data = fn->data;
-   unsigned int new_value;
-   int error;
-
-   error = kstrtouint(buf, 0, &new_value);
-   if (error)
-   return error;
-
-   if (new_value > 1)
-   return -ERANGE;
-
-   data->sensors[0].axis_align.rel_report_enabled = new_value;
-
-   return count;
-}
-
-static DEVICE_ATTR(relreport, RMI_RW_ATTR,
-  rmi_f11_relreport_show, rmi_f11_relreport_store);
-
 static ssize_t rmi_f11_rezero_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
@@ -906,7 +870,6 @@ static ssize_t rmi_f11_rezero_store(struct device *dev,
 static DEVICE_ATTR(rezero, RMI_WO_ATTR, NULL, rmi_f11_rezero_store);
 
 static struct attribute *rmi_f11_attrs[] = {
-   &dev_attr_relreport.attr,
&dev_attr_rezero.attr,
NULL
 };
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index d4e1438..3969e61 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -73,7 +73,6 @@ enum rmi_attn_polarity {
  *   automatically enabled for this sensor.
  */
 struct rmi_f11_2d_axis_alignment {
-   bool rel_report_enabled;
u32 swap_axes;  /* boolean, but u32 is needed by debugfs API */
u32 flip_x; /* boolean */
u32 flip_y; /* boolean */
-- 
1.7.11.7

--
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/


[PATCH 1/2] Input: RMI4 - rework F11 sysfs and debugfs attributes

2012-11-27 Thread Dmitry Torokhov
Avoid rolling our own debugfs operations and use the standar ones instead.
Also switch to using attribute groups to create sysfs attributes.

The max_x and max_y can be retrieced via EVIOGABS and so attributes moved
over to debugfs.

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/rmi4/rmi_f11.c | 918 +--
 include/linux/rmi.h  |  20 +-
 2 files changed, 186 insertions(+), 752 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 8457ab4..717e2d8 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -20,6 +20,7 @@
 #define FUNCTION_DATA f11_data
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -29,12 +30,6 @@
 #include 
 #include "rmi_driver.h"
 
-#ifdef CONFIG_RMI4_DEBUG
-#include 
-#include 
-#include 
-#endif
-
 #define F11_MAX_NUM_OF_SENSORS 8
 #define F11_MAX_NUM_OF_FINGERS 10
 #define F11_MAX_NUM_OF_TOUCH_SHAPES16
@@ -787,19 +782,7 @@ struct f11_2d_data {
  * @input - input device for absolute pointing stream
  * @mouse_input - input device for relative pointing stream.
  * @input_phys - buffer for the absolute phys name for this sensor.
- * @input_mouse_phys - buffer for the relative phys name for this sensor.
- * @debugfs_flip - inverts one or both axes.  Useful in prototyping new
- * systems.
- * @debugfs_flip - coordinate clipping range for one or both axes.  Useful in
- * prototyping new systems.
- * @debugfs_delta_threshold - adjusts motion sensitivity for relative reports
- * and (in reduced reporting mode) absolute reports.  Useful in prototyping new
- * systems.
- * @debugfs_offset - offsets one or both axes.  Useful in prototyping new
- * systems.
- * @debugfs_swap - swaps X and Y axes.  Useful in prototyping new systems.
- * @debugfs_type_a - forces type A behavior.  Useful in bringing up old systems
- * when you're not sure if you've got a Type A or Type B sensor.
+ * @input_phys_mouse - buffer for the relative phys name for this sensor.
  */
 struct f11_2d_sensor {
struct rmi_f11_2d_axis_alignment axis_align;
@@ -811,22 +794,13 @@ struct f11_2d_sensor {
u8 *data_pkt;
int pkt_size;
u8 sensor_index;
-   bool type_a;
+   u32 type_a; /* boolean but debugfs API requires u32 */
enum rmi_f11_sensor_type sensor_type;
struct input_dev *input;
struct input_dev *mouse_input;
struct rmi_function *fn;
char input_phys[NAME_BUFFER_SIZE];
char input_phys_mouse[NAME_BUFFER_SIZE];
-
-#ifdef CONFIG_RMI4_DEBUG
-   struct dentry *debugfs_flip;
-   struct dentry *debugfs_clip;
-   struct dentry *debugfs_delta_threshold;
-   struct dentry *debugfs_offset;
-   struct dentry *debugfs_swap;
-   struct dentry *debugfs_type_a;
-#endif
 };
 
 /** Data pertaining to F11 in general.  For per-sensor data, see struct
@@ -849,10 +823,6 @@ struct f11_data {
struct mutex dev_controls_mutex;
u16 rezero_wait_ms;
struct f11_2d_sensor sensors[F11_MAX_NUM_OF_SENSORS];
-
-#ifdef CONFIG_RMI4_DEBUG
-   struct dentry *debugfs_rezero_wait;
-#endif
 };
 
 enum finger_state_values {
@@ -862,71 +832,56 @@ enum finger_state_values {
F11_RESERVED= 0x03
 };
 
-static ssize_t f11_maxPos_show(struct device *dev,
-struct device_attribute *attr,
-char *buf)
-{
-   struct rmi_function *fn;
-   struct f11_data *data;
-
-   fn = to_rmi_function(dev);
-   data = fn->data;
-
-   return snprintf(buf, PAGE_SIZE, "%u %u\n",
-   data->sensors[0].max_x, data->sensors[0].max_y);
-}
-
-static ssize_t f11_relreport_show(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
+static ssize_t rmi_f11_relreport_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
 {
-   struct rmi_function *fn;
-   struct f11_data *instance_data;
-
-   fn = to_rmi_function(dev);
-   instance_data = fn->data;
+   struct rmi_function *fn = to_rmi_function(dev);
+   struct f11_data *data = fn->data;
 
return snprintf(buf, PAGE_SIZE, "%u\n",
-   instance_data->
-   sensors[0].axis_align.rel_report_enabled);
+   data->sensors[0].axis_align.rel_report_enabled);
 }
 
-static ssize_t f11_relreport_store(struct device *dev,
-struct device_attribute *attr,
-const char *buf,
-size_t count)
+static ssize_t rmi_f11_relreport_store(struct device *dev,
+  struct device_attribute 

Re: [PATCH 3/3] Input: bu21013_ts - Add support for Device Tree booting

2012-11-27 Thread Dmitry Torokhov
Hi Lee,

On Tue, Nov 27, 2012 at 01:13:10PM +, Lee Jones wrote:
> Now we can register the BU21013_ts touch screen when booting with
> Device Tree enabled. Here we parse all the necessary components
> previously expected to be passed from platform data.

I applied these 3 patches, but for DT we also need to specify compatible
ID and set up of_match_table pointer. Please send me a follow-up patches
doing that and also describing DT bindings for BU21013.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 1/4] Input RMI4 - rename rmi_function_container to rmi_function

2012-11-27 Thread Dmitry Torokhov
On Tue, Nov 27, 2012 at 03:43:05PM -0800, Christopher Heiny wrote:
> On 11/27/2012 01:21 AM, Dmitry Torokhov wrote:
> >To save my old fingers...
> >
> >Signed-off-by: Dmitry Torokhov
> >---
> >
> >It looks like this driver(s) need some love and I might have some time so I
> >will refresh my "synaptics" branch with the patches you have sent and start
> >working off it. If you have updates I would appreciate if you also make them
> >available relative to that branch. When we are ready we'll squash them all
> >together and apply to the official branch.
> 
> No problem - let me know which branch/tag to work with, and we'll be
> happy to patch against that for the next round.

synaptics-rmi4. It is based off 3.7-rc6.


> 
> >
> >Thanks.
> >
> >  drivers/input/rmi4/rmi_driver.c | 158 +++--
> >  drivers/input/rmi4/rmi_driver.h |   4 +-
> >  drivers/input/rmi4/rmi_f01.c| 298 
> > 
> >  drivers/input/rmi4/rmi_f11.c| 258 +-
> >  include/linux/rmi.h |  22 ++-
> >  5 files changed, 368 insertions(+), 372 deletions(-)
> >
> >diff --git a/drivers/input/rmi4/rmi_driver.c 
> >b/drivers/input/rmi4/rmi_driver.c
> >index 05a73ae..e8a4b52 100644
> >--- a/drivers/input/rmi4/rmi_driver.c
> >+++ b/drivers/input/rmi4/rmi_driver.c
> >@@ -594,7 +594,7 @@ static struct device_attribute bsr_attribute = 
> >__ATTR(bsr, RMI_RW_ATTR,
> >
> >  static void rmi_free_function_list(struct rmi_device *rmi_dev)
> >  {
> >-struct rmi_function_container *entry, *n;
> >+struct rmi_function *entry, *n;
> > struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> >
> > if (!data) {
> >@@ -613,44 +613,44 @@ static void rmi_free_function_list(struct rmi_device 
> >*rmi_dev)
> > }
> >  }
> >
> >-static void release_fc_device(struct device *dev)
> >+static void release_function_device(struct device *dev)
> >  {
> > dev_dbg(dev, "REMOVING KOBJ!");
> > kobject_put(&dev->kobj);
> >  }
> 
> Hmmm.  Since rmi_function_container has evolved into a child device
> of the RMI4 module, maybe it would be better renamed
> rmi_function_device or rmi_function_dev?  I find this clearer, but
> can live with just rmi_function if you prefer that.

I just prefer something reasonably short so I still like rmi_function or
something of similar length.

> 
> 
> Similarly, rmi_function_handler has evolved into a driver for such
> devices, so perhaps it should be renamed rmi_function_driver?

No preference here.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 2/4] Input: RMI4 - move sensor driver and F01 handler into the core

2012-11-29 Thread Dmitry Torokhov
Hi Chris,
On Wed, Nov 28, 2012 at 08:54:32PM -0800, Christopher Heiny wrote:
> On 11/27/2012 01:21 AM, Dmitry Torokhov wrote:
> >There is no point in having the sensor driver and F01 handler separate
> >from the RMI core since it is not useful without them and having them
> >all together simplifies initialization among other things.
> 
> Hi Dmitry,
> 
> I've been looking at this patch as well as your patch 3/4 changes,
> and I'm not sure it's for the better.
> 
> One thing that confuses me is that these appear to go against the
> advice we've been getting over the past months to rely more on
> standard kernel bus and driver implementations, instead of the
> "roll-your-own" implementation we had been using before.
> 
> More importantly, the patches inextricably link the sensor driver
> implementation and the F01 driver implementation to the bus
> implementation, and means that any given system can have only one
> way of managing F01.  As you observed, a sensor is pretty much
> useless without an F01 handler, but I am reasonably sure that there
> will be future systems that have more than one RMI4 sensor in them,
> and there is a strong possibility that these sensors may have
> different requirements for handling F01.  In the near future, then,
> these changes will have to be refactored back to something more like
> the structure of our 2012/11/16 patch set.
> 
> Additionally, having F01 as a special case means that when we start
> implementing things such as support for request_firmware(), there
> will have to be a bunch of special case code to deal with F01, since
> it's no longer "just another function driver".  That seems to go in
> exactly the opposite direction of the simplification that you're
> trying to achieve.

But F01 continues to being "just another function driver" even with my
changes. It is still registered as rmi_fucntion_handler and uses
standard matching mechanisms to bind to rmi_functions registered by the
sensor driver. What I changed is the fact that rmi_f01 is no longer a
separate module which could be loaded after loading rmi_bus and it can't
be unloaded without unloading rmi_bus. This simplifies things and makes
it easier to have rmi core compiled as a module.

Also I do not quite follow your idea that devices might have different
requirements for handling F01. If that is true then be _can't_ implement
"F01" as "another function driver"... But that is orthogonal for the 3/4
change we are discussing here.

Thanks.

-- 
Dmitry
--
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/


[PATCH] Input: gpio_keys_polled - switch to using gpio_request_one()

2012-11-29 Thread Dmitry Torokhov
This saves us a few lines of code.

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/keyboard/gpio_keys_polled.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c 
b/drivers/input/keyboard/gpio_keys_polled.c
index d72d0e5..f686fd9 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -270,22 +270,14 @@ static int gpio_keys_polled_probe(struct platform_device 
*pdev)
goto err_free_gpio;
}
 
-   error = gpio_request(gpio,
-button->desc ? button->desc : DRV_NAME);
+   error = gpio_request_one(gpio, GPIOF_IN,
+button->desc ?: DRV_NAME);
if (error) {
dev_err(dev, "unable to claim gpio %u, err=%d\n",
gpio, error);
goto err_free_gpio;
}
 
-   error = gpio_direction_input(gpio);
-   if (error) {
-   dev_err(dev,
-   "unable to set direction on gpio %u, err=%d\n",
-   gpio, error);
-   goto err_free_gpio;
-   }
-
bdata->can_sleep = gpio_cansleep(gpio);
bdata->last_state = -1;
bdata->threshold = DIV_ROUND_UP(button->debounce_interval,
-- 
1.7.11.7


-- 
Dmitry
--
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/


[PATCH] Input: gpio_keys - switch to using gpio_request_one()

2012-11-29 Thread Dmitry Torokhov
This saves us a few lines of code.

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/keyboard/gpio_keys.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c 
b/drivers/input/keyboard/gpio_keys.c
index 79435de..d327f5a 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -440,21 +440,13 @@ static int gpio_keys_setup_key(struct platform_device 
*pdev,
 
if (gpio_is_valid(button->gpio)) {
 
-   error = gpio_request(button->gpio, desc);
+   error = gpio_request_one(button->gpio, GPIOF_IN, desc);
if (error < 0) {
dev_err(dev, "Failed to request GPIO %d, error %d\n",
button->gpio, error);
return error;
}
 
-   error = gpio_direction_input(button->gpio);
-   if (error < 0) {
-   dev_err(dev,
-   "Failed to configure direction for GPIO %d, 
error %d\n",
-   button->gpio, error);
-   goto fail;
-   }
-
if (button->debounce_interval) {
error = gpio_set_debounce(button->gpio,
button->debounce_interval * 1000);
-- 
1.7.11.7


-- 
Dmitry
--
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/


Re: [PATCH] input: drop unnecessary dependencies on TTY

2013-02-06 Thread Dmitry Torokhov
On Tue, Feb 05, 2013 at 05:58:34PM -0800, Joe Millenbach wrote:
> Backing out changes made in earlier TTY removal patch.  Switched
> to only one dependency in SERPORT on TTY instead of many incorrect
> dependencies.
> 
> Signed-off-by: Joe Millenbach 
> Reported-by: Dmitry Torokhov 

Acked-by: Dmitry Torokhov 

Thanks for doing this!

-- 
Dmitry
--
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/


Re: [Pv-drivers] [PATCH 0/1] VM Sockets for Linux upstreaming

2013-02-08 Thread Dmitry Torokhov
Hi David,

On Wed, Feb 06, 2013 at 04:23:55PM -0800, Andy King wrote:
> In an effort to improve the out-of-the-box experience with Linux kernels for
> VMware users, VMware is working on readying the VM Sockets (VSOCK, formerly
> VMCI Sockets) (vsock) kernel module for inclusion in the Linux kernel. The
> purpose of this post is to acquire feedback on the vsock kernel module.
> 
> Unlike previous submissions, where the new socket family was entirely reliant
> on VMware's VMCI PCI device (and thus VMware's hypervisor), VM Sockets is now
> completely[1] separated out into two parts, each in its own module:
> 
> o Core socket code, which is transport-neutral and invokes transport
>   callbacks to communicate with the hypervisor.  This is vsock.ko.
> o A VMCI transport, which communicates over VMCI with the VMware hypervisor.
>   This is vmw_vsock_vmci_transport.ko, and it registers with the core module
>   as a transport.
> 
> This should provide a path to introducing additional transports, for example
> virtio, with the ultimate goal being to make this new socket family
> hypervisor-neutral.

As Andy mentioned in another e-mail, we would like very much to get
vsock in 3.9 release, so now that it is split into hypervisor neutral
and transport parts is there any high level issues that we need to
resolve before the code can be accepted?

Thanks,
Dmitry
--
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/


Re: [PATCH] input: don't call input_dev_release_keys() in resume

2013-04-04 Thread Dmitry Torokhov
Hi Oskar,

On Thu, Mar 07, 2013 at 03:01:22PM +0100, oskar.and...@sonymobile.com wrote:
> From: Aleksej Makarov 
> 
> When waking up the platform by pressing a specific key, sending a
> release on that key makes it impossible to react on the event in
> user-space.
> 

No, we can not simply not release keys after resume from suspend, as
this leads to keys being stuck. Consider you are holding an 'I' key on
your external USB keyboard and close your laptop's lid. Then you release
the key and leave. Later you come back, open the lid waking the laptop
and observe endless stream of 'I' in your open terminal.

Maybe we should release the keys during suspend time? I am not sure how
Android infrastructure will react to this though...

Thanks.

-- 
Dmitry
--
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/


Re: linux-next: manual merge of the tty tree with the input tree

2013-01-28 Thread Dmitry Torokhov
On Mon, Jan 28, 2013 at 02:09:31PM -0800, Joe Millenbach wrote:
> On Mon, Jan 28, 2013 at 9:33 AM, Dmitry Torokhov
>  wrote:
> > On Mon, Jan 28, 2013 at 06:46:15AM -0800, Greg KH wrote:
> >> On Mon, Jan 28, 2013 at 08:44:24PM +1100, Stephen Rothwell wrote:
> >> > Hi Greg,
> >> >
> >> > Today's linux-next merge of the tty tree got a conflict in
> >> > drivers/input/keyboard/Kconfig between commit 6f2ac009f29b ("Input:
> >> > goldfish - virtual input event driver") from the input tree and commit
> >> > 4f73bc4dd3e8 ("tty: Added a CONFIG_TTY option to allow removal of TTY")
> >> > from the tty tree.
> >> >
> >> > I fixed it up (see below - I am not sure if GOLDFISH_EVENTS needs TTY or
> >> > not) and can carry the fix as necessary (no action is required).
> >> >
> >> > --
> >> > Cheers,
> >> > Stephen Rothwells...@canb.auug.org.au
> >> >
> >> > diff --cc drivers/input/keyboard/Kconfig
> >> > index 078305e,008f96a..000
> >> > --- a/drivers/input/keyboard/Kconfig
> >> > +++ b/drivers/input/keyboard/Kconfig
> >> > @@@ -479,16 -482,8 +482,18 @@@ config KEYBOARD_SAMSUN
> >> >   To compile this driver as a module, choose M here: the
> >> >   module will be called samsung-keypad.
> >> >
> >> > + if TTY
> >> > +
> >> >  +config KEYBOARD_GOLDFISH_EVENTS
> >> >  +  depends on GOLDFISH
> >> >  +  tristate "Generic Input Event device for Goldfish"
> >> >  +  help
> >> >  +Say Y here to get an input event device for the Goldfish virtual
> >>
> >> Looks good, thanks.
> >
> > Greg,
> >
> > Please drop 4f73bc4dd3e8563ef4109f293a092820dff66d92, at least the parts
> > related to input. As far as I know nothing except serport driver
> > depends on tty and we do not need to introduce this kind of
> > dependencie. Anyone needing slim config can simply try disabling
> > input (or parts of it) without needing an artificial dependencies.
> >
> > Thanks.
> >
> > --
> > Dmitry
> 
> Dmitry and Greg,
> 
> SERIO needs TTY, 

No it does not. There is only one (1) serio driver that needs the tty
layer and that is serport.

> and the majority of the input changes are adding "depends on TTY" to
> things that "select SERIO" as they break the dependency chain.  In
> other words, enabling a component that selects SERIO will turn SERIO
> on even when SERIO depends on TTY and TTY is disabled.

Except that it does not. Are you confusing SERIO with SERIAL by any
chance?

In the future it would be nice if you CCed people involved in the
subsystem you are changing.

Thanks.

-- 
Dmitry
--
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/


Re: linux-next: manual merge of the tty tree with the input tree

2013-01-28 Thread Dmitry Torokhov
On Tue, Jan 29, 2013 at 10:59:17AM +1100, Josh Triplett wrote:
> On Mon, Jan 28, 2013 at 02:44:43PM -0800, Dmitry Torokhov wrote:
> > On Mon, Jan 28, 2013 at 02:09:31PM -0800, Joe Millenbach wrote:
> > > On Mon, Jan 28, 2013 at 9:33 AM, Dmitry Torokhov
> > >  wrote:
> > > > On Mon, Jan 28, 2013 at 06:46:15AM -0800, Greg KH wrote:
> > > >> On Mon, Jan 28, 2013 at 08:44:24PM +1100, Stephen Rothwell wrote:
> > > >> > Hi Greg,
> > > >> >
> > > >> > Today's linux-next merge of the tty tree got a conflict in
> > > >> > drivers/input/keyboard/Kconfig between commit 6f2ac009f29b ("Input:
> > > >> > goldfish - virtual input event driver") from the input tree and 
> > > >> > commit
> > > >> > 4f73bc4dd3e8 ("tty: Added a CONFIG_TTY option to allow removal of 
> > > >> > TTY")
> > > >> > from the tty tree.
> > > >> >
> > > >> > I fixed it up (see below - I am not sure if GOLDFISH_EVENTS needs 
> > > >> > TTY or
> > > >> > not) and can carry the fix as necessary (no action is required).
> > > >> >
> > > >> > --
> > > >> > Cheers,
> > > >> > Stephen Rothwells...@canb.auug.org.au
> > > >> >
> > > >> > diff --cc drivers/input/keyboard/Kconfig
> > > >> > index 078305e,008f96a..000
> > > >> > --- a/drivers/input/keyboard/Kconfig
> > > >> > +++ b/drivers/input/keyboard/Kconfig
> > > >> > @@@ -479,16 -482,8 +482,18 @@@ config KEYBOARD_SAMSUN
> > > >> >   To compile this driver as a module, choose M here: the
> > > >> >   module will be called samsung-keypad.
> > > >> >
> > > >> > + if TTY
> > > >> > +
> > > >> >  +config KEYBOARD_GOLDFISH_EVENTS
> > > >> >  +  depends on GOLDFISH
> > > >> >  +  tristate "Generic Input Event device for Goldfish"
> > > >> >  +  help
> > > >> >  +Say Y here to get an input event device for the Goldfish 
> > > >> > virtual
> > > >>
> > > >> Looks good, thanks.
> > > >
> > > > Greg,
> > > >
> > > > Please drop 4f73bc4dd3e8563ef4109f293a092820dff66d92, at least the parts
> > > > related to input. As far as I know nothing except serport driver
> > > > depends on tty and we do not need to introduce this kind of
> > > > dependencie. Anyone needing slim config can simply try disabling
> > > > input (or parts of it) without needing an artificial dependencies.
> > > >
> > > > Thanks.
> > > >
> > > > --
> > > > Dmitry
> > > 
> > > Dmitry and Greg,
> > > 
> > > SERIO needs TTY, 
> > 
> > No it does not. There is only one (1) serio driver that needs the tty
> > layer and that is serport.
> > 
> > > and the majority of the input changes are adding "depends on TTY" to
> > > things that "select SERIO" as they break the dependency chain.  In
> > > other words, enabling a component that selects SERIO will turn SERIO
> > > on even when SERIO depends on TTY and TTY is disabled.
> > 
> > Except that it does not. Are you confusing SERIO with SERIAL by any
> > chance?
> 
> A few serial drivers don't actually need the TTY layer.  However, most

Can you please tell me why you are talking about serial layer here?

> do, including many that don't obviously appear to at first glance.  For
> instance, MOUSE_PS2 doesn't *appear* to need TTY, but without the
> dependency, having MOUSE_PS2 enabled and TTY disabled produces a kernel
> that doesn't build.

Compile log please.

>  (Also keep in mind that many other headers include
> .)  Many of the drivers that don't actually need TTY
> nonetheless won't typically appear on systems small enough to want to
> compile out TTY.
> 
> In any case, it seems simple enough to whittle down dependencies on TTY
> later on, but for a first pass, the conserative approach seems
> preferable.

However my approach would be not to touch anything except serport
driver (which indeed depends on TTY layer).

> 
> > In the future it would be nice if you CCed people involved in the
> > subsystem you are changing.
> 
> Such as the maintainers of the TTY and serial layers?  Alan Cox OKed
> this conservative addition of dependencies in the original version of
> this change, and Greg had no complaints at the time.

Maintainer of _SERIO_ (not SERIAL, SERIO) layer, yours truly.

-- 
Dmitry
--
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/


Re: linux-next: manual merge of the tty tree with the input tree

2013-01-28 Thread Dmitry Torokhov
On Tue, Jan 29, 2013 at 04:13:21PM +1100, Josh Triplett wrote:
> On Mon, Jan 28, 2013 at 04:23:57PM -0800, Dmitry Torokhov wrote:
> > On Tue, Jan 29, 2013 at 10:59:17AM +1100, Josh Triplett wrote:
> > > On Mon, Jan 28, 2013 at 02:44:43PM -0800, Dmitry Torokhov wrote:
> > > > On Mon, Jan 28, 2013 at 02:09:31PM -0800, Joe Millenbach wrote:
> > > > > On Mon, Jan 28, 2013 at 9:33 AM, Dmitry Torokhov
> > > > >  wrote:
> > > > > > On Mon, Jan 28, 2013 at 06:46:15AM -0800, Greg KH wrote:
> > > > > >> On Mon, Jan 28, 2013 at 08:44:24PM +1100, Stephen Rothwell wrote:
> > > > > >> > Hi Greg,
> > > > > >> >
> > > > > >> > Today's linux-next merge of the tty tree got a conflict in
> > > > > >> > drivers/input/keyboard/Kconfig between commit 6f2ac009f29b 
> > > > > >> > ("Input:
> > > > > >> > goldfish - virtual input event driver") from the input tree and 
> > > > > >> > commit
> > > > > >> > 4f73bc4dd3e8 ("tty: Added a CONFIG_TTY option to allow removal 
> > > > > >> > of TTY")
> > > > > >> > from the tty tree.
> > > > > >> >
> > > > > >> > I fixed it up (see below - I am not sure if GOLDFISH_EVENTS 
> > > > > >> > needs TTY or
> > > > > >> > not) and can carry the fix as necessary (no action is required).
> > > > > >> >
> > > > > >> > --
> > > > > >> > Cheers,
> > > > > >> > Stephen Rothwells...@canb.auug.org.au
> > > > > >> >
> > > > > >> > diff --cc drivers/input/keyboard/Kconfig
> > > > > >> > index 078305e,008f96a..000
> > > > > >> > --- a/drivers/input/keyboard/Kconfig
> > > > > >> > +++ b/drivers/input/keyboard/Kconfig
> > > > > >> > @@@ -479,16 -482,8 +482,18 @@@ config KEYBOARD_SAMSUN
> > > > > >> >   To compile this driver as a module, choose M here: the
> > > > > >> >   module will be called samsung-keypad.
> > > > > >> >
> > > > > >> > + if TTY
> > > > > >> > +
> > > > > >> >  +config KEYBOARD_GOLDFISH_EVENTS
> > > > > >> >  +  depends on GOLDFISH
> > > > > >> >  +  tristate "Generic Input Event device for Goldfish"
> > > > > >> >  +  help
> > > > > >> >  +Say Y here to get an input event device for the Goldfish 
> > > > > >> > virtual
> > > > > >>
> > > > > >> Looks good, thanks.
> > > > > >
> > > > > > Greg,
> > > > > >
> > > > > > Please drop 4f73bc4dd3e8563ef4109f293a092820dff66d92, at least the 
> > > > > > parts
> > > > > > related to input. As far as I know nothing except serport driver
> > > > > > depends on tty and we do not need to introduce this kind of
> > > > > > dependencie. Anyone needing slim config can simply try disabling
> > > > > > input (or parts of it) without needing an artificial dependencies.
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > > > --
> > > > > > Dmitry
> > > > > 
> > > > > Dmitry and Greg,
> > > > > 
> > > > > SERIO needs TTY, 
> > > > 
> > > > No it does not. There is only one (1) serio driver that needs the tty
> > > > layer and that is serport.
> > > > 
> > > > > and the majority of the input changes are adding "depends on TTY" to
> > > > > things that "select SERIO" as they break the dependency chain.  In
> > > > > other words, enabling a component that selects SERIO will turn SERIO
> > > > > on even when SERIO depends on TTY and TTY is disabled.
> > > > 
> > > > Except that it does not. Are you confusing SERIO with SERIAL by any
> > > > chance?
> > > 
> > > A few serial drivers don't actually need the TTY layer.  However, most
> > 
> > Can you please tell me why you are talking about serial layer here?
> 
> Probab

Re: linux-next: manual merge of the tty tree with the input tree

2013-01-28 Thread Dmitry Torokhov
Hi Joe,

On Mon, Jan 28, 2013 at 10:26:47PM -0800, Joe Millenbach wrote:
> On Mon, Jan 28, 2013 at 9:33 PM, Dmitry Torokhov
>  wrote:
> > Please revert the input changes and add *ONE* new dependency to the
> > serport driver.
> >
> > Thanks.
> >
> > --
> > Dmitry
> 
> Apologies on this.  I must have misunderstood the problem originally,
> and I definitely misunderstood your solution until I looked at it
> again.  I just went through applying the changes I'd created minus the
> driver/input changes, plus your SERPORT TTY dependency.  You were
> right that it solves the dependency issue and is much more elegant.  I
> thought it was going to leave dangling select dependencies for users
> to deal with later.  Sorry I was so hesitant at first.
> 
> I'll be making up a new patch for Greg after I get it reviewed.

Thank you for making the changes.

By the "dangling select dependencies" I assume you mean "select SERIO"
that several drivers do? "select SERIO" selects only serio core which is
self contained and has no additional dependencies [so far], that is why
it is being selected rather than being depended upon.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 04/05] input: RMI4 F01 device control

2013-01-31 Thread Dmitry Torokhov
Hi Chris,

On Fri, Jan 18, 2013 at 05:12:44PM -0800, Christopher Heiny wrote:
> In addition to the changes described in 0/0 of this patchset, this patch
> includes device serialization updated to conform to the latest RMI4
> specification.

I was looking at the various aspects of the RMI4 patchset, trying to
fix the issues that I see, but there is one big issue that I simply do
not have time to tackle - the driver is completely broken on big endian
architectures due to reliance on bitfileds when exchanging the data with
the device.

Consider the following structures:

>  struct f01_device_status {
> - u8 status_code:4;
> + enum rmi_device_status status_code:4;
>   u8 reserved:2;
>   u8 flash_prog:1;
>   u8 unconfigured:1;
> @@ -159,4 +136,113 @@ struct f01_device_control_0 {
>   u8 configured:1;
>  } __attribute__((__packed__));
>  
> +/**
> + * @reset - set this bit to force a firmware reset of the sensor.
> + */
> +struct f01_device_commands {
> + u8 reset:1;
> + u8 reserved:7;
> +};
> +

To make this work on BE boxes you either need to add #ifdefs to the
structures reversing the order of fields or use bit shifts and masks to
get/set specific bits in bytes.

I tried converting F01 code (you can see the [likely broken as I can't
test] result in my tree), but I really do not have time for F11.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH v3 0/2] Input: twl6040-vibra cleanups (devm_* conversion, system wq)

2013-01-25 Thread Dmitry Torokhov
Hi Peter,

On Thu, Jan 24, 2013 at 12:44:29PM +0100, Peter Ujfalusi wrote:
> Hi Dmitry,
> 
> On 01/14/2013 04:34 PM, Peter Ujfalusi wrote:
> > Hi Dmitry,
> > 
> > Changes since v2:
> > - twl4030-vibra patches are left out (they have been applied)
> > - Do not use devm_input_allocate_device() in twl6040-vibra
> 
> Do you want me to resend this two patch for 3.9?

No, I queued them for 3.9.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH -v2 2/2] x86: Make Linux guest support optional

2013-01-25 Thread Dmitry Torokhov
Hi Borislav,

On Fri, Jan 25, 2013 at 06:59:37PM +0100, Borislav Petkov wrote:
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 8e1a9ec53003..cb467656e684 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -724,7 +724,7 @@ static int __init hv_acpi_init(void)
>  {
>   int ret, t;
>  
> - if (x86_hyper != &x86_hyper_ms_hyperv)
> + if (!x86_hyper || strncmp(x86_hyper->name, "VMware", 6))
>   return -ENODEV;

I assume this is a typo as I doubt MS is using the same signature as we
do.

>  
>   init_completion(&probe_event);
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index cb56e270da11..85f15a6d8798 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -786,7 +786,7 @@ static int __init vmballoon_init(void)
>* Check if we are running on VMware's hypervisor and bail out
>* if we are not.
>*/
> - if (x86_hyper != &x86_hyper_vmware)
> + if (!x86_hyper || strncmp(x86_hyper->name, "VMware", 6))
>   return -ENODEV;
>

I wonder why you decided to switch from address matching (which is quite
precise and would potentially allow adding signatures without needing
to change the drivers) to string matching?

Thanks,
Dmitry
--
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/


Re: [PATCH -v2 2/2] x86: Make Linux guest support optional

2013-01-25 Thread Dmitry Torokhov
Hi Borislav,

On Fri, Jan 25, 2013 at 06:59:37PM +0100, Borislav Petkov wrote:
> From: Borislav Petkov 
> 
> Put all config options needed to run Linux as a guest behind a
> CONFIG_HYPERVISOR_GUEST menu so that they don't get built-in by
> default but selected by the user. Also, move x86_hyper into an
> unconditionally-built compilation unit because it is exported with the
> non-GPL flavour and we can't know whatever uses it on the outside.

I am confused by this statement... How can EXPORT_SYMBOL() vs
EXPORT_SYMBOL_GPL() help you determining if it is used out of tree or
not?

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH -v2 2/2] x86: Make Linux guest support optional

2013-01-25 Thread Dmitry Torokhov
On Fri, Jan 25, 2013 at 07:23:35PM +0100, Borislav Petkov wrote:
> On Fri, Jan 25, 2013 at 10:07:31AM -0800, Dmitry Torokhov wrote:
> > > - if (x86_hyper != &x86_hyper_ms_hyperv)
> > > + if (!x86_hyper || strncmp(x86_hyper->name, "VMware", 6))
> > >   return -ENODEV;
> > 
> > I assume this is a typo as I doubt MS is using the same signature as we
> > do.
> 
> Yeah, Vmware is assimilating Microsoft. :-)
> 
> Damn copy-paste crap. Sorry, will fix.
> 
> > > diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> > > index cb56e270da11..85f15a6d8798 100644
> > > --- a/drivers/misc/vmw_balloon.c
> > > +++ b/drivers/misc/vmw_balloon.c
> > > @@ -786,7 +786,7 @@ static int __init vmballoon_init(void)
> > >* Check if we are running on VMware's hypervisor and bail out
> > >* if we are not.
> > >*/
> > > - if (x86_hyper != &x86_hyper_vmware)
> > > + if (!x86_hyper || strncmp(x86_hyper->name, "VMware", 6))
> > >   return -ENODEV;
> > >
> > 
> > I wonder why you decided to switch from address matching (which is quite
> > precise and would potentially allow adding signatures without needing
> > to change the drivers) to string matching?
> 
> Ok, this is a bit more involved. If we put all hypervisor stuff behind
> CONFIG_HYPERVISOR_GUEST and in the cases where this is disabled,
> the build fails because the x86_hyper_vmware symbol is undefined.
> Alternatively, this doesn't happen when we look at the name.

Can't we make the balloon driver depend on CONFIG_HYPERVISOR_GUEST?

> 
> But why would you want to add signatures? Isn't "VMware" enough as a
> name for the vmware hypervisor and why would you need more? And besides,
> hypervisor_x86.name is only a single const char * so you'd need to
> change that struct to add more names...Hm.

We do not _need_ more and have no plans to introduce new ones AFAIK, but
I think that comparing an address of a structure instead of string
comparison is much more elegant and it is safer from typos.

Thanks,
Dmitry
--
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/


Re: [PATCH -v2 2/2] x86: Make Linux guest support optional

2013-01-25 Thread Dmitry Torokhov
On Fri, Jan 25, 2013 at 07:32:13PM +0100, Borislav Petkov wrote:
> On Fri, Jan 25, 2013 at 10:12:51AM -0800, Dmitry Torokhov wrote:
> > Hi Borislav,
> > 
> > On Fri, Jan 25, 2013 at 06:59:37PM +0100, Borislav Petkov wrote:
> > > From: Borislav Petkov 
> > > 
> > > Put all config options needed to run Linux as a guest behind a
> > > CONFIG_HYPERVISOR_GUEST menu so that they don't get built-in by
> > > default but selected by the user. Also, move x86_hyper into an
> > > unconditionally-built compilation unit because it is exported with the
> > > non-GPL flavour and we can't know whatever uses it on the outside.
> > 
> > I am confused by this statement... How can EXPORT_SYMBOL() vs
> > EXPORT_SYMBOL_GPL() help you determining if it is used out of tree or
> > not?
> 
> Maybe this wasn't formulated clear enough - I wanted to say that with
> the !GPL flavor, any code (proprietary included) can link to those
> symbols and we cannot know who uses it and if we hide that symbol,
> someone would probably come crying.
> 
> Because if we could, I would've done trivial accessor functions which
> would return NULL when CONFIG_HYPERVISOR_GUEST is not set so that those
> cases build and boot fine, and we wouldn't have the need to have this
> symbol always present (like is the case now and I had to move it to
> setup.c).
> 
> If it were _GPL, we would've fixed all its users to switch to the
> accessors.
> 
> Makes more sense?

No, not really as EXPORT_SYMBOL_GPL() in no way implies that the code
using it lives in the mainline. Also, EXPORT_SYMBOL() does not imply
that it forms an ABI and can't be changed ever.

Thanks.

-- 
Dmitry
--
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/


Re: [Pv-drivers] [PATCH 1/1] VSOCK: Introduce VM Sockets

2013-01-25 Thread Dmitry Torokhov
Hi Neil,

On Friday, January 25, 2013 06:59:53 PM Neil Horman wrote:
> On Fri, Jan 25, 2013 at 09:37:50AM -0800, ack...@vmware.com wrote:
> > +
> > +config VMWARE_VSOCK
> > +   tristate "Virtual Socket protocol"
> > +   depends on VMWARE_VMCI
> 
> What is CONFIG_VMWARE_VMCI?  I don't find that in any Kconfig in the tree?
> 
> I''m still looking over the rest, but I get build issues if I just remove
> the dependency.

VMCI is in linux-next at the moment.

Thanks,
Dmitry
--
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/


Re: linux-next: manual merge of the tty tree with the input tree

2013-01-28 Thread Dmitry Torokhov
On Mon, Jan 28, 2013 at 06:46:15AM -0800, Greg KH wrote:
> On Mon, Jan 28, 2013 at 08:44:24PM +1100, Stephen Rothwell wrote:
> > Hi Greg,
> > 
> > Today's linux-next merge of the tty tree got a conflict in
> > drivers/input/keyboard/Kconfig between commit 6f2ac009f29b ("Input:
> > goldfish - virtual input event driver") from the input tree and commit
> > 4f73bc4dd3e8 ("tty: Added a CONFIG_TTY option to allow removal of TTY")
> > from the tty tree.
> > 
> > I fixed it up (see below - I am not sure if GOLDFISH_EVENTS needs TTY or
> > not) and can carry the fix as necessary (no action is required).
> > 
> > -- 
> > Cheers,
> > Stephen Rothwells...@canb.auug.org.au
> > 
> > diff --cc drivers/input/keyboard/Kconfig
> > index 078305e,008f96a..000
> > --- a/drivers/input/keyboard/Kconfig
> > +++ b/drivers/input/keyboard/Kconfig
> > @@@ -479,16 -482,8 +482,18 @@@ config KEYBOARD_SAMSUN
> >   To compile this driver as a module, choose M here: the
> >   module will be called samsung-keypad.
> >   
> > + if TTY
> > + 
> >  +config KEYBOARD_GOLDFISH_EVENTS
> >  +  depends on GOLDFISH
> >  +  tristate "Generic Input Event device for Goldfish"
> >  +  help
> >  +Say Y here to get an input event device for the Goldfish virtual
> 
> Looks good, thanks.

Greg,

Please drop 4f73bc4dd3e8563ef4109f293a092820dff66d92, at least the parts
related to input. As far as I know nothing except serport driver
depends on tty and we do not need to introduce this kind of
dependencie. Anyone needing slim config can simply try disabling
input (or parts of it) without needing an artificial dependencies.

Thanks.

-- 
Dmitry
--
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/


[PATCH 2/5] W1: w1-gpio - switch to using dev_pm_ops

2013-02-22 Thread Dmitry Torokhov
Signed-off-by: Dmitry Torokhov 
---
 drivers/w1/masters/w1-gpio.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 799dafd..c45b9ae 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -176,11 +176,10 @@ static int w1_gpio_remove(struct platform_device *pdev)
return 0;
 }
 
-#ifdef CONFIG_PM
-
-static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
+#ifdef CONFIG_PM_SLEEP
+static int w1_gpio_suspend(struct device *dev)
 {
-   struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+   const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
 
if (pdata->enable_external_pullup)
pdata->enable_external_pullup(0);
@@ -188,31 +187,28 @@ static int w1_gpio_suspend(struct platform_device *pdev, 
pm_message_t state)
return 0;
 }
 
-static int w1_gpio_resume(struct platform_device *pdev)
+static int w1_gpio_resume(struct device *dev)
 {
-   struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+   const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
 
if (pdata->enable_external_pullup)
pdata->enable_external_pullup(1);
 
return 0;
 }
-
-#else
-#define w1_gpio_suspendNULL
-#define w1_gpio_resume NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(w1_gpio_pm_ops, w1_gpio_suspend, w1_gpio_resume);
+
 static struct platform_driver w1_gpio_driver = {
.driver = {
.name   = "w1-gpio",
.owner  = THIS_MODULE,
+   .pm = &w1_gpio_pm_ops,
.of_match_table = of_match_ptr(w1_gpio_dt_ids),
},
.probe = w1_gpio_probe,
.remove = w1_gpio_remove,
-   .suspend = w1_gpio_suspend,
-   .resume = w1_gpio_resume,
 };
 
 module_platform_driver(w1_gpio_driver);
-- 
1.7.11.7

--
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/


[PATCH 3/5] W1: w1-gpio - guard DT IDs with CONFIG_OF

2013-02-22 Thread Dmitry Torokhov
This fixes the following warning:

  CC  drivers/w1/masters/w1-gpio.o
drivers/w1/masters/w1-gpio.c:50:28: warning: ‘w1_gpio_dt_ids’ defined but not 
used [-Wunused-variable]

Also provide stub for w1_gpio_probe_dt() if device tree support is
disabled.

Signed-off-by: Dmitry Torokhov 
---
 drivers/w1/masters/w1-gpio.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index c45b9ae..aa97a96 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -47,6 +47,8 @@ static u8 w1_gpio_read_bit(void *data)
return gpio_get_value(pdata->pin) ? 1 : 0;
 }
 
+#ifdef CONFIG_OF
+
 static struct of_device_id w1_gpio_dt_ids[] = {
{ .compatible = "w1-gpio" },
{}
@@ -72,6 +74,15 @@ static int w1_gpio_probe_dt(struct platform_device *pdev)
return 0;
 }
 
+#else
+
+static inline int w1_gpio_probe_dt(struct platform_device *pdev)
+{
+   return -ENOSYS;
+}
+
+#endif
+
 static int w1_gpio_probe(struct platform_device *pdev)
 {
struct w1_bus_master *master;
-- 
1.7.11.7

--
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/


[PATCH 5/5] W1: w1-gpio - switch to using managed resources (devm)

2013-02-22 Thread Dmitry Torokhov
This simplifies error unwinding and device teardown.

Signed-off-by: Dmitry Torokhov 
---
 drivers/w1/masters/w1-gpio.c | 31 ++-
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index ee6b6e3..19439f8 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -116,25 +116,26 @@ static int w1_gpio_probe(struct platform_device *pdev)
return err;
}
 
-   w1_gpio = kzalloc(sizeof(struct w1_gpio), GFP_KERNEL);
+   w1_gpio = devm_kzalloc(&pdev->dev, sizeof(struct w1_gpio), GFP_KERNEL);
if (!w1_gpio) {
dev_err(&pdev->dev, "Out of memory\n");
return -ENOMEM;
}
 
-   err = gpio_request(pdata->pin, "w1");
+   err = devm_gpio_request(&pdev->dev, pdata->pin, "w1");
if (err) {
dev_err(&pdev->dev, "gpio_request (pin) failed\n");
-   goto free_master;
+   return err;
}
 
if (gpio_is_valid(pdata->ext_pullup_enable_pin)) {
-   err = gpio_request_one(pdata->ext_pullup_enable_pin,
-  GPIOF_INIT_LOW, "w1 pullup");
+   err = devm_gpio_request_one(&pdev->dev,
+   pdata->ext_pullup_enable_pin,
+   GPIOF_INIT_LOW, "w1 pullup");
if (err < 0) {
-   dev_err(&pdev->dev, "gpio_request_one "
-   "(ext_pullup_enable_pin) failed\n");
-   goto free_gpio;
+   dev_err(&pdev->dev,
+   "gpio_request_one (ext_pullup_enable_pin) 
failed\n");
+   return err;
}
}
 
@@ -154,7 +155,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
err = w1_add_master_device(&w1_gpio->master);
if (err) {
dev_err(&pdev->dev, "w1_add_master device failed\n");
-   goto free_gpio_ext_pu;
+   return err;
}
 
if (pdata->enable_external_pullup)
@@ -166,16 +167,6 @@ static int w1_gpio_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, w1_gpio);
 
return 0;
-
- free_gpio_ext_pu:
-   if (gpio_is_valid(pdata->ext_pullup_enable_pin))
-   gpio_free(pdata->ext_pullup_enable_pin);
- free_gpio:
-   gpio_free(pdata->pin);
- free_master:
-   kfree(w1_gpio);
-
-   return err;
 }
 
 static int w1_gpio_remove(struct platform_device *pdev)
@@ -190,8 +181,6 @@ static int w1_gpio_remove(struct platform_device *pdev)
gpio_set_value(pdata->ext_pullup_enable_pin, 0);
 
w1_remove_master_device(&w1_gpio->master);
-   gpio_free(pdata->pin);
-   kfree(w1_gpio);
 
return 0;
 }
-- 
1.7.11.7

--
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/


[PATCH 4/5] W1: w1-gpio - rework handling of platform data

2013-02-22 Thread Dmitry Torokhov
The platform data in the dveice structure does not belong to the driver
and so it should not be trying to alter it, but instead use a local pointer
and populate it with a local copy in case we are dealing with device tree
setup.

Also allow mixed setups where platform data coexists with device tree and
prefer kernel-supplied data (it may be easier to fiddle in kernel structure
before committing final result to device tree).

Signed-off-by: Dmitry Torokhov 
---
 drivers/w1/masters/w1-gpio.c | 93 +---
 1 file changed, 53 insertions(+), 40 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index aa97a96..ee6b6e3 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -23,28 +23,33 @@
 #include "../w1.h"
 #include "../w1_int.h"
 
+struct w1_gpio {
+   struct w1_bus_master master;
+   const struct w1_gpio_platform_data *pdata;
+};
+
 static void w1_gpio_write_bit_dir(void *data, u8 bit)
 {
-   struct w1_gpio_platform_data *pdata = data;
+   struct w1_gpio *w1_gpio = data;
 
if (bit)
-   gpio_direction_input(pdata->pin);
+   gpio_direction_input(w1_gpio->pdata->pin);
else
-   gpio_direction_output(pdata->pin, 0);
+   gpio_direction_output(w1_gpio->pdata->pin, 0);
 }
 
 static void w1_gpio_write_bit_val(void *data, u8 bit)
 {
-   struct w1_gpio_platform_data *pdata = data;
+   struct w1_gpio *w1_gpio = data;
 
-   gpio_set_value(pdata->pin, bit);
+   gpio_set_value(w1_gpio->pdata->pin, bit);
 }
 
 static u8 w1_gpio_read_bit(void *data)
 {
-   struct w1_gpio_platform_data *pdata = data;
+   struct w1_gpio *w1_gpio = data;
 
-   return gpio_get_value(pdata->pin) ? 1 : 0;
+   return gpio_get_value(w1_gpio->pdata->pin) ? 1 : 0;
 }
 
 #ifdef CONFIG_OF
@@ -55,38 +60,43 @@ static struct of_device_id w1_gpio_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, w1_gpio_dt_ids);
 
-static int w1_gpio_probe_dt(struct platform_device *pdev)
+static struct w1_gpio_platform_data *
+w1_gpio_probe_dt(struct platform_device *pdev)
 {
-   struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+   struct w1_gpio_platform_data *pdata;
struct device_node *np = pdev->dev.of_node;
 
+   if (!np)
+   return ERR_PTR(-ENOENT);
+
pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
-   return -ENOMEM;
+   return ERR_PTR(-ENOMEM);
 
if (of_get_property(np, "linux,open-drain", NULL))
pdata->is_open_drain = 1;
 
pdata->pin = of_get_gpio(np, 0);
pdata->ext_pullup_enable_pin = of_get_gpio(np, 1);
-   pdev->dev.platform_data = pdata;
 
-   return 0;
+   return pdata;
 }
 
 #else
 
-static inline int w1_gpio_probe_dt(struct platform_device *pdev)
+static inline struct w1_gpio_platform_data *
+w1_gpio_probe_dt(struct platform_device *pdev)
 {
-   return -ENOSYS;
+   return NULL;
 }
 
 #endif
 
 static int w1_gpio_probe(struct platform_device *pdev)
 {
-   struct w1_bus_master *master;
-   struct w1_gpio_platform_data *pdata;
+   const struct w1_gpio_platform_data *pdata =
+   dev_get_platdata(&pdev->dev);
+   struct w1_gpio *w1_gpio;
struct pinctrl *pinctrl;
int err;
 
@@ -94,23 +104,20 @@ static int w1_gpio_probe(struct platform_device *pdev)
if (IS_ERR(pinctrl))
dev_warn(&pdev->dev, "unable to select pin group\n");
 
-   if (of_have_populated_dt()) {
-   err = w1_gpio_probe_dt(pdev);
-   if (err < 0) {
-   dev_err(&pdev->dev, "Failed to parse DT\n");
-   return err;
-   }
-   }
-
-   pdata = pdev->dev.platform_data;
+   if (!pdata)
+   pdata = w1_gpio_probe_dt(pdev);
 
if (!pdata) {
dev_err(&pdev->dev, "No configuration data\n");
return -ENXIO;
+   } else if (IS_ERR(pdata)) {
+   err = PTR_ERR(pdata);
+   dev_err(&pdev->dev, "Failed to parse DT\n");
+   return err;
}
 
-   master = kzalloc(sizeof(struct w1_bus_master), GFP_KERNEL);
-   if (!master) {
+   w1_gpio = kzalloc(sizeof(struct w1_gpio), GFP_KERNEL);
+   if (!w1_gpio) {
dev_err(&pdev->dev, "Out of memory\n");
return -ENOMEM;
}
@@ -131,18 +138,20 @@ static int w1_gpio_probe(struct platform_device *pdev)
}
}
 
-   master->data = pdata;
-   master->read_bit = w1_gpio_read_bit;
+   w1_gpio->pdata = pdata;
+
+   w1_gpio->master.data = w1_gpio;
+

[PATCH 1/5] W1: w1-gpio - fix incorrect __init/__exit markups

2013-02-23 Thread Dmitry Torokhov
We should not be using __init/__exit markups on probe() and remove()
methods unless platform_device_probe() is used, because other methods
allow unbinding device through sysfs and these methods should not be
discarded.

Signed-off-by: Dmitry Torokhov 
---

This was just compiled - no hardware.

 drivers/w1/masters/w1-gpio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 85b363a..799dafd 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -72,7 +72,7 @@ static int w1_gpio_probe_dt(struct platform_device *pdev)
return 0;
 }
 
-static int __init w1_gpio_probe(struct platform_device *pdev)
+static int w1_gpio_probe(struct platform_device *pdev)
 {
struct w1_bus_master *master;
struct w1_gpio_platform_data *pdata;
@@ -158,7 +158,7 @@ static int __init w1_gpio_probe(struct platform_device 
*pdev)
return err;
 }
 
-static int __exit w1_gpio_remove(struct platform_device *pdev)
+static int w1_gpio_remove(struct platform_device *pdev)
 {
struct w1_bus_master *master = platform_get_drvdata(pdev);
struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
@@ -210,7 +210,7 @@ static struct platform_driver w1_gpio_driver = {
.of_match_table = of_match_ptr(w1_gpio_dt_ids),
},
.probe = w1_gpio_probe,
-   .remove = __exit_p(w1_gpio_remove),
+   .remove = w1_gpio_remove,
.suspend = w1_gpio_suspend,
.resume = w1_gpio_resume,
 };
-- 
1.7.11.7

--
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/


Re: [PATCH 4/5] W1: w1-gpio - rework handling of platform data

2013-02-23 Thread Dmitry Torokhov
On Sat, Feb 23, 2013 at 06:55:45PM +0200, Ville Syrjälä wrote:
> On Fri, Feb 22, 2013 at 11:58:39PM -0800, Dmitry Torokhov wrote:
> > The platform data in the dveice structure does not belong to the driver
> > and so it should not be trying to alter it, but instead use a local pointer
> > and populate it with a local copy in case we are dealing with device tree
> > setup.
> > 
> > Also allow mixed setups where platform data coexists with device tree and
> > prefer kernel-supplied data (it may be easier to fiddle in kernel structure
> > before committing final result to device tree).
> > 
> > Signed-off-by: Dmitry Torokhov 
> > ---
> >  drivers/w1/masters/w1-gpio.c | 93 
> > +---
> >  1 file changed, 53 insertions(+), 40 deletions(-)
> > 
> > diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
> > index aa97a96..ee6b6e3 100644
> > --- a/drivers/w1/masters/w1-gpio.c
> > +++ b/drivers/w1/masters/w1-gpio.c
> > @@ -23,28 +23,33 @@
> >  #include "../w1.h"
> >  #include "../w1_int.h"
> >  
> > +struct w1_gpio {
> > +   struct w1_bus_master master;
> > +   const struct w1_gpio_platform_data *pdata;
> > +};
> 
> I don't understand why you need this wrapper struct. It doesn't
> seem to serve much purpose, other than to cause code churn in
> the patch.

Hm, yes, you are right, I should be able to get rid of it...

Thanks.

-- 
Dmitry
--
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/


[RFC/PATCH] devres: allow adding custom actions to the stack

2013-02-23 Thread Dmitry Torokhov
Sometimes drivers need to execute one-off actions in their error handling
or device teardown paths. An example would be toggling a GPIO line to
reset the controlled device into predefined state.

To allow performing such actions when using managed resources let's allow
adding them to stack/group of devres resources.

Signed-off-by: Dmitry Torokhov 
---
 drivers/base/devres.c  | 74 ++
 include/linux/device.h |  4 +++
 2 files changed, 78 insertions(+)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 6683906..507379e 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -671,6 +671,80 @@ int devres_release_group(struct device *dev, void *id)
 EXPORT_SYMBOL_GPL(devres_release_group);
 
 /*
+ * Custom devres actions allow inserting a simple function call
+ * into the teadown sequence.
+ */
+
+struct action_devres {
+   void *data;
+   void (*action)(void *);
+};
+
+static int devm_action_match(struct device *dev, void *res, void *p)
+{
+   struct action_devres *devres = res;
+   struct action_devres *target = p;
+
+   return devres->action == target->action &&
+  devres->data == target->data;
+}
+
+static void devm_action_release(struct device *dev, void *res)
+{
+   struct action_devres *devres = res;
+
+   devres->action(devres->data);
+}
+
+/**
+ * devm_add_action() - add a custom action to list of managed resources
+ * @dev: Device that owns the action
+ * @action: Function that should be called
+ * @data: Pointer to data passed to @action implementation
+ *
+ * This adds a custom action to the list of managed resources so that
+ * it gets executed as part of standard resource unwinding.
+ */
+int devm_add_action(struct device *dev, void (*action)(void *), void *data)
+{
+   struct action_devres *devres;
+
+   devres = devres_alloc(devm_action_release,
+ sizeof(struct action_devres), GFP_KERNEL);
+   if (!devres)
+   return -ENOMEM;
+
+   devres->data = data;
+   devres->action = action;
+
+   devres_add(dev, devres);
+   return 0;
+}
+EXPORT_SYMBOL_GPL(devm_add_action);
+
+/**
+ * devm_remove_action() - removes previously added custom action
+ * @dev: Device that owns the action
+ * @action: Function implementing the action
+ * @data: Pointer to data passed to @action implementation
+ *
+ * Removes instance of @action previously added by devm_add_action().
+ * Both action and data should match one of the existing entries.
+ */
+void devm_remove_action(struct device *dev, void (*action)(void *), void *data)
+{
+   struct action_devres devres = {
+   .data = data,
+   .action = action,
+   };
+
+   WARN_ON(devres_destroy(dev, devm_action_release, devm_action_match,
+  &devres));
+
+}
+EXPORT_SYMBOL_GPL(devm_remove_action);
+
+/*
  * Managed kzalloc/kfree
  */
 static void devm_kzalloc_release(struct device *dev, void *res)
diff --git a/include/linux/device.h b/include/linux/device.h
index 2fbf088..ba7c802 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -578,6 +578,10 @@ extern void devm_kfree(struct device *dev, void *p);
 void __iomem *devm_request_and_ioremap(struct device *dev,
struct resource *res);
 
+/* allows to add/remove a custom action to devres stack */
+int devm_add_action(struct device *dev, void (*action)(void *), void *data);
+void devm_remove_action(struct device *dev, void (*action)(void *), void 
*data);
+
 struct device_dma_parameters {
/*
 * a low level driver may set these to teach IOMMU code about
-- 
1.7.11.7


-- 
Dmitry
--
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/


Re: [PATCH v5 1/3] mfd: syscon: Removed support for unloading

2013-02-23 Thread Dmitry Torokhov
On Sat, Feb 23, 2013 at 04:51:19PM -0700, Stephen Warren wrote:
> On 02/22/2013 10:28 PM, Alexander Shiyan wrote:
> >> On 02/22/2013 10:15 PM, Alexander Shiyan wrote:
> >>> The driver can be used in various subsystems and therefore should not
> >>> be unloaded when it is defined in the kernel configuration, so remove
> >>> support for unloading it.
> >>
> >> Why not fix the clients to module_get() at the appropriate times; then
> >> you could still allow unloading, couldn't you?
> > 
> > I has explain this before.
> 
> If multiple people have asked this, perhaps it'd be a good idea to
> include the answer in the commit description.
> 
> > Driver defined as "bool" and loaded via postcore_initcall.
> 
> Being defined as a "bool" sounds like a reasonable reason that no
> remove() is required.

No, it is not - I can still unbind the device from driver via sysfs. Now
what will happen is resources are leaked and won't be available when
trying to bin (via sysfs) again.

This patch is broken.

Thanks.

-- 
Dmitry
--
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/


[PATCH] Media: remove incorrect __exit markups

2013-02-24 Thread Dmitry Torokhov
Even if bus is not hot-pluggable, the devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes.

Signed-off-by: Dmitry Torokhov 
---
 drivers/media/i2c/adp1653.c  | 4 ++--
 drivers/media/i2c/smiapp/smiapp-core.c   | 4 ++--
 drivers/media/platform/soc_camera/omap1_camera.c | 4 ++--
 drivers/media/radio/radio-si4713.c   | 4 ++--
 drivers/media/rc/ir-rx51.c   | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
index df16380..ef75abe 100644
--- a/drivers/media/i2c/adp1653.c
+++ b/drivers/media/i2c/adp1653.c
@@ -447,7 +447,7 @@ free_and_quit:
return ret;
 }
 
-static int __exit adp1653_remove(struct i2c_client *client)
+static int adp1653_remove(struct i2c_client *client)
 {
struct v4l2_subdev *subdev = i2c_get_clientdata(client);
struct adp1653_flash *flash = to_adp1653_flash(subdev);
@@ -476,7 +476,7 @@ static struct i2c_driver adp1653_i2c_driver = {
.pm = &adp1653_pm_ops,
},
.probe  = adp1653_probe,
-   .remove = __exit_p(adp1653_remove),
+   .remove = adp1653_remove,
.id_table   = adp1653_id_table,
 };
 
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 83c7ed7..cae4f46 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2833,7 +2833,7 @@ static int smiapp_probe(struct i2c_client *client,
 sensor->src->pads, 0);
 }
 
-static int __exit smiapp_remove(struct i2c_client *client)
+static int smiapp_remove(struct i2c_client *client)
 {
struct v4l2_subdev *subdev = i2c_get_clientdata(client);
struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
@@ -2881,7 +2881,7 @@ static struct i2c_driver smiapp_i2c_driver = {
.pm = &smiapp_pm_ops,
},
.probe  = smiapp_probe,
-   .remove = __exit_p(smiapp_remove),
+   .remove = smiapp_remove,
.id_table = smiapp_id_table,
 };
 
diff --git a/drivers/media/platform/soc_camera/omap1_camera.c 
b/drivers/media/platform/soc_camera/omap1_camera.c
index 39a77f0..5f548ac 100644
--- a/drivers/media/platform/soc_camera/omap1_camera.c
+++ b/drivers/media/platform/soc_camera/omap1_camera.c
@@ -1677,7 +1677,7 @@ exit:
return err;
 }
 
-static int __exit omap1_cam_remove(struct platform_device *pdev)
+static int omap1_cam_remove(struct platform_device *pdev)
 {
struct soc_camera_host *soc_host = to_soc_camera_host(&pdev->dev);
struct omap1_cam_dev *pcdev = container_of(soc_host,
@@ -1709,7 +1709,7 @@ static struct platform_driver omap1_cam_driver = {
.name   = DRIVER_NAME,
},
.probe  = omap1_cam_probe,
-   .remove = __exit_p(omap1_cam_remove),
+   .remove = omap1_cam_remove,
 };
 
 module_platform_driver(omap1_cam_driver);
diff --git a/drivers/media/radio/radio-si4713.c 
b/drivers/media/radio/radio-si4713.c
index 1507c9d..8ae8442d 100644
--- a/drivers/media/radio/radio-si4713.c
+++ b/drivers/media/radio/radio-si4713.c
@@ -328,7 +328,7 @@ exit:
 }
 
 /* radio_si4713_pdriver_remove - remove the device */
-static int __exit radio_si4713_pdriver_remove(struct platform_device *pdev)
+static int radio_si4713_pdriver_remove(struct platform_device *pdev)
 {
struct v4l2_device *v4l2_dev = platform_get_drvdata(pdev);
struct radio_si4713_device *rsdev = container_of(v4l2_dev,
@@ -350,7 +350,7 @@ static struct platform_driver radio_si4713_pdriver = {
.name   = "radio-si4713",
},
.probe  = radio_si4713_pdriver_probe,
-   .remove = __exit_p(radio_si4713_pdriver_remove),
+   .remove = radio_si4713_pdriver_remove,
 };
 
 module_platform_driver(radio_si4713_pdriver);
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 8ead492..31b955b 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -464,14 +464,14 @@ static int lirc_rx51_probe(struct platform_device *dev)
return 0;
 }
 
-static int __exit lirc_rx51_remove(struct platform_device *dev)
+static int lirc_rx51_remove(struct platform_device *dev)
 {
return lirc_unregister_driver(lirc_rx51_driver.minor);
 }
 
 struct platform_driver lirc_rx51_platform_driver = {
.probe  = lirc_rx51_probe,
-   .remove = __exit_p(lirc_rx51_remove),
+   .remove = lirc_rx51_remove,
.suspend= lirc_rx51_suspend,
.resume = lirc_rx51_resume,
.driver = {
-- 
1.7.11.7


-- 
Dmitry
--
To unsubscribe from this list: send the lin

[PATCH] Regulator: db8500-prcmu - remove incorrect __exit markup

2013-02-24 Thread Dmitry Torokhov
Even if bus is not hot-pluggable, the devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes.

Signed-off-by: Dmitry Torokhov 
---
 drivers/regulator/db8500-prcmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/db8500-prcmu.c b/drivers/regulator/db8500-prcmu.c
index 219d162..a53c11a 100644
--- a/drivers/regulator/db8500-prcmu.c
+++ b/drivers/regulator/db8500-prcmu.c
@@ -528,7 +528,7 @@ static int db8500_regulator_probe(struct platform_device 
*pdev)
return 0;
 }
 
-static int __exit db8500_regulator_remove(struct platform_device *pdev)
+static int db8500_regulator_remove(struct platform_device *pdev)
 {
int i;
 
@@ -553,7 +553,7 @@ static struct platform_driver db8500_regulator_driver = {
.owner = THIS_MODULE,
},
.probe = db8500_regulator_probe,
-   .remove = __exit_p(db8500_regulator_remove),
+   .remove = db8500_regulator_remove,
 };
 
 static int __init db8500_regulator_init(void)
-- 
1.7.11.7


-- 
Dmitry
--
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/


[PATCH] SPI: spi-ppc4xx - remove incorrect __exit markup

2013-02-24 Thread Dmitry Torokhov
Even if bus is not hot-pluggable, the devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes.

Signed-off-by: Dmitry Torokhov 
---
 drivers/spi/spi-ppc4xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-ppc4xx.c b/drivers/spi/spi-ppc4xx.c
index 7a85f22..027b474 100644
--- a/drivers/spi/spi-ppc4xx.c
+++ b/drivers/spi/spi-ppc4xx.c
@@ -560,7 +560,7 @@ free_master:
return ret;
 }
 
-static int __exit spi_ppc4xx_of_remove(struct platform_device *op)
+static int spi_ppc4xx_of_remove(struct platform_device *op)
 {
struct spi_master *master = dev_get_drvdata(&op->dev);
struct ppc4xx_spi *hw = spi_master_get_devdata(master);
@@ -583,7 +583,7 @@ MODULE_DEVICE_TABLE(of, spi_ppc4xx_of_match);
 
 static struct platform_driver spi_ppc4xx_of_driver = {
.probe = spi_ppc4xx_of_probe,
-   .remove = __exit_p(spi_ppc4xx_of_remove),
+   .remove = spi_ppc4xx_of_remove,
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
-- 
1.7.11.7


-- 
Dmitry
--
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/


[PATCH 2/5] W1: w1-gpio - switch to using dev_pm_ops

2013-02-24 Thread Dmitry Torokhov
Signed-off-by: Dmitry Torokhov 
---
 drivers/w1/masters/w1-gpio.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 799dafd..c45b9ae 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -176,11 +176,10 @@ static int w1_gpio_remove(struct platform_device *pdev)
return 0;
 }
 
-#ifdef CONFIG_PM
-
-static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
+#ifdef CONFIG_PM_SLEEP
+static int w1_gpio_suspend(struct device *dev)
 {
-   struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+   const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
 
if (pdata->enable_external_pullup)
pdata->enable_external_pullup(0);
@@ -188,31 +187,28 @@ static int w1_gpio_suspend(struct platform_device *pdev, 
pm_message_t state)
return 0;
 }
 
-static int w1_gpio_resume(struct platform_device *pdev)
+static int w1_gpio_resume(struct device *dev)
 {
-   struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+   const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
 
if (pdata->enable_external_pullup)
pdata->enable_external_pullup(1);
 
return 0;
 }
-
-#else
-#define w1_gpio_suspendNULL
-#define w1_gpio_resume NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(w1_gpio_pm_ops, w1_gpio_suspend, w1_gpio_resume);
+
 static struct platform_driver w1_gpio_driver = {
.driver = {
.name   = "w1-gpio",
.owner  = THIS_MODULE,
+   .pm = &w1_gpio_pm_ops,
.of_match_table = of_match_ptr(w1_gpio_dt_ids),
},
.probe = w1_gpio_probe,
.remove = w1_gpio_remove,
-   .suspend = w1_gpio_suspend,
-   .resume = w1_gpio_resume,
 };
 
 module_platform_driver(w1_gpio_driver);
-- 
1.7.11.7

--
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/


[PATCH 5/5] W1: w1-gpio - switch to using managed resources (devm)

2013-02-24 Thread Dmitry Torokhov
This simplifies error unwinding and device teardown.

Signed-off-by: Dmitry Torokhov 
---
 drivers/w1/masters/w1-gpio.c | 32 +++-
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 465ce52..464b1a8 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -111,25 +111,27 @@ static int w1_gpio_probe(struct platform_device *pdev)
return err;
}
 
-   master = kzalloc(sizeof(struct w1_bus_master), GFP_KERNEL);
+   master = devm_kzalloc(&pdev->dev,
+ sizeof(struct w1_bus_master), GFP_KERNEL);
if (!master) {
dev_err(&pdev->dev, "Out of memory\n");
return -ENOMEM;
}
 
-   err = gpio_request(pdata->pin, "w1");
+   err = devm_gpio_request(&pdev->dev, pdata->pin, "w1");
if (err) {
dev_err(&pdev->dev, "gpio_request (pin) failed\n");
-   goto free_master;
+   return err;
}
 
if (gpio_is_valid(pdata->ext_pullup_enable_pin)) {
-   err = gpio_request_one(pdata->ext_pullup_enable_pin,
-  GPIOF_INIT_LOW, "w1 pullup");
+   err = devm_gpio_request_one(&pdev->dev,
+   pdata->ext_pullup_enable_pin,
+   GPIOF_INIT_LOW, "w1 pullup");
if (err < 0) {
-   dev_err(&pdev->dev, "gpio_request_one "
-   "(ext_pullup_enable_pin) failed\n");
-   goto free_gpio;
+   dev_err(&pdev->dev,
+   "gpio_request_one (ext_pullup_enable_pin) 
failed\n");
+   return err;
}
}
 
@@ -147,7 +149,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
err = w1_add_master_device(master);
if (err) {
dev_err(&pdev->dev, "w1_add_master device failed\n");
-   goto free_gpio_ext_pu;
+   return err;
}
 
if (pdata->enable_external_pullup)
@@ -159,16 +161,6 @@ static int w1_gpio_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, master);
 
return 0;
-
- free_gpio_ext_pu:
-   if (gpio_is_valid(pdata->ext_pullup_enable_pin))
-   gpio_free(pdata->ext_pullup_enable_pin);
- free_gpio:
-   gpio_free(pdata->pin);
- free_master:
-   kfree(master);
-
-   return err;
 }
 
 static int w1_gpio_remove(struct platform_device *pdev)
@@ -183,8 +175,6 @@ static int w1_gpio_remove(struct platform_device *pdev)
gpio_set_value(pdata->ext_pullup_enable_pin, 0);
 
w1_remove_master_device(master);
-   gpio_free(pdata->pin);
-   kfree(master);
 
return 0;
 }
-- 
1.7.11.7

--
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/


[PATCH 4/5] W1: w1-gpio - rework handling of platform data

2013-02-24 Thread Dmitry Torokhov
The platform data in the dveice structure does not belong to the driver
and so it should not be trying to alter it, but instead use a local pointer
and populate it with a local copy in case we are dealing with device tree
setup.

Also allow mixed setups where platform data coexists with device tree and
prefer kernel-supplied data (it may be easier to fiddle in kernel structure
before committing final result to device tree).

Signed-off-by: Dmitry Torokhov 
---
 drivers/w1/masters/w1-gpio.c | 44 +---
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index aa97a96..465ce52 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -55,30 +55,34 @@ static struct of_device_id w1_gpio_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, w1_gpio_dt_ids);
 
-static int w1_gpio_probe_dt(struct platform_device *pdev)
+static struct w1_gpio_platform_data *
+w1_gpio_probe_dt(struct platform_device *pdev)
 {
-   struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+   struct w1_gpio_platform_data *pdata;
struct device_node *np = pdev->dev.of_node;
 
+   if (!np)
+   return ERR_PTR(-ENOENT);
+
pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
-   return -ENOMEM;
+   return ERR_PTR(-ENOMEM);
 
if (of_get_property(np, "linux,open-drain", NULL))
pdata->is_open_drain = 1;
 
pdata->pin = of_get_gpio(np, 0);
pdata->ext_pullup_enable_pin = of_get_gpio(np, 1);
-   pdev->dev.platform_data = pdata;
 
-   return 0;
+   return pdata;
 }
 
 #else
 
-static inline int w1_gpio_probe_dt(struct platform_device *pdev)
+static inline struct w1_gpio_platform_data *
+w1_gpio_probe_dt(struct platform_device *pdev)
 {
-   return -ENOSYS;
+   return NULL;
 }
 
 #endif
@@ -94,19 +98,17 @@ static int w1_gpio_probe(struct platform_device *pdev)
if (IS_ERR(pinctrl))
dev_warn(&pdev->dev, "unable to select pin group\n");
 
-   if (of_have_populated_dt()) {
-   err = w1_gpio_probe_dt(pdev);
-   if (err < 0) {
-   dev_err(&pdev->dev, "Failed to parse DT\n");
-   return err;
-   }
-   }
-
-   pdata = pdev->dev.platform_data;
+   pdata = dev_get_platdata(&pdev->dev);
+   if (!pdata)
+   pdata = w1_gpio_probe_dt(pdev);
 
if (!pdata) {
dev_err(&pdev->dev, "No configuration data\n");
return -ENXIO;
+   } else if (IS_ERR(pdata)) {
+   err = PTR_ERR(pdata);
+   dev_err(&pdev->dev, "Failed to parse DT\n");
+   return err;
}
 
master = kzalloc(sizeof(struct w1_bus_master), GFP_KERNEL);
@@ -172,7 +174,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
 static int w1_gpio_remove(struct platform_device *pdev)
 {
struct w1_bus_master *master = platform_get_drvdata(pdev);
-   struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+   const struct w1_gpio_platform_data *pdata = master->data;
 
if (pdata->enable_external_pullup)
pdata->enable_external_pullup(0);
@@ -190,7 +192,9 @@ static int w1_gpio_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int w1_gpio_suspend(struct device *dev)
 {
-   const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
+   struct platform_device *pdev = to_platform_device(dev);
+   struct w1_bus_master *master = platform_get_drvdata(pdev);
+   const struct w1_gpio_platform_data *pdata = master->data;
 
if (pdata->enable_external_pullup)
pdata->enable_external_pullup(0);
@@ -200,7 +204,9 @@ static int w1_gpio_suspend(struct device *dev)
 
 static int w1_gpio_resume(struct device *dev)
 {
-   const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
+   struct platform_device *pdev = to_platform_device(dev);
+   struct w1_bus_master *master = platform_get_drvdata(pdev);
+   const struct w1_gpio_platform_data *pdata = master->data;
 
if (pdata->enable_external_pullup)
pdata->enable_external_pullup(1);
-- 
1.7.11.7

--
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/


[PATCH 3/5] W1: w1-gpio - guard DT IDs with CONFIG_OF

2013-02-24 Thread Dmitry Torokhov
This fixes the following warning:

  CC  drivers/w1/masters/w1-gpio.o
drivers/w1/masters/w1-gpio.c:50:28: warning: ‘w1_gpio_dt_ids’ defined but not 
used [-Wunused-variable]

Also provide stub for w1_gpio_probe_dt() if device tree support is
disabled.

Signed-off-by: Dmitry Torokhov 
---
 drivers/w1/masters/w1-gpio.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index c45b9ae..aa97a96 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -47,6 +47,8 @@ static u8 w1_gpio_read_bit(void *data)
return gpio_get_value(pdata->pin) ? 1 : 0;
 }
 
+#ifdef CONFIG_OF
+
 static struct of_device_id w1_gpio_dt_ids[] = {
{ .compatible = "w1-gpio" },
{}
@@ -72,6 +74,15 @@ static int w1_gpio_probe_dt(struct platform_device *pdev)
return 0;
 }
 
+#else
+
+static inline int w1_gpio_probe_dt(struct platform_device *pdev)
+{
+   return -ENOSYS;
+}
+
+#endif
+
 static int w1_gpio_probe(struct platform_device *pdev)
 {
struct w1_bus_master *master;
-- 
1.7.11.7

--
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/


[PATCH v2 1/5] W1: w1-gpio - fix incorrect __init/__exit markups

2013-02-24 Thread Dmitry Torokhov
We should not be using __init/__exit markups on probe() and remove()
methods unless platform_device_probe() is used, because other methods
allow unbinding device through sysfs and these methods should not be
discarded.

Signed-off-by: Dmitry Torokhov 
---

Paatch #4 was adjusted according to Ville's comments.

 drivers/w1/masters/w1-gpio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 85b363a..799dafd 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -72,7 +72,7 @@ static int w1_gpio_probe_dt(struct platform_device *pdev)
return 0;
 }
 
-static int __init w1_gpio_probe(struct platform_device *pdev)
+static int w1_gpio_probe(struct platform_device *pdev)
 {
struct w1_bus_master *master;
struct w1_gpio_platform_data *pdata;
@@ -158,7 +158,7 @@ static int __init w1_gpio_probe(struct platform_device 
*pdev)
return err;
 }
 
-static int __exit w1_gpio_remove(struct platform_device *pdev)
+static int w1_gpio_remove(struct platform_device *pdev)
 {
struct w1_bus_master *master = platform_get_drvdata(pdev);
struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
@@ -210,7 +210,7 @@ static struct platform_driver w1_gpio_driver = {
.of_match_table = of_match_ptr(w1_gpio_dt_ids),
},
.probe = w1_gpio_probe,
-   .remove = __exit_p(w1_gpio_remove),
+   .remove = w1_gpio_remove,
.suspend = w1_gpio_suspend,
.resume = w1_gpio_resume,
 };
-- 
1.7.11.7

--
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/


Re: [PATCH v5 1/3] mfd: syscon: Removed support for unloading

2013-02-24 Thread Dmitry Torokhov
On Mon, Feb 25, 2013 at 10:53:42AM +0400, Alexander Shiyan wrote:
> > On Sat, Feb 23, 2013 at 04:51:19PM -0700, Stephen Warren wrote:
> > > On 02/22/2013 10:28 PM, Alexander Shiyan wrote:
> > > >> On 02/22/2013 10:15 PM, Alexander Shiyan wrote:
> > > >>> The driver can be used in various subsystems and therefore should not
> > > >>> be unloaded when it is defined in the kernel configuration, so remove
> > > >>> support for unloading it.
> > > >>
> > > >> Why not fix the clients to module_get() at the appropriate times; then
> > > >> you could still allow unloading, couldn't you?
> > > > 
> > > > I has explain this before.
> > > 
> > > If multiple people have asked this, perhaps it'd be a good idea to
> > > include the answer in the commit description.
> > > 
> > > > Driver defined as "bool" and loaded via postcore_initcall.
> > > 
> > > Being defined as a "bool" sounds like a reasonable reason that no
> > > remove() is required.
> > 
> > No, it is not - I can still unbind the device from driver via sysfs. Now
> > what will happen is resources are leaked and won't be available when
> > trying to bin (via sysfs) again.
> > 
> > This patch is broken.
> 
> I will try to resolve the dispute.
> Initially, the first part of the patch (this) was an attempt to prevent the 
> unloading
> (unregister) of the driver (not the device), ie remove module_exit call.
> The patch also removes the code to remove the device. Inclusion in this patch
> of the code was a mistake. Because of this, people are confused about the 
> terms,
> including me.
> Code, relating to the removal of the device should be moved to the third 
> patch.
> Since all moved to using the managed resources, it should not have any 
> questions.
> 
> About uloading (unregister) driver: As far i understand "__exit" section is
> completely discarded when kernel compiled without module support.
> But where is this call (module_exit) is placing in case "bool" driver and
> kernel is compiled with module support? Fixme please.

In both cases __exit code goes to .exit.text section which is either
discarded (code is compiled into the kernel) or kept (code is built as a
module).

Still, because the device can forcibly be unbound from the driver, all
these changes are pretty useless for the stated purpose. All in all, it
seems to me that this code should not be a driver at all, but rather
rolled into regmap infrastructure and be part of it's initialization
process.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH] Media: remove incorrect __exit markups

2013-02-25 Thread Dmitry Torokhov
On Mon, Feb 25, 2013 at 08:49:26AM +0100, Guennadi Liakhovetski wrote:
> Hi Dmitry
> 
> On Sun, 24 Feb 2013, Dmitry Torokhov wrote:
> 
> > Even if bus is not hot-pluggable, the devices can be unbound from the
> > driver via sysfs, so we should not be using __exit annotations on
> > remove() methods. The only exception is drivers registered with
> > platform_driver_probe() which specifically disables sysfs bind/unbind
> > attributes.
> > 
> > Signed-off-by: Dmitry Torokhov 
> > ---
> >  drivers/media/i2c/adp1653.c  | 4 ++--
> >  drivers/media/i2c/smiapp/smiapp-core.c   | 4 ++--
> >  drivers/media/platform/soc_camera/omap1_camera.c | 4 ++--
> >  drivers/media/radio/radio-si4713.c   | 4 ++--
> >  drivers/media/rc/ir-rx51.c   | 4 ++--
> >  5 files changed, 10 insertions(+), 10 deletions(-)
> 
> [snip]
> 
> > diff --git a/drivers/media/platform/soc_camera/omap1_camera.c 
> > b/drivers/media/platform/soc_camera/omap1_camera.c
> > index 39a77f0..5f548ac 100644
> > --- a/drivers/media/platform/soc_camera/omap1_camera.c
> > +++ b/drivers/media/platform/soc_camera/omap1_camera.c
> > @@ -1677,7 +1677,7 @@ exit:
> > return err;
> >  }
> >  
> > -static int __exit omap1_cam_remove(struct platform_device *pdev)
> > +static int omap1_cam_remove(struct platform_device *pdev)
> >  {
> > struct soc_camera_host *soc_host = to_soc_camera_host(&pdev->dev);
> > struct omap1_cam_dev *pcdev = container_of(soc_host,
> > @@ -1709,7 +1709,7 @@ static struct platform_driver omap1_cam_driver = {
> > .name   = DRIVER_NAME,
> > },
> > .probe  = omap1_cam_probe,
> > -   .remove = __exit_p(omap1_cam_remove),
> > +   .remove = omap1_cam_remove,
> >  };
> >  
> >  module_platform_driver(omap1_cam_driver);
> 
> This looks correct, but don't we also have to remove __init from 
> omap1_cam_probe()? Or would that be a separate patch?

Thanks Guennadi, I missed that one, will update the patch.

-- 
Dmitry
--
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/


Re: [PATCH v6 6/6] Input: Add ChromeOS EC keyboard driver

2013-02-25 Thread Dmitry Torokhov
On Mon, Feb 25, 2013 at 02:08:41PM -0800, Simon Glass wrote:
> Use the key-matrix layer to interpret key scan information from the EC
> and inject input based on the FDT-supplied key map. This driver registers
> itself with the ChromeOS EC driver to perform communications.
> 
> The matrix-keypad FDT binding is used with a small addition to control
> ghosting.
> 
> Signed-off-by: Simon Glass 
> Signed-off-by: Luigi Semenzato 
> Signed-off-by: Vincent Palatin 

Acked-by: Dmitry Torokhov 

> ---
> Changes in v6:
> - Fix incorrect indentation in cros_ec_keyb_process()
> - Remove unnecessary assignment to NULL in probe function
> 
> Changes in v5:
> - Fix {} style nit in cros_ec_keyb_has_ghosting
> - Correct key lookup logic which was broken in previous version
> - Switch cros_ec_keyb driver to use devm
> 
> Changes in v4:
> - Add 'depends on MFD_CROS_EC' to Kconfig
> - Remove use of wake_notifier
> - Remove manual code to locate device tree node
> - Add resume handler to clear keyboard scan buffer if required
> 
> Changes in v3:
> - Remove 'select MFD_CROS_EC' from Kconfig as it isn't necessary
> - Remove old_state by using input layer's idev->key
> - Move inner loop of cros_ec_keyb_has_ghosting() into its own function and 
> simplify
> - Add check for not finding the device tree node
> - Remove comment about leaking matrix_keypad_build_keymap()
> - Use platform_get_drvdata() where possible
> - Remove call to input_free_device() after input_unregister_device()
> 
> Changes in v2:
> - Remove use of __devinit/__devexit
> - Use function to read matrix-keypad parameters from DT
> - Remove key autorepeat parameters from DT binding and driver
> - Use unsigned int for rows/cols
> 
>  .../devicetree/bindings/input/cros-ec-keyb.txt |  72 +
>  drivers/input/keyboard/Kconfig |  12 +
>  drivers/input/keyboard/Makefile|   1 +
>  drivers/input/keyboard/cros_ec_keyb.c  | 334 
> +
>  4 files changed, 419 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/cros-ec-keyb.txt
>  create mode 100644 drivers/input/keyboard/cros_ec_keyb.c
> 
> diff --git a/Documentation/devicetree/bindings/input/cros-ec-keyb.txt 
> b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
> new file mode 100644
> index 000..0f6355c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
> @@ -0,0 +1,72 @@
> +ChromeOS EC Keyboard
> +
> +Google's ChromeOS EC Keyboard is a simple matrix keyboard implemented on
> +a separate EC (Embedded Controller) device. It provides a message for reading
> +key scans from the EC. These are then converted into keycodes for processing
> +by the kernel.
> +
> +This binding is based on matrix-keymap.txt and extends/modifies it as 
> follows:
> +
> +Required properties:
> +- compatible: "google,cros-ec-keyb"
> +
> +Optional properties:
> +- google,needs-ghost-filter: True to enable a ghost filter for the matrix
> +keyboard. This is recommended if the EC does not have its own logic or
> +hardware for this.
> +
> +
> +Example:
> +
> +cros-ec-keyb {
> + compatible = "google,cros-ec-keyb";
> + keypad,num-rows = <8>;
> + keypad,num-columns = <13>;
> + google,needs-ghost-filter;
> + /*
> +  * Keymap entries take the form of 0xRRCC where
> +  * RR=Row CC=Column =Key Code
> +  * The values below are for a US keyboard layout and
> +  * are taken from the Linux driver. Note that the
> +  * 102ND key is not used for US keyboards.
> +  */
> + linux,keymap = <
> + /* CAPSLCK F1 B  F10 */
> + 0x0001003a 0x0002003b 0x00030030 0x00040044
> + /* N   =  R_ALT  ESC */
> + 0x00060031 0x0008000d 0x000a0064 0x01010001
> + /* F4  G  F7 H   */
> + 0x0102003e 0x01030022 0x01040041 0x01060023
> + /* '   F9 BKSPACEL_CTRL  */
> + 0x01080028 0x01090043 0x010b000e 0x021d
> + /* TAB F3 T  F6  */
> + 0x0201000f 0x0202003d 0x02030014 0x02040040
> + /* ]   Y  102ND  [   */
> + 0x0205001b 0x02060015 0x02070056 0x0208001a
> + /* F8  GRAVE  F2 5   */
> + 0x02090042 0x03010029 0x0302003c 0x03030006
> + /* F5  6  -  \   */
> + 0x0304003f 0x03060007 0x0308000c 0x030b002b
> + /* R_CTRL  A  D  

Re: [RFC/PATCH] devres: allow adding custom actions to the stack

2013-02-25 Thread Dmitry Torokhov
On Mon, Feb 25, 2013 at 03:48:01PM -0800, Tejun Heo wrote:
> On Sat, Feb 23, 2013 at 08:05:57PM -0800, Dmitry Torokhov wrote:
> > Sometimes drivers need to execute one-off actions in their error handling
> > or device teardown paths. An example would be toggling a GPIO line to
> > reset the controlled device into predefined state.
> > 
> > To allow performing such actions when using managed resources let's allow
> > adding them to stack/group of devres resources.
> > 
> > Signed-off-by: Dmitry Torokhov 
> 
> Acked-by: Tejun Heo 
> 

Thanks Tejun.

Greg, if possible I'd like to carry this in my tree as I have driver
changes depending on this.

Thanks.

-- 
Dmitry
--
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/


[PATCH v2] Media: remove incorrect __init/__exit markups

2013-02-25 Thread Dmitry Torokhov
Even if bus is not hot-pluggable, the devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes.

Similarly probe() methods should not be marked __init unless
platform_driver_probe() is used.

Signed-off-by: Dmitry Torokhov 
---

v1->v2: removed __init markup on omap1_cam_probe() that was pointed out
by Guennadi Liakhovetski.

 drivers/media/i2c/adp1653.c  | 4 ++--
 drivers/media/i2c/smiapp/smiapp-core.c   | 4 ++--
 drivers/media/platform/soc_camera/omap1_camera.c | 6 +++---
 drivers/media/radio/radio-si4713.c   | 4 ++--
 drivers/media/rc/ir-rx51.c   | 4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
index df16380..ef75abe 100644
--- a/drivers/media/i2c/adp1653.c
+++ b/drivers/media/i2c/adp1653.c
@@ -447,7 +447,7 @@ free_and_quit:
return ret;
 }
 
-static int __exit adp1653_remove(struct i2c_client *client)
+static int adp1653_remove(struct i2c_client *client)
 {
struct v4l2_subdev *subdev = i2c_get_clientdata(client);
struct adp1653_flash *flash = to_adp1653_flash(subdev);
@@ -476,7 +476,7 @@ static struct i2c_driver adp1653_i2c_driver = {
.pm = &adp1653_pm_ops,
},
.probe  = adp1653_probe,
-   .remove = __exit_p(adp1653_remove),
+   .remove = adp1653_remove,
.id_table   = adp1653_id_table,
 };
 
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 83c7ed7..cae4f46 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2833,7 +2833,7 @@ static int smiapp_probe(struct i2c_client *client,
 sensor->src->pads, 0);
 }
 
-static int __exit smiapp_remove(struct i2c_client *client)
+static int smiapp_remove(struct i2c_client *client)
 {
struct v4l2_subdev *subdev = i2c_get_clientdata(client);
struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
@@ -2881,7 +2881,7 @@ static struct i2c_driver smiapp_i2c_driver = {
.pm = &smiapp_pm_ops,
},
.probe  = smiapp_probe,
-   .remove = __exit_p(smiapp_remove),
+   .remove = smiapp_remove,
.id_table = smiapp_id_table,
 };
 
diff --git a/drivers/media/platform/soc_camera/omap1_camera.c 
b/drivers/media/platform/soc_camera/omap1_camera.c
index 39a77f0..e5091b7 100644
--- a/drivers/media/platform/soc_camera/omap1_camera.c
+++ b/drivers/media/platform/soc_camera/omap1_camera.c
@@ -1546,7 +1546,7 @@ static struct soc_camera_host_ops omap1_host_ops = {
.poll   = omap1_cam_poll,
 };
 
-static int __init omap1_cam_probe(struct platform_device *pdev)
+static int omap1_cam_probe(struct platform_device *pdev)
 {
struct omap1_cam_dev *pcdev;
struct resource *res;
@@ -1677,7 +1677,7 @@ exit:
return err;
 }
 
-static int __exit omap1_cam_remove(struct platform_device *pdev)
+static int omap1_cam_remove(struct platform_device *pdev)
 {
struct soc_camera_host *soc_host = to_soc_camera_host(&pdev->dev);
struct omap1_cam_dev *pcdev = container_of(soc_host,
@@ -1709,7 +1709,7 @@ static struct platform_driver omap1_cam_driver = {
.name   = DRIVER_NAME,
},
.probe  = omap1_cam_probe,
-   .remove = __exit_p(omap1_cam_remove),
+   .remove = omap1_cam_remove,
 };
 
 module_platform_driver(omap1_cam_driver);
diff --git a/drivers/media/radio/radio-si4713.c 
b/drivers/media/radio/radio-si4713.c
index 1507c9d..8ae8442d 100644
--- a/drivers/media/radio/radio-si4713.c
+++ b/drivers/media/radio/radio-si4713.c
@@ -328,7 +328,7 @@ exit:
 }
 
 /* radio_si4713_pdriver_remove - remove the device */
-static int __exit radio_si4713_pdriver_remove(struct platform_device *pdev)
+static int radio_si4713_pdriver_remove(struct platform_device *pdev)
 {
struct v4l2_device *v4l2_dev = platform_get_drvdata(pdev);
struct radio_si4713_device *rsdev = container_of(v4l2_dev,
@@ -350,7 +350,7 @@ static struct platform_driver radio_si4713_pdriver = {
.name   = "radio-si4713",
},
.probe  = radio_si4713_pdriver_probe,
-   .remove = __exit_p(radio_si4713_pdriver_remove),
+   .remove = radio_si4713_pdriver_remove,
 };
 
 module_platform_driver(radio_si4713_pdriver);
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 8ead492..31b955b 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -464,14 +464,14 @@ static int lirc_rx51_probe(struct platform_device *dev)
return 0;
 }
 
-static int __exit lirc_rx51_remove(struct

Re: [PATCH] ipack: add missing put_device() after device_register() failed

2013-02-26 Thread Dmitry Torokhov
On Tue, Feb 26, 2013 at 10:03:15AM +0100, Samuel Iglesias Gonsalvez wrote:
> put_device() must be called after device_register() fails,
> since device_register() always initializes the refcount
> on the device structure to one.
> 
> dev->id is free'd inside of ipack_device_release function.
> So, it's not needed to do it here.
> 
> Signed-off-by: Samuel Iglesias Gonsalvez 
> ---
>  drivers/ipack/ipack.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ipack/ipack.c b/drivers/ipack/ipack.c
> index 7ec6b20..3588ccf 100644
> --- a/drivers/ipack/ipack.c
> +++ b/drivers/ipack/ipack.c
> @@ -449,7 +449,7 @@ int ipack_device_register(struct ipack_device *dev)
>  
>   ret = device_register(&dev->dev);
>   if (ret < 0)
> - kfree(dev->id);
> + put_device(&dev->dev);

Now callers have no idea if they have to free (put_device) it in case of
failure or it is already done for them, because a few lines earlier, if
pack_device_read_id() fails, it simply returns error code.

IMO if a function did not allocate object it should not try to free it,
callers should dispose of the device as they see fit.

What is missing, however, is dev->id = NULL assignment so that if caller
does put_device() kit won't double-free the memory. Also it would make
sense to split device_register into device_init() and device_add() so
that device_init() is very first thing you do and in case of all
failures callers should use put_device().

Also tpci200.c need to learn to clean up after itself and I also not
sure why it tries to provide its own release function.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH V2] rtc: add devm_rtc_device_{register,unregister}()

2013-02-26 Thread Dmitry Torokhov
On Tue, Feb 26, 2013 at 10:21:06AM +0900, Jingoo Han wrote:
> +/**
> + * devm_rtc_device_unregister - resource managed devm_rtc_device_unregister()
> + * @dev: the device to unregister
> + * @rtc: the RTC class device to unregister
> + *
> + * Deallocated a rtc allocated with devm_rtc_device_register(). Normally this
> + * function will not need to be called and the resource management code will
> + * ensure that the resource is freed.
> + */
> +void devm_rtc_device_unregister(struct device *dev, struct rtc_device *rtc)

Why do you need a separate function? You can add a flag to struct rtc_device
so it knows whether it is devm-managed or not and behave accordingly.
And then you can do

#define devm_rtc_device_unregister rtc_device_unregister

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH] gpio: em: Add Device Tree support

2013-02-26 Thread Dmitry Torokhov
Hi Magnus,

On Tue, Feb 26, 2013 at 10:26:23PM +0900, Magnus Damm wrote:
> From: Magnus Damm 
> 
> Update the Emma Mobile GPIO driver to add DT support.
> 

...

> @@ -366,15 +387,33 @@ static int em_gio_remove(struct platform
>   return 0;
>  }
>  

#ifdef CONFIG_OF here? No need to have extra aliases in modules if OF
support is not enabled (or is entire ARM arch now enables device tree?).

> +static const struct of_device_id em_gio_dt_ids[] = {
> + { .compatible = "renesas,em-gio", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
> +
>  static struct platform_driver em_gio_device_driver = {
>   .probe  = em_gio_probe,
>   .remove = em_gio_remove,
>   .driver = {
>   .name   = "em_gio",
> + .of_match_table = em_gio_dt_ids,
> + .owner  = THIS_MODULE,
>   }
>  };
>  

-- 
Dmitry
--
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/


Re: [PATCH v4] drivers/tty: Folding Android's keyreset driver in sysRQ

2013-02-27 Thread Dmitry Torokhov
On Wed, Feb 27, 2013 at 10:58:44AM -0700, Mathieu Poirier wrote:
> On 13-02-27 09:57 AM, Linus Torvalds wrote:
> > On Tue, Feb 26, 2013 at 11:33 PM, Dave Airlie  wrote:
> >>
> >> It looks to me like the weak bit isn't working so well
> >>
> >> if (platform_sysrq_reset_seq) {
> >> for (i = 0; i < ARRAY_SIZE(sysrq_reset_seq); i++) {
> >> key = platform_sysrq_reset_seq[i];
> >>   6d:   66 8b 8c 00 00 00 00mov0x0(%eax,%eax,1),%cx
> >>   74:   00
> >>
> >> is around where it craps out.
> >> gcc version 4.7.2 20121109 (Red Hat 4.7.2-8) (GCC)
> >> Fedora 18 machine.
> > 
> > Hmm. I would love to blame gcc, but no, I think the code is crap.
> > 
> > The whole 'platform_sysrq_reset_seq[]' thing is broken in current git,
> > and it apparently only happens to work by mistake for most of us.
> > 
> > Doing a "grep" for it shows all three uses:
> > 
> >git grep platform_sysrq_reset_seq
> > 
> >   extern unsigned short platform_sysrq_reset_seq[] __weak;
> >   if (platform_sysrq_reset_seq) {
> > key = platform_sysrq_reset_seq[i];
> > 
> > and the thing is, if it is declared as an array (not a pointer), then
> > I think it is perfectly understandable that when then testing the
> > *address* of that array, gcc just says "you're stupid, you're testing
> > something that cannot possibly be NULL, so I'll throw your idiotic
> > test away".
> > 
> > And gcc would be completely correct. That test is moronic. You just
> > said that platform_sysrq_reset_seq[] was an external array, there is
> > no way in hell that is NULL.
> > 
> > Now, if it was a _pointer_, that would be a different thing entirely.
> > A pointer can have a NULL value. A named array, not so much.
> > 
> > So I *think* the fix might be something like the attached. Totally
> > untested. It may compile, or it may not.
> > 
> > Linus
> > 
> 
> Your fix is compiling, running and yielding the correct results -
> apologies about that.
> 

Actually defining platform_sysrq_reset_seq as __weak array was my doing
so the fault is actually mine. Mathieu had a function there originally.

Thanks.

-- 
Dmitry
--
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/


Re: uhid: broken interface: 32/64-bit compatibility

2013-02-18 Thread Dmitry Torokhov
Hi David,

On Fri, Feb 15, 2013 at 12:46:55PM +0100, David Herrmann wrote:
> Hi Kirill
> 
> On Fri, Feb 15, 2013 at 12:29 PM, Kirill A. Shutemov
>  wrote:
> > Hi David and all,
> >
> > There's claim in uhid.h that the interface is "compatible even between
> > architectures". But it obviously is not true: struct uhid_create_req
> > contains pointer which breaks everything.
> >
> > The easy way to demonstrate the issue is compile uhid-example.c with -m32
> > and try to run it on 64 bit kernel. Creating of the device will fail.
> 
> Indeed, we missed that. We should probably also notify the HIDP
> developers as "struct hidp_connadd_req" suffers from the same
> problems. (CC'ed)

I believe this issue has already been taken care of (see
compat_hidp_connadd_req and hidp_sock_compat_ioctl).

Thanks.

-- 
Dmitry
--
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/


Re: uhid: broken interface: 32/64-bit compatibility

2013-02-18 Thread Dmitry Torokhov
On Mon, Feb 18, 2013 at 11:28:40AM +0100, Jiri Kosina wrote:
> On Fri, 15 Feb 2013, Dmitry Torokhov wrote:
> 
> > > Here's my attempt to fix the issue.
> > > 
> > > Not sure if tricks with padding in a good idea. We can  just use __u64
> > > instead of pointer, but it will require update of userspace to silence
> > > cast warning and will cause warning if you will try to use updated
> > > userspace with old kernel headers.
> > > 
> > > Any comments?
> > 
> > This does not fix anything really, we simply have to deal with compat
> > interface.
> > 
> > Compiled but not tested.
> > 
> > Thanks.
> 
> Sorry for late response, I have been extremely busy doing some skiing :-)

Sounds good. At least someone has the right priorities ;)

-- 
Dmitry
--
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/


Re: [PATCH v4 6/6] Input: Add ChromeOS EC keyboard driver

2013-02-19 Thread Dmitry Torokhov
On Mon, Feb 18, 2013 at 11:20:43PM -0800, Joe Perches wrote:
> On Mon, 2013-02-18 at 20:13 -0800, Simon Glass wrote:
> > On Sat, Feb 16, 2013 at 12:49 PM, Dmitry Torokhov
> > > On Fri, Feb 15, 2013 at 08:16:12PM -0800, Simon Glass wrote:
> > >> + for (row = 0; row < ckdev->rows; row++) {
> > >> + if (cros_ec_keyb_row_has_ghosting(ckdev, buf, row))
> > >> + return true;
> > >> + }
> > >
> > > No need for curly braces here. I would not care if not for below.
> > 
> > OK I dont't think I even knew about that rule. Actually, what is that rule?
> 
> There is no rule, uses with and without braces
> exist in about similar numbers in the kernel.
> 
> Both are used ~2000 times.
> 
> Newer uses more commonly have braces and I think
> using braces is a better style.

I consider this example of a single statement body (albeit nested) and
therefore no braces are necessary, as mentioned in out coding style.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH] Input: psmouse - retry getid command in psmouse_probe()

2013-02-19 Thread Dmitry Torokhov
Hi Chung-yih,

On Mon, Feb 18, 2013 at 05:45:07PM +0800, Chung-Yih Wang (王崇懿) wrote:
> Yes, I could add CONFIG_MOUSE_PS2_SYNAPTICS for the change as we only
> need it for synaptics touchpad/touchpoint on lenovo's machines.
> 

I do not think it will solve anything as all distributions have
CONFIG_MOUSE_PS2_SYNAPTICS enabled.

Could you tell me what the response is to the initial GETID command that
you see on these laptops?

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH] Input: synaptics - add retry mechanism for reconnect

2013-02-19 Thread Dmitry Torokhov
Hi Chung-yih,

On Mon, Feb 18, 2013 at 04:40:21PM +0800, Chung-yih Wang wrote:
> On the Samsung Series 5 Chromebook, the Synaptics ClickPad will sometimes not
> send an "0xFA" PS/2 ACK in response to a command byte during the reconnect
> sequence following a resume from suspend. From our experiments, the failure 
> can
> happen during any byte of any of the commands in the reconnect batch.
> 
> This failure results in a timeout in the psmouse driver. In addition, the
> ClickPad will often also respond to the next PS/2 command byte with an "0xFE"
> PS/2 RESEND response.

Since you control the firmware of the device can you figure out under
what condition the touchpad does not acknowledge the commands sent to it
and fix it there?

Thanks.

-- 
Dmitry
--
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/


[git pull] Input updates for 3.9-rc0

2013-02-19 Thread Dmitry Torokhov
Hi Linus,

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
or
master.kernel.org:/pub/scm/linux/kernel/git/dtor/input.git for-linus

to receive first round of updates for the input subsystem.

You will get 2 new touchpad drivers - Cypress APA I2C Trackpad and
Cypress PS/2 touchpad and a big update to ALPS driver from Kevin
Cernekee that adds support for "Rushmore" touchpads and paves way for
adding support for "Dolphin" touchpads.

There is also a new input driver for Goldfish emulator and also Android
keyreset driver was folded into SysRq code.

A few more drivers were updated with device tree bindings and others
got some small cleanups and fixes.


Changelog:
-

Benson Leung (2):
  Input: add driver for Cypress APA I2C Trackpad
  Input: cyapa - add support for smbus protocol

Brian Swetland (1):
  Input: goldfish - virtual input event driver

Daniel Kurtz (1):
  Input: synaptics - fix 1->3 contact transition reporting

Dmitry Torokhov (6):
  Input: atkbd - fix a typo in a message
  Input: walkera0701 - set up input device's parent
  Input: walkera0701 - switch to using pr_xxx() for messages
  Input: walkera0701 - use proper error codes
  Input: walkera0701 - claim parport when opening the device
  Input: cyttsp-spi - remove duplicate MODULE_ALIAS()

Dudley Du (1):
  Input: add support for Cypress PS/2 Trackpads

Heiko Carstens (1):
  Input: add couple of missing GENERIC_HARDIRQS dependencies

Henrik Rydberg (2):
  Input: MT - do not apply filtering on emulated events
  Input: synaptics - initialize pointer emulation usage

Javier Martin (1):
  Input: qt2160 - add support for LEDs

Kamal Mostafa (1):
  Input: increase struct ps2dev cmdbuf[] to 8 bytes

Kevin Cernekee (13):
  Input: ALPS - document the alps.h data structures
  Input: ALPS - copy "model" info into alps_data struct
  Input: ALPS - move alps_get_model() down below hw_init code
  Input: ALPS - introduce helper function for repeated commands
  Input: ALPS - rework detection sequence
  Input: ALPS - use function pointers for different protocol handlers
  Input: ALPS - move {addr,nibble}_command settings into alps_set_defaults()
  Input: ALPS - rework detection of Pinnacle AGx touchpads
  Input: ALPS - fix command mode check
  Input: ALPS - move pixel and bitmap info into alps_data struct
  Input: ALPS - make the V3 packet field decoder "pluggable"
  Input: ALPS - add support for "Rushmore" touchpads
  Input: ALPS - enable trackstick on Rushmore touchpads

Laxman Dewangan (4):
  Input: tegra-kbc - fix build warning
  Input: tegra-kbc - use devm_* for resource allocation
  Input: tegra-kbc - add support for rows/columns configuration from dt
  Input: tegra-kbc - remove default keymap

Liu Ying (1):
  Input: imx_keypad - add device tree support

Mark Brown (2):
  Input: wm831x-ts - convert to devm_input_allocate_device()
  Input: wm831x-on - convert to devm_input_allocate_device()

Mathieu Poirier (1):
  Input: sysrq - allow specifying alternate reset sequence

Michael Trimarchi (2):
  Input: bma150 - fix checking pm_runtime_get_sync() return value
  Input: bma150 - make some defines public and fix some comments

Pali Rohár (1):
  Input: tsc2005 - add MODULE_ALIAS

Peter Ujfalusi (4):
  Input: twl4030-vibra - switch to using managed resources
  Input: twl4030-vibra - Use system workqueue
  Input: twl6040-vibra - code cleanup in probe with devm_* conversion
  Input: twl6040-vibra - use system workqueue

Ping Cheng (3):
  Input: wacom - prepare for syncing with input-mt changes
  Input: wacom - use new input-mt routines
  Input: wacom - add support for DTH-2242

Sachin Kamat (1):
  Input: mms114 - switch to using managed resources

Shawn Nematbakhsh (1):
  Input: atkbd - fix multi-byte scancode handling on reconnect

Stephen Warren (1):
  Input: tegra-kbc - require CONFIG_OF, remove platform data

Vipul Kumar Samar (1):
  Input: stmpe-ts - report BTN_TOUCH event

Wolfram Sang (4):
  Input: adxl34x - consistently use read/write encapsulation
  Input: adxl34x - don't set THRESH_TAP twice
  Input: adxl34x - make platform_data include self contained
  Input: adxl34x - default platform_data should not use defines from driver


Diffstat:


 .../devicetree/bindings/input/imx-keypad.txt   |  53 ++
 .../bindings/input/nvidia,tegra20-kbc.txt  |  22 +
 drivers/input/Kconfig  |   2 +-
 drivers/input/input-mt.c   |   1 +
 drivers/input/joystick/walkera0701.c   |  82 +-
 drivers/input/keyboard/Kconfig |  16 +-
 drivers/input/keyboard/Makefile|   1 +
 drivers/input/keyboard/atkbd.c

Re: [PATCH 07/11] mfd: menelaus: use devm_request_irq() and devm_kzalloc()

2013-02-19 Thread Dmitry Torokhov
Hi Jongoo,

On Wed, Feb 20, 2013 at 03:12:38PM +0900, Jingoo Han wrote:
> Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> more simple.
> 

...

> @@ -1269,9 +1266,7 @@ static int __exit menelaus_remove(struct i2c_client 
> *client)
>  {
>   struct menelaus_chip*menelaus = i2c_get_clientdata(client);
>  
> - free_irq(client->irq, menelaus);
>   flush_work(&menelaus->work);
> - kfree(menelaus);
>   the_menelaus = NULL;
>   return 0;

This conversion is certainly wrong - you really want to disable IRQ and
then wait for the scheduled work to finish before freeing memory. Here
you flush work but nothing stops IRQ from firing and scheduling that
work again.

Please, be *extra* careful with devm_request_irq() conversions.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 08/11] mfd: ezx-pcap: use devm_request_irq() and devm_kzalloc()

2013-02-19 Thread Dmitry Torokhov
On Wed, Feb 20, 2013 at 03:13:06PM +0900, Jingoo Han wrote:
> Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> more simple.
> 
> Signed-off-by: Jingoo Han 
> ---
>  drivers/mfd/ezx-pcap.c |   16 +---
>  1 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mfd/ezx-pcap.c b/drivers/mfd/ezx-pcap.c
> index b7a61f0..8dea3a9 100644
> --- a/drivers/mfd/ezx-pcap.c
> +++ b/drivers/mfd/ezx-pcap.c
> @@ -403,7 +403,6 @@ static int ezx_pcap_remove(struct spi_device *spi)
>   /* cleanup ADC */
>   adc_irq = pcap_to_irq(pcap, (pdata->config & PCAP_SECOND_PORT) ?
>   PCAP_IRQ_ADCDONE2 : PCAP_IRQ_ADCDONE);
> - free_irq(adc_irq, pcap);
>   mutex_lock(&pcap->adc_mutex);
>   for (i = 0; i < PCAP_ADC_MAXQ; i++)
>   kfree(pcap->adc_queue[i]);
> @@ -415,8 +414,6 @@ static int ezx_pcap_remove(struct spi_device *spi)
>  
>   destroy_workqueue(pcap->workqueue);
>  
> - kfree(pcap);
> -

I am pretty sure this conversion is wrong as well. Pretty much
work/workqueue and devm_request_irq() do not mix.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 10/11] mfd: tps65010: use devm_request_irq() and devm_kzalloc()

2013-02-19 Thread Dmitry Torokhov
On Wed, Feb 20, 2013 at 03:14:05PM +0900, Jingoo Han wrote:
> Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> more simple.
> 
> Signed-off-by: Jingoo Han 
> ---
>  drivers/mfd/tps65010.c |   15 +--
>  1 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mfd/tps65010.c b/drivers/mfd/tps65010.c
> index da2691f..a5438cc 100644
> --- a/drivers/mfd/tps65010.c
> +++ b/drivers/mfd/tps65010.c
> @@ -525,11 +525,8 @@ static int __exit tps65010_remove(struct i2c_client 
> *client)
>   dev_dbg(&client->dev, "board %s %s err %d\n",
>   "teardown", client->name, status);
>   }
> - if (client->irq > 0)
> - free_irq(client->irq, tps);
>   cancel_delayed_work_sync(&tps->work);
>   debugfs_remove(tps->file);
> - kfree(tps);
>   the_tps = NULL;
>   return 0;

Nope, again work and IRQ have to be handled in certain order.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 05/11] mfd: twl6040: use devm_gpio_request_one() and devm_request_threaded_irq()

2013-02-19 Thread Dmitry Torokhov
Hi Jingoo,

On Wed, Feb 20, 2013 at 03:11:38PM +0900, Jingoo Han wrote:
> Use devm_gpio_request_one() and devm_request_threaded_irq() to make
> cleanup paths and  more simple.
> 
> Signed-off-by: Jingoo Han 
> ---
>  drivers/mfd/twl6040.c |   26 +++---
>  1 files changed, 7 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> index f361bf3..2d38512 100644
> --- a/drivers/mfd/twl6040.c
> +++ b/drivers/mfd/twl6040.c
> @@ -586,8 +586,8 @@ static int twl6040_probe(struct i2c_client *client,
>   twl6040->audpwron = -EINVAL;
>  
>   if (gpio_is_valid(twl6040->audpwron)) {
> - ret = gpio_request_one(twl6040->audpwron, GPIOF_OUT_INIT_LOW,
> -"audpwron");
> + ret = devm_gpio_request_one(twl6040->dev, twl6040->audpwron,
> + GPIOF_OUT_INIT_LOW, "audpwron");
>   if (ret)
>   goto gpio_err;
>   }
> @@ -596,14 +596,14 @@ static int twl6040_probe(struct i2c_client *client,
>   IRQF_ONESHOT, 0, &twl6040_irq_chip,
>   &twl6040->irq_data);
>   if (ret < 0)
> - goto irq_init_err;
> + goto gpio_err;
>  
>   twl6040->irq_ready = regmap_irq_get_virq(twl6040->irq_data,
>  TWL6040_IRQ_READY);
>   twl6040->irq_th = regmap_irq_get_virq(twl6040->irq_data,
>  TWL6040_IRQ_TH);
>  
> - ret = request_threaded_irq(twl6040->irq_ready, NULL,
> + ret = devm_request_threaded_irq(twl6040->dev, twl6040->irq_ready, NULL,
>  twl6040_readyint_handler, IRQF_ONESHOT,
>  "twl6040_irq_ready", twl6040);
>   if (ret) {
> @@ -611,12 +611,12 @@ static int twl6040_probe(struct i2c_client *client,
>   goto readyirq_err;
>   }
>  
> - ret = request_threaded_irq(twl6040->irq_th, NULL,
> + ret = devm_request_threaded_irq(twl6040->dev, twl6040->irq_th, NULL,
>  twl6040_thint_handler, IRQF_ONESHOT,
>  "twl6040_irq_th", twl6040);
>   if (ret) {
>   dev_err(twl6040->dev, "Thermal IRQ request failed: %d\n", ret);
> - goto thirq_err;
> + goto readyirq_err;
>   }
>  
>   /* dual-access registers controlled by I2C only */
> @@ -676,19 +676,12 @@ static int twl6040_probe(struct i2c_client *client,
>   ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
> NULL, 0, NULL);
>   if (ret)
> - goto mfd_err;
> + goto readyirq_err;
>  
>   return 0;
>  
> -mfd_err:
> - free_irq(twl6040->irq_th, twl6040);
> -thirq_err:
> - free_irq(twl6040->irq_ready, twl6040);
>  readyirq_err:
>   regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
> -irq_init_err:
> - if (gpio_is_valid(twl6040->audpwron))
> - gpio_free(twl6040->audpwron);
>  gpio_err:
>   regulator_bulk_disable(TWL6040_NUM_SUPPLIES, twl6040->supplies);
>  power_err:
> @@ -706,11 +699,6 @@ static int twl6040_remove(struct i2c_client *client)
>   if (twl6040->power_count)
>   twl6040_power(twl6040, 0);
>  
> - if (gpio_is_valid(twl6040->audpwron))
> - gpio_free(twl6040->audpwron);
> -
> - free_irq(twl6040->irq_ready, twl6040);
> - free_irq(twl6040->irq_th, twl6040);
>   regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
>  
>   mfd_remove_devices(&client->dev);

Are you sure it is OK to have sub-devices removed and regmap destroyed
with IRQs still active?

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 07/11] mfd: menelaus: use devm_request_irq() and devm_kzalloc()

2013-02-20 Thread Dmitry Torokhov
On Wed, Feb 20, 2013 at 05:05:10PM +0900, Jingoo Han wrote:
> On Wednesday, February 20, 2013 4:31 PM, Dmitry Torokhov wrote:
> > 
> > Hi Jongoo,
> > 
> > On Wed, Feb 20, 2013 at 03:12:38PM +0900, Jingoo Han wrote:
> > > Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> > > more simple.
> > >
> > 
> > ...
> > 
> > > @@ -1269,9 +1266,7 @@ static int __exit menelaus_remove(struct i2c_client 
> > > *client)
> > >  {
> > >   struct menelaus_chip*menelaus = i2c_get_clientdata(client);
> > >
> > > - free_irq(client->irq, menelaus);
> > >   flush_work(&menelaus->work);
> > > - kfree(menelaus);
> > >   the_menelaus = NULL;
> > >   return 0;
> > 
> > This conversion is certainly wrong - you really want to disable IRQ and
> > then wait for the scheduled work to finish before freeing memory. Here
> > you flush work but nothing stops IRQ from firing and scheduling that
> > work again.
> 
> Yes, you're right. 
> I will use devm_free_irq() before flush_work().

Why change it at all if you have to call it manually in both error
unwinding and menelaus_remove() cases?

BTW, that __exit markup on menelaus_remove() is surprising... I am
pretty sure it can be unbound via sysfs and so there will be a nasty
oops.

-- 
Dmitry
--
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/


Re: [PATCH 07/11] mfd: menelaus: use devm_request_irq() and devm_kzalloc()

2013-02-20 Thread Dmitry Torokhov
On Wed, Feb 20, 2013 at 05:17:30PM +0900, Jingoo Han wrote:
> On Wednesday, February 20, 2013 5:10 PM, Dmitry Torokhov wrote:
> > 
> > On Wed, Feb 20, 2013 at 05:05:10PM +0900, Jingoo Han wrote:
> > > On Wednesday, February 20, 2013 4:31 PM, Dmitry Torokhov wrote:
> > > >
> > > > Hi Jongoo,
> > > >
> > > > On Wed, Feb 20, 2013 at 03:12:38PM +0900, Jingoo Han wrote:
> > > > > Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> > > > > more simple.
> > > > >
> > > >
> > > > ...
> > > >
> > > > > @@ -1269,9 +1266,7 @@ static int __exit menelaus_remove(struct 
> > > > > i2c_client *client)
> > > > >  {
> > > > >   struct menelaus_chip*menelaus = i2c_get_clientdata(client);
> > > > >
> > > > > - free_irq(client->irq, menelaus);
> > > > >   flush_work(&menelaus->work);
> > > > > - kfree(menelaus);
> > > > >   the_menelaus = NULL;
> > > > >   return 0;
> > > >
> > > > This conversion is certainly wrong - you really want to disable IRQ and
> > > > then wait for the scheduled work to finish before freeing memory. Here
> > > > you flush work but nothing stops IRQ from firing and scheduling that
> > > > work again.
> > >
> > > Yes, you're right.
> > > I will use devm_free_irq() before flush_work().
> > 
> > Why change it at all if you have to call it manually in both error
> > unwinding and menelaus_remove() cases?
> > 
> > BTW, that __exit markup on menelaus_remove() is surprising... I am
> > pretty sure it can be unbound via sysfs and so there will be a nasty
> > oops.
> 
> I see, I will not modify this menelaus mfd driver.
> Thank you.

Same goes for the other drivers that can't safely use devm_request_irq()
please.

Thanks.

-- 
Dmitry
--
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/


[PATCH] hp-wmi: fix handling of platform device

2013-02-20 Thread Dmitry Torokhov
The driver will not quite work if someone unbinds the platform device
from the platform driver via sysfs (moreover it will bomb is the driver
built into the kernel as hp_wmi_bios_remove is marked as __exit and will
not be present in the kernel).

To fix it let's use platform_driver_probe() instead of
platform_driver_register(), which disables binding/unbinding via sysfs.
This also allows us to mark hp_wmi_bios_setup as __init and discard it
once module is initialized.

Signed-off-by: Dmitry Torokhov 
---

Compiled only, no hardware to run.

 drivers/platform/x86/hp-wmi.c | 69 +++
 1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 1dde7ac..de78024 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -72,10 +72,6 @@ enum hp_wmi_event_ids {
HPWMI_LOCK_SWITCH = 7,
 };
 
-static int hp_wmi_bios_setup(struct platform_device *device);
-static int __exit hp_wmi_bios_remove(struct platform_device *device);
-static int hp_wmi_resume_handler(struct device *device);
-
 struct bios_args {
u32 signature;
u32 command;
@@ -157,21 +153,6 @@ struct rfkill2_device {
 static int rfkill2_count;
 static struct rfkill2_device rfkill2[HPWMI_MAX_RFKILL2_DEVICES];
 
-static const struct dev_pm_ops hp_wmi_pm_ops = {
-   .resume  = hp_wmi_resume_handler,
-   .restore  = hp_wmi_resume_handler,
-};
-
-static struct platform_driver hp_wmi_driver = {
-   .driver = {
-   .name = "hp-wmi",
-   .owner = THIS_MODULE,
-   .pm = &hp_wmi_pm_ops,
-   },
-   .probe = hp_wmi_bios_setup,
-   .remove = hp_wmi_bios_remove,
-};
-
 /*
  * hp_wmi_perform_query
  *
@@ -778,7 +759,7 @@ fail:
return err;
 }
 
-static int hp_wmi_bios_setup(struct platform_device *device)
+static int __init hp_wmi_bios_setup(struct platform_device *device)
 {
int err;
 
@@ -874,12 +855,29 @@ static int hp_wmi_resume_handler(struct device *device)
return 0;
 }
 
+static const struct dev_pm_ops hp_wmi_pm_ops = {
+   .resume  = hp_wmi_resume_handler,
+   .restore  = hp_wmi_resume_handler,
+};
+
+static struct platform_driver hp_wmi_driver = {
+   .driver = {
+   .name = "hp-wmi",
+   .owner = THIS_MODULE,
+   .pm = &hp_wmi_pm_ops,
+   },
+   .remove = __exit_p(hp_wmi_bios_remove),
+};
+
 static int __init hp_wmi_init(void)
 {
int err;
int event_capable = wmi_has_guid(HPWMI_EVENT_GUID);
int bios_capable = wmi_has_guid(HPWMI_BIOS_GUID);
 
+   if (!bios_capable && !event_capable)
+   return -ENODEV;
+
if (event_capable) {
err = hp_wmi_input_setup();
if (err)
@@ -887,34 +885,29 @@ static int __init hp_wmi_init(void)
}
 
if (bios_capable) {
-   err = platform_driver_register(&hp_wmi_driver);
-   if (err)
-   goto err_driver_reg;
-   hp_wmi_platform_dev = platform_device_alloc("hp-wmi", -1);
-   if (!hp_wmi_platform_dev) {
-   err = -ENOMEM;
-   goto err_device_alloc;
+   hp_wmi_platform_dev =
+   platform_device_register_simple("hp-wmi", -1, NULL, 0);
+   if (IS_ERR(hp_wmi_platform_dev)) {
+   err = PTR_ERR(hp_wmi_platform_dev);
+   goto err_destroy_input;
}
-   err = platform_device_add(hp_wmi_platform_dev);
+
+   err = platform_driver_probe(&hp_wmi_driver, hp_wmi_bios_setup);
if (err)
-   goto err_device_add;
+   goto err_unregister_device;
}
 
-   if (!bios_capable && !event_capable)
-   return -ENODEV;
-
return 0;
 
-err_device_add:
-   platform_device_put(hp_wmi_platform_dev);
-err_device_alloc:
-   platform_driver_unregister(&hp_wmi_driver);
-err_driver_reg:
+err_unregister_device:
+   platform_device_unregister(hp_wmi_platform_dev);
+err_destroy_input:
if (event_capable)
hp_wmi_input_destroy();
 
return err;
 }
+module_init(hp_wmi_init);
 
 static void __exit hp_wmi_exit(void)
 {
@@ -926,6 +919,4 @@ static void __exit hp_wmi_exit(void)
platform_driver_unregister(&hp_wmi_driver);
}
 }
-
-module_init(hp_wmi_init);
 module_exit(hp_wmi_exit);
-- 
1.7.11.7

-- 
Dmitry
--
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/


[PATCH] I2C: i2c-pxa - remove incorrect __exit annotations

2013-02-20 Thread Dmitry Torokhov
The remove() methods should not be marked __exit unless we are using
platform_driver_probe() which disables unbinding device from driver
via sysfs.

Signed-off-by: Dmitry Torokhov 
---

Compiled only, no hardware.

 drivers/i2c/busses/i2c-pxa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..4a79e76 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1215,7 +1215,7 @@ emalloc:
return ret;
 }
 
-static int __exit i2c_pxa_remove(struct platform_device *dev)
+static int i2c_pxa_remove(struct platform_device *dev)
 {
struct pxa_i2c *i2c = platform_get_drvdata(dev);
 
@@ -1269,7 +1269,7 @@ static const struct dev_pm_ops i2c_pxa_dev_pm_ops = {
 
 static struct platform_driver i2c_pxa_driver = {
.probe  = i2c_pxa_probe,
-   .remove = __exit_p(i2c_pxa_remove),
+   .remove = i2c_pxa_remove,
.driver = {
.name   = "pxa2xx-i2c",
.owner  = THIS_MODULE,
-- 
1.7.11.7


-- 
Dmitry
--
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/


[PATCH] MFD: remove incorrect __exit annotations

2013-02-20 Thread Dmitry Torokhov
Even if bus is not hot-pluggable, the devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods.

Signed-off-by: Dmitry Torokhov 
---

Not tested - no hardware.

 drivers/mfd/menelaus.c | 4 ++--
 drivers/mfd/tps65010.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/menelaus.c b/drivers/mfd/menelaus.c
index 998ce8c..fda3211 100644
--- a/drivers/mfd/menelaus.c
+++ b/drivers/mfd/menelaus.c
@@ -1265,7 +1265,7 @@ fail1:
return err;
 }
 
-static int __exit menelaus_remove(struct i2c_client *client)
+static int menelaus_remove(struct i2c_client *client)
 {
struct menelaus_chip*menelaus = i2c_get_clientdata(client);
 
@@ -1287,7 +1287,7 @@ static struct i2c_driver menelaus_i2c_driver = {
.name   = DRIVER_NAME,
},
.probe  = menelaus_probe,
-   .remove = __exit_p(menelaus_remove),
+   .remove = menelaus_remove,
.id_table   = menelaus_id,
 };
 
diff --git a/drivers/mfd/tps65010.c b/drivers/mfd/tps65010.c
index da2691f..9030233 100644
--- a/drivers/mfd/tps65010.c
+++ b/drivers/mfd/tps65010.c
@@ -514,7 +514,7 @@ static int tps65010_gpio_get(struct gpio_chip *chip, 
unsigned offset)
 
 static struct tps65010 *the_tps;
 
-static int __exit tps65010_remove(struct i2c_client *client)
+static int tps65010_remove(struct i2c_client *client)
 {
struct tps65010 *tps = i2c_get_clientdata(client);
struct tps65010_board   *board = client->dev.platform_data;
@@ -687,7 +687,7 @@ static struct i2c_driver tps65010_driver = {
.name   = "tps65010",
},
.probe  = tps65010_probe,
-   .remove = __exit_p(tps65010_remove),
+   .remove = tps65010_remove,
.id_table = tps65010_id,
 };
 
-- 
1.7.11.7


-- 
Dmitry
--
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/


Re: [PATCH v5 2/6] mfd: Add ChromeOS EC implementation

2013-02-20 Thread Dmitry Torokhov
On Wed, Feb 20, 2013 at 09:24:35AM -0800, Simon Glass wrote:
> This is the base EC implementation, which provides a high level
> interface to the EC for use by the rest of the kernel. The actual
> communcations is dealt with by a separate protocol driver which
> registers itself with this interface.
> 
> Interrupts are passed on through a notifier.
> 
> A simple message structure is used to pass messages to the
> protocol driver.
> Signed-off-by: Simon Glass 
> Signed-off-by: Che-Liang Chiou 
> Signed-off-by: Jonathan Kliegman 
> Signed-off-by: Luigi Semenzato 
> Signed-off-by: Olof Johansson 
> Signed-off-by: Vincent Palatin 

...

>  
> +config MFD_CROS_EC
> + bool "Support ChromeOS Embedded Controller"
> + help
> +   If you say Y here you get support for the ChromeOS Embedded
> +   Controller (EC) providing keyboard, battery and power services.
> +   You also ned to enable the driver for the bus you are using. The
> +   protocol for talking to the EC is defined by the bus driver.

Why can't it be a module?

> +
> +#ifndef __LINUX_MFD_CROS_EC_H
> +#define __LINUX_MFD_CROS_EC_H
> +
> +struct i2c_msg;

I do not believe this is needed here.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH v5 5/6] Input: matrix-keymap: Add function to read the new DT binding

2013-02-20 Thread Dmitry Torokhov
On Wed, Feb 20, 2013 at 09:24:38AM -0800, Simon Glass wrote:
> We now have a binding which adds two parameters to the matrix keypad DT
> node. This is separate from the GPIO-driven matrix keypad binding, and
> unfortunately incompatible, since that uses row-gpios/col-gpios for the
> row and column counts.
> 
> So the easiest option here is to provide a function for non-GPIO drivers
> to use to decode the binding.
> 
> Note: We could in fact create an entirely separate structure to hold
> these two fields, but it does not seem worth it, yet. If we have more
> parameters then we can add this, and then refactor each driver to hold
> such a structure.
> 
> Signed-off-by: Simon Glass 
> Tested-by: Sourav Poddar  (v2)

Acked-by: Dmitry Torokhov 

I suppose the EC driver will go through MFD tree, right?

-- 
Dmitry
--
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/


Re: [PATCH v5 6/6] Input: Add ChromeOS EC keyboard driver

2013-02-20 Thread Dmitry Torokhov
On Wed, Feb 20, 2013 at 09:24:39AM -0800, Simon Glass wrote:
> +
> + code = keycodes[pos];
> + new_state = kb_state[col] & (1 << row);
> + if (!!new_state != test_bit(code, idev->key)) {
> + dev_dbg(ckdev->dev,
> + "changed: [r%d c%d]: byte %02x\n",
> + row, col, new_state);
> +
> + input_report_key(idev, code, new_state);

Incorrect indentation.


> + }

...

> +static int cros_ec_keyb_probe(struct platform_device *pdev)
> +{
> + struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> + struct device *dev = ec->dev;
> + struct cros_ec_keyb *ckdev = NULL;
> + struct input_dev *idev = NULL;

Why are we assigning these to NULL?

Otherwise

Acked-by: Dmitry Torokhov 

-- 
Dmitry
--
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/


Re: [PATCH v5 3/6] mfd: Add ChromeOS EC I2C driver

2013-02-20 Thread Dmitry Torokhov
On Wed, Feb 20, 2013 at 09:24:36AM -0800, Simon Glass wrote:
> +#ifdef DEBUG
> + dev_dbg(ec_dev->dev, "packet: ");
> + for (i = 0; i < i2c_msg[1].len; i++)
> + dev_cont(ec_dev->dev, " %02x", in_buf[i]);
> + dev_cont(ec_dev->dev, ", sum = %02x\n", sum);

There is "%ph" now so you can simply write:

dev_dbg(ec_dev->dev, "packet: %*ph, sum = %02x\n",
i2c_msg[1].len, in_buf[i], sum);

> +#endif

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH] Input: fix Cypress PS/2 Trackpad in Dell XPS12

2013-02-21 Thread Dmitry Torokhov
On Thu, Feb 21, 2013 at 10:55:55AM -0800, Kamal Mostafa wrote:
> Avoid firmware glitch in Cypress PS/2 Trackpad firmware version 11
> (as observed in Dell XPS12) which prevents driver from recognizing
> the trackpad.
> 
> BugLink: http://launchpad.net/bugs/1103594
> 
> Signed-off-by: Kamal Mostafa 
> Cc: Dudley Du 
> ---
>  drivers/input/mouse/cypress_ps2.c |   19 +--
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/mouse/cypress_ps2.c 
> b/drivers/input/mouse/cypress_ps2.c
> index 1673dc6..f51765f 100644
> --- a/drivers/input/mouse/cypress_ps2.c
> +++ b/drivers/input/mouse/cypress_ps2.c
> @@ -236,6 +236,13 @@ static int cypress_read_fw_version(struct psmouse 
> *psmouse)
>   cytp->fw_version = param[2] & FW_VERSION_MASX;
>   cytp->tp_metrics_supported = (param[2] & TP_METRICS_MASK) ? 1 : 0;
>  
> + /*
> +  * Trackpad fw_version 11 (in Dell XPS12) yields a bogus response to
> +  * CYTP_CMD_READ_TP_METRICS so do not try to use it. LP: #1103594.
> +  */
> + if (cytp->fw_version >= 11)
> + cytp->tp_metrics_supported = 0;
> +

Isn't this the only chunk that is actually needed to fix the issue?

Thanks.

>   psmouse_dbg(psmouse, "cytp->fw_version = %d\n", cytp->fw_version);
>   psmouse_dbg(psmouse, "cytp->tp_metrics_supported = %d\n",
>cytp->tp_metrics_supported);
> @@ -258,6 +265,9 @@ static int cypress_read_tp_metrics(struct psmouse 
> *psmouse)
>   cytp->tp_res_x = cytp->tp_max_abs_x / cytp->tp_width;
>   cytp->tp_res_y = cytp->tp_max_abs_y / cytp->tp_high;
>  
> + if (!cytp->tp_metrics_supported)
> + return 0;
> +
>   memset(param, 0, sizeof(param));
>   if (cypress_send_ext_cmd(psmouse, CYTP_CMD_READ_TP_METRICS, param) == 
> 0) {
>   /* Update trackpad parameters. */
> @@ -315,18 +325,15 @@ static int cypress_read_tp_metrics(struct psmouse 
> *psmouse)
>  
>  static int cypress_query_hardware(struct psmouse *psmouse)
>  {
> - struct cytp_data *cytp = psmouse->private;
>   int ret;
>  
>   ret = cypress_read_fw_version(psmouse);
>   if (ret)
>   return ret;
>  
> - if (cytp->tp_metrics_supported) {
> - ret = cypress_read_tp_metrics(psmouse);
> - if (ret)
> - return ret;
> - }
> + ret = cypress_read_tp_metrics(psmouse);
> + if (ret)
> + return ret;
>  
>   return 0;
>  }
> -- 
> 1.7.10.4
> 

-- 
Dmitry
--
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/


Re: [PATCH] Input: fix Cypress PS/2 Trackpad in Dell XPS12

2013-02-21 Thread Dmitry Torokhov
On Thu, Feb 21, 2013 at 12:02:46PM -0800, Kamal Mostafa wrote:
> On Thu, 2013-02-21 at 11:56 -0800, Dmitry Torokhov wrote:
> > On Thu, Feb 21, 2013 at 10:55:55AM -0800, Kamal Mostafa wrote:
> > > Avoid firmware glitch in Cypress PS/2 Trackpad firmware version 11
> > > (as observed in Dell XPS12) which prevents driver from recognizing
> > > the trackpad.
> > > 
> > > BugLink: http://launchpad.net/bugs/1103594
> > > 
> > > Signed-off-by: Kamal Mostafa 
> > > Cc: Dudley Du 
> > > ---
> > >  drivers/input/mouse/cypress_ps2.c |   19 +--
> > >  1 file changed, 13 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/input/mouse/cypress_ps2.c 
> > > b/drivers/input/mouse/cypress_ps2.c
> > > index 1673dc6..f51765f 100644
> > > --- a/drivers/input/mouse/cypress_ps2.c
> > > +++ b/drivers/input/mouse/cypress_ps2.c
> > > @@ -236,6 +236,13 @@ static int cypress_read_fw_version(struct psmouse 
> > > *psmouse)
> > >   cytp->fw_version = param[2] & FW_VERSION_MASX;
> > >   cytp->tp_metrics_supported = (param[2] & TP_METRICS_MASK) ? 1 : 0;
> > >  
> > > + /*
> > > +  * Trackpad fw_version 11 (in Dell XPS12) yields a bogus response to
> > > +  * CYTP_CMD_READ_TP_METRICS so do not try to use it. LP: #1103594.
> > > +  */
> > > + if (cytp->fw_version >= 11)
> > > + cytp->tp_metrics_supported = 0;
> > > +
> > 
> > Isn't this the only chunk that is actually needed to fix the issue?
> 
> No, the other parts are needed also:  The handling of
> tp_metrics_supported gets moved so that the default values now get set
> first, then overridden only if tp_metrics_supported is true.
> Previously, the defaults wouldn't get set if it wasn't true (which was a
> bug that never manifested).

OK, fair enough.

BTW, can you tell me what server you published your PGP key to? It does
not seem to be present on the usual ones...

Thanks,
Dmitry

-- 
Dmitry
--
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/


Dangerous devm_request_irq() conversions

2013-02-21 Thread Dmitry Torokhov
Hi,

It looks like a whole slew of devm_request_irq() conversions just got
applied to mainline and many of them are quite broken.

Consider fd5231ce336e038037b4f0190a6838bdd6e17c6d or
c1879fe80c61f3be6f2ddb82509c2e7f92a484fe: the drivers udsed first to
free IRQ and then unregister the corresponding device ensuring that IRQ
handler, while it runs, has the device available. The mechanic
conversion to devm_request_irq() reverses the order of these operations
opening the race window where IRQ can reference device (or other
resource) that is already gone.

It would be nice if these could be reverted and revioewed again for
correctness.

In general any conversion to devm_request_irq() needs double and triple
checking.

Thanks.

-- 
Dmitry
--
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/


Re: Dangerous devm_request_irq() conversions

2013-02-21 Thread Dmitry Torokhov
On Fri, Feb 22, 2013 at 04:12:36PM +0900, Jingoo Han wrote:
> On Friday, February 22, 2013 3:54 PM, Dmitry Torokhov wrote:
> > 
> > Hi,
> > 
> > It looks like a whole slew of devm_request_irq() conversions just got
> > applied to mainline and many of them are quite broken.
> > 
> > Consider fd5231ce336e038037b4f0190a6838bdd6e17c6d or
> > c1879fe80c61f3be6f2ddb82509c2e7f92a484fe: the drivers udsed first to
> > free IRQ and then unregister the corresponding device ensuring that IRQ
> > handler, while it runs, has the device available. The mechanic
> > conversion to devm_request_irq() reverses the order of these operations
> > opening the race window where IRQ can reference device (or other
> > resource) that is already gone.
> > 
> > It would be nice if these could be reverted and revioewed again for
> > correctness.
> 
> Um, other RTC drivers already have been using devm_request_threaded_irq() or
> devm_request_irq() like this, before I added these patches.
> 
> For example, 
> ./drivers/rtc/rtc-tegra.c
> ./drivers/rtc/rtc-spear.c
> ./drivers/rtc/rtc-s3c.c
> ./drivers/rtc/rtc-mxc.c
> ./drivers/rtc/rtc-ds1553.c
> ./drivers/rtc/rtc-ds1511.c
> ./drivers/rtc/rtc-snvs.c
> ./drivers/rtc/rtc-imxdi.c
> ./drivers/rtc/rtc-tx4939.c
> ./drivers/rtc/rtc-mv.c
> ./drivers/rtc/rtc-coh901331.c
> ./drivers/rtc/rtc-stk17ta8.c
> ./drivers/rtc/rtc-lpc32xx.c
> ./drivers/rtc/rtc-tps65910.c
> ./drivers/rtc/rtc-rc5t583.c
> 
> 
> Also, even more, some RTC drivers calls rtc_device_unregister() first,
> then calls free_irq() later.
> 
> For example,
> ./drivers/rtc/rtc-vr41xx.c
> ./drivers/rtc/rtc-da9052.c
> ./drivers/rtc/rtc-isl1208.c
> ./drivers/rtc/rtc-88pm860x.c
> ./drivers/rtc/rtc-tps6586x.c
> ./drivers/rtc/rtc-mpc5121.c
> ./drivers/rtc/rtc-m48t59.c
> 
> 
> Please, don't argue revert without concrete reasons.

What more concrete reason do you need? I explained to you the exact
reason on the patches I noticed before and also on the 2 commits
referenced above: blind conversion to devm_* changes order of operation
which may be deadly with IRQs (but others, like clocks and regulators,
are important too).

The fact that crap slipped in the kernel before is not the valid reason
for adding more of the same crap.

Please *understand* APIs you are using before making changes.

> 
> If these devm_request_threaded_irq() or devm_request_irq() make the problem,
> devm_free_irq() will be added later.

And the point? If you use devm_request_irq() and then call
devm_free_irq() manually in all paths what you achieved is waste of
memory required for devm_* tracking.

-- 
Dmitry
--
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/


Re: Dangerous devm_request_irq() conversions

2013-02-22 Thread Dmitry Torokhov
On Fri, Feb 22, 2013 at 04:57:29PM +0900, Jingoo Han wrote:
> On Friday, February 22, 2013 4:27 PM, Dmitry Torokhov wrote:
> > On Fri, Feb 22, 2013 at 04:12:36PM +0900, Jingoo Han wrote:
> > > On Friday, February 22, 2013 3:54 PM, Dmitry Torokhov wrote:
> > > >
> > > > Hi,
> > > >
> > > > It looks like a whole slew of devm_request_irq() conversions just got
> > > > applied to mainline and many of them are quite broken.
> > > >
> > > > Consider fd5231ce336e038037b4f0190a6838bdd6e17c6d or
> > > > c1879fe80c61f3be6f2ddb82509c2e7f92a484fe: the drivers udsed first to
> > > > free IRQ and then unregister the corresponding device ensuring that IRQ
> > > > handler, while it runs, has the device available. The mechanic
> > > > conversion to devm_request_irq() reverses the order of these operations
> > > > opening the race window where IRQ can reference device (or other
> > > > resource) that is already gone.
> > > >
> > > > It would be nice if these could be reverted and revioewed again for
> > > > correctness.
> > >
> > > Um, other RTC drivers already have been using devm_request_threaded_irq() 
> > > or
> > > devm_request_irq() like this, before I added these patches.
> > >
> > > For example,
> > > ./drivers/rtc/rtc-tegra.c
> > > ./drivers/rtc/rtc-spear.c
> > > ./drivers/rtc/rtc-s3c.c
> > > ./drivers/rtc/rtc-mxc.c
> > > ./drivers/rtc/rtc-ds1553.c
> > > ./drivers/rtc/rtc-ds1511.c
> > > ./drivers/rtc/rtc-snvs.c
> > > ./drivers/rtc/rtc-imxdi.c
> > > ./drivers/rtc/rtc-tx4939.c
> > > ./drivers/rtc/rtc-mv.c
> > > ./drivers/rtc/rtc-coh901331.c
> > > ./drivers/rtc/rtc-stk17ta8.c
> > > ./drivers/rtc/rtc-lpc32xx.c
> > > ./drivers/rtc/rtc-tps65910.c
> > > ./drivers/rtc/rtc-rc5t583.c
> > >
> > >
> > > Also, even more, some RTC drivers calls rtc_device_unregister() first,
> > > then calls free_irq() later.
> > >
> > > For example,
> > > ./drivers/rtc/rtc-vr41xx.c
> > > ./drivers/rtc/rtc-da9052.c
> > > ./drivers/rtc/rtc-isl1208.c
> > > ./drivers/rtc/rtc-88pm860x.c
> > > ./drivers/rtc/rtc-tps6586x.c
> > > ./drivers/rtc/rtc-mpc5121.c
> > > ./drivers/rtc/rtc-m48t59.c
> > >
> > >
> > > Please, don't argue revert without concrete reasons.
> > 
> > What more concrete reason do you need? I explained to you the exact
> > reason on the patches I noticed before and also on the 2 commits
> > referenced above: blind conversion to devm_* changes order of operation
> > which may be deadly with IRQs (but others, like clocks and regulators,
> > are important too).
> > 
> > The fact that crap slipped in the kernel before is not the valid reason
> > for adding more of the same crap.
> > 
> > Please *understand* APIs you are using before making changes.
> > 
> > >
> > > If these devm_request_threaded_irq() or devm_request_irq() make the 
> > > problem,
> > > devm_free_irq() will be added later.
> > 
> > And the point? If you use devm_request_irq() and then call
> > devm_free_irq() manually in all paths what you achieved is waste of
> > memory required for devm_* tracking.
> 
> CC'ed Al Viro, Tejun Heo
> 
> 
> So, is there any report that the devm_request_threaded_irq() makes
> the deadly problem related IRQ in such cases?
> 
> According to your comment, it seems that there is no reason to use
> devm_request_irq() or devm_request_threaded_irq().

devm_request_irq() or devm_request_threaded_irq() are OK if:

1. _ALL_ resources in the driver are controlled by devm_* so that order
of freeing is not disturbed, or

2. You can shut off interrupts in the chip so that interrupts will not
be generated even though IRQ handler is installed, or

3. Some combination of above.

Note that it is not only interrupts, other resources like clocks and
regulators and pins and other objects are important too, IRQ is just the
most dangerous.

> 
> Please, argue that it would be better to deprecate devm_request_irq()
> or devm_request_threaded_irq().
> 

No, they do have their use. But probably people doing the conversion
should be required to put $100 in escrow ;)

-- 
Dmitry
--
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/


Re: linux-next: build failure after merge of the final tree

2013-03-12 Thread Dmitry Torokhov
On Tue, Mar 12, 2013 at 10:57:19PM +1100, Stephen Rothwell wrote:
> On Tue, 12 Mar 2013 10:31:09 +0100 Daniel Hellstrom  
> wrote:
> >
> > APBPS2 driver is missing linux/slab.h required on at least PowerPC. See 
> > patch below. How do you want me to proceed?
> 
> That is up to Dmitry - either a fix up patch (with SoB and commit
> message) or a replacement patch.

I fixed up the commit and refreshed the branch.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 2/4] Input: cyapa - Firmware update via request firmware

2013-03-13 Thread Dmitry Torokhov
On Wed, Mar 13, 2013 at 04:50:49PM -0700, Benson Leung wrote:
> +
> +#define BYTE_PER_LINE  8
> +void cyapa_dump_data(struct cyapa *cyapa, size_t length, const u8 *data)
> +{
> +#ifdef DEBUG
> + struct device *dev = &cyapa->client->dev;
> + int i;
> + char buf[BYTE_PER_LINE * 3 + 1];
> + char *s = buf;
> +
> + for (i = 0; i < length; i++) {
> + s += sprintf(s, " %02x", data[i]);
> + if ((i + 1) == length || ((i + 1) % BYTE_PER_LINE) == 0) {
> + dev_dbg(dev, "%s\n", buf);
> + s = buf;
> + }
> + }
> +#endif
> +}
> +#undef BYTE_PER_LINE

We have dev_dbg("%*ph\n", BYTES_PER_LINE, data[...]); for this.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH v2 4/8] drivers: input: use module_platform_driver_probe()

2013-03-14 Thread Dmitry Torokhov
Hi Fabio,

On Thursday, March 14, 2013 06:09:34 PM Fabio Porcedda wrote:
> This patch converts the drivers to use the
> module_platform_driver_probe() macro which makes the code smaller and
> a bit simpler.

I already have patches from Sachin Kamat for this, I am waiting for -rc3
to sync up with mainline and pick up the macro before applying them.

Thanks.

-- 
Dmitry
--
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/


[git pull] Input updates for 3.6-rc4

2012-08-30 Thread Dmitry Torokhov
Hi Linus,

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
or
master.kernel.org:/pub/scm/linux/kernel/git/dtor/input.git for-linus

to receive updates for the input subsystem.

Changelog:
-

Dmitry Torokhov (1):
  Input: i8042 - add Gigabyte T1005 series netbooks to noloop table

Guenter Roeck (1):
  Input: edt-ft5x06 - fix build error when compiling wthout CONFIG_DEBUG_FS

Jason Gerecke (1):
  Input: wacom - add support for EMR on Cintiq 24HD touch

Michael Grzeschik (1):
  Input: imx_keypad - reset the hardware before enabling


Diffstat:


 drivers/input/keyboard/imx_keypad.c|  3 +++
 drivers/input/serio/i8042-x86ia64io.h  | 14 ++
 drivers/input/tablet/wacom_wac.c   |  6 +-
 drivers/input/touchscreen/edt-ft5x06.c |  2 +-
 4 files changed, 23 insertions(+), 2 deletions(-)

-- 
Dmitry



pgpqkqnmEnzbd.pgp
Description: PGP signature


Re: [PATCH] Input: Let the FT5x06 driver build without debugfs

2012-08-30 Thread Dmitry Torokhov
On Thu, Aug 30, 2012 at 03:26:21PM -0700, David Rientjes wrote:
> On Tue, 21 Aug 2012, Dmitry Torokhov wrote:
> 
> > > > > On 08/17/2012 02:15 AM, Eric W. Biederman wrote:
> > > > >> When testing to make certain my user namespace code works in
> > > > >> various configurations I tripped over the tf5x06.c not building
> > > > >> with debugfs disabled.
> > > > >
> > > > > Sorry for that.
> > > > >
> > > > > There already is a patch for this issue which I slightly prefer. You
> > > > > can find it in the mail from Guenther Roeck:
> > > > >
> > > > > http://www.mail-archive.com/linux-input@vger.kernel.org/msg00646.html
> > > > 
> > > > If you guys could get that merged into 3.6 I would appreciate it.
> > > > 
> > > Yes, that would be great. I don't recall seeing an e-mail from Dmitry 
> > > accepting
> > > it, though.
> > 
> > Applied, sorry for the delay.
> > 
> 
> This still affects Linus' tree and causes a build breakage without debugfs 
> configured.  Considering the driver went into 3.5-rc5, could you please 
> push this fix for 3.6

I Just sent a pull request to Linus.

> (and mark it for stable backport)?

I am pretty sure it was merged in 3.6 merge window, not 3.5, so no need
to mark for stable.

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH v2] drivers/tty: Folding Android's keyreset driver in sysRQ

2012-08-30 Thread Dmitry Torokhov
Hi Matthieu,

On Thu, Aug 30, 2012 at 04:30:54PM -0600, mathieu.poir...@linaro.org wrote:
> From: "Mathieu J. Poirier" 
> 
> This patch adds keyreset functionality to the sysrq driver. It
> allows certain button/key combinations to be used in order to
> trigger device resets.
> 
> The first time the key-combo is detected a work function that syncs
> the filesystems is scheduled and the kernel rebooted. If all the keys
> are released and then pressed again, it calls panic. Reboot on panic
> should be set for this to work.  A platform device that specify a
> reset key-combo should be added to the board file to trigger the
> feature.

Why do we need to involve a platform device and not use, for example, a module
parameter, that could be set up from userspace?

Also, why do we need reset_fn() and not simply invoke SysRq-B handler
that should call ctrl_alt_del() for us?

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

2012-08-30 Thread Dmitry Torokhov
On Thu, Aug 30, 2012 at 02:12:04PM +0100, Lee Jones wrote:
> > Sorry for the delay. Yes, this shoudl be fine, but since it is
> > essentially a revert of the original patch it should be pushed in as
> > such.
> 
> How's this?
> 

Excellent.

> Author: Lee Jones 
> Date:   Thu Aug 30 14:08:19 2012 +0100
> 
> Revert "input: ab8500-ponkey: Create AB8500 domain IRQ mapping"
> 
> This reverts commit ca3b3faf9bee4dc5df4f10eae2d1e48f7de0a8ad.
> 
> There was a plan to place ab8500_irq_get_virq() calls in each AB8500
> child device prior to requesting an IRQ, but as we're no longer using
> Device Tree to collect our IRQ numbers, it's actually better to allow
> the core to do this during device registration time. So the IRQ number
> we pull from its resource has already been converted to a virtual IRQ.
> 
> CC: Dmitry Torokhov 
> CC: linux-in...@vger.kernel.org
> Acked-by: Linus Walleij 

Acked-by: Dmitry Torokhov 

> Signed-off-by: Lee Jones 
> 
> diff --git a/drivers/input/misc/ab8500-ponkey.c 
> b/drivers/input/misc/ab8500-ponkey.c
> index f06231b..84ec691 100644
> --- a/drivers/input/misc/ab8500-ponkey.c
> +++ b/drivers/input/misc/ab8500-ponkey.c
> @@ -74,8 +74,8 @@ static int __devinit ab8500_ponkey_probe(struct 
> platform_device *pdev)
>  
> ponkey->idev = input;
> ponkey->ab8500 = ab8500;
> -   ponkey->irq_dbf = ab8500_irq_get_virq(ab8500, irq_dbf);
> -   ponkey->irq_dbr = ab8500_irq_get_virq(ab8500, irq_dbr);
> +   ponkey->irq_dbf = irq_dbf;
> +   ponkey->irq_dbr = irq_dbr;
>  
> input->name = "AB8500 POn(PowerOn) Key";
> input->dev.parent = &pdev->dev;

-- 
Dmitry
--
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/


Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

2012-08-30 Thread Dmitry Torokhov
On Thu, Aug 30, 2012 at 04:02:21PM -0700, Dmitry Torokhov wrote:
> On Thu, Aug 30, 2012 at 02:12:04PM +0100, Lee Jones wrote:
> > > Sorry for the delay. Yes, this shoudl be fine, but since it is
> > > essentially a revert of the original patch it should be pushed in as
> > > such.
> > 
> > How's this?
> > 
> 
> Excellent.

I assume you will be merging it with the rest of AB8500 patches, right?

-- 
Dmitry
--
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/


Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

2012-08-31 Thread Dmitry Torokhov
On Fri, Aug 31, 2012 at 08:31:33AM +0100, Lee Jones wrote:
> On Thu, Aug 30, 2012 at 04:03:23PM -0700, Dmitry Torokhov wrote:
> > On Thu, Aug 30, 2012 at 04:02:21PM -0700, Dmitry Torokhov wrote:
> > > On Thu, Aug 30, 2012 at 02:12:04PM +0100, Lee Jones wrote:
> > > > > Sorry for the delay. Yes, this shoudl be fine, but since it is
> > > > > essentially a revert of the original patch it should be pushed in as
> > > > > such.
> > > > 
> > > > How's this?
> > > > 
> > > 
> > > Excellent.
> > 
> > I assume you will be merging it with the rest of AB8500 patches, right?
> 
> Yes, if that's okay with you of course?

Sure, please go ahead,

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH v2] drivers/tty: Folding Android's keyreset driver in sysRQ

2012-08-31 Thread Dmitry Torokhov
On Fri, Aug 31, 2012 at 11:02:27PM +0100, Alan Cox wrote:
> > > Why do we need to involve a platform device and not use, for example, a 
> > > module
> > > parameter, that could be set up from userspace?
> > 
> > The platform device comes from the original design and was included to
> > minimise the amount of changes in code that make use of the current
> > keyreset driver.
> 
> The platform device is IMHO the right answer. In this class of devices
> the stuff is compiled in, the userspace is Android, there are no modules
> and there is no reason for it to be configurable.

It does not matter if it is built in or not, /sys/module/XXX/parameters
is still there, and while havig it in kernel is "easy" you could as
easily stuff needed data into a sysfs attribute during booting.

And we should be able to get this from DT even without the platform
device (this was the next step, wasn't it?).

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH v2] drivers/tty: Folding Android's keyreset driver in sysRQ

2012-08-31 Thread Dmitry Torokhov
On Fri, Aug 31, 2012 at 04:57:04PM -0600, Mathieu Poirier wrote:
> On 12-08-31 04:41 PM, Dmitry Torokhov wrote:
> > On Fri, Aug 31, 2012 at 11:02:27PM +0100, Alan Cox wrote:
> >>>> Why do we need to involve a platform device and not use, for example, a 
> >>>> module
> >>>> parameter, that could be set up from userspace?
> >>>
> >>> The platform device comes from the original design and was included to
> >>> minimise the amount of changes in code that make use of the current
> >>> keyreset driver.
> >>
> >> The platform device is IMHO the right answer. In this class of devices
> >> the stuff is compiled in, the userspace is Android, there are no modules
> >> and there is no reason for it to be configurable.
> > 
> > It does not matter if it is built in or not, /sys/module/XXX/parameters
> > is still there, and while havig it in kernel is "easy" you could as
> > easily stuff needed data into a sysfs attribute during booting.
> > 
> > And we should be able to get this from DT even without the platform
> > device (this was the next step, wasn't it?).
> 
> Correct - my hope was to get the main functionality accepted before
> adding DT support.  Do you think the lack of DT support is a blocker for
> acceptance ?  Please confirm.
> 

No, lack of DT is not a blocker, but I am unconvinced that we need
platform device.

Thanks,

-- 
Dmitry
--
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/


Re: [PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

2012-09-04 Thread Dmitry Torokhov
Hi Vojtech,

On Tue, Aug 14, 2012 at 12:11:54AM +0200, Vojtech Bocek wrote:
> Initial input event has not yet arrived in joydev_connect()
> where values are set, which means default values of input_absinfo
> are used for init event, not the actual values from joystick.

So what guarantees that joystick events will arrive in time, before
joydev_generate_startup_event() is called? It looks like your solution
is racy...

I wonder if we should not generate the startup event until we have seen
at least one EV_SYN, i.e. entire device state has been transmitted to
us.
> 
> Signed-off-by: Vojtech Bocek 
> ---
>  drivers/input/joydev.c |9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
> index 26043cc..11f24b4 100644
> --- a/drivers/input/joydev.c
> +++ b/drivers/input/joydev.c
> @@ -318,9 +318,14 @@ static int joydev_generate_startup_event(struct 
> joydev_client *client,
>   event->value = !!test_bit(joydev->keypam[event->number],
> input->key);
>   } else {
> + int evnum = client->startup - joydev->nkey;
> + int val = input_abs_get_val(input, 
> joydev->abspam[evnum]);
> +
> + joydev->abs[evnum] = joydev_correct(val, 
> &joydev->corr[evnum]);
> +
>   event->type = JS_EVENT_AXIS | JS_EVENT_INIT;
> - event->number = client->startup - joydev->nkey;
> - event->value = joydev->abs[event->number];
> + event->number = evnum;
> + event->value = joydev->abs[evnum];
>   }
>   client->startup++;
>   }
> -- 
> 1.7.10.4
> 

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH] input: gpio_keys_polled: fix dt pdata->nbuttons

2012-09-12 Thread Dmitry Torokhov
On Wed, Sep 12, 2012 at 06:27:23PM -0300, Alexandre Pereira da Silva wrote:
> pdata->nbuttons should be updated by the dt code.
> 
> Signed-off-by: Alexandre Pereira da Silva 

Applied, thank you Alexandre.
 
> ---
>  drivers/input/keyboard/gpio_keys_polled.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/keyboard/gpio_keys_polled.c 
> b/drivers/input/keyboard/gpio_keys_polled.c
> index 7908952..f2142de 100644
> --- a/drivers/input/keyboard/gpio_keys_polled.c
> +++ b/drivers/input/keyboard/gpio_keys_polled.c
> @@ -129,6 +129,7 @@ gpio_keys_polled_get_devtree_pdata(struct device *dev)
>   }
>  
>   pdata->buttons = (struct gpio_keys_button *)(pdata + 1);
> + pdata->nbuttons = nbuttons;
>  
>   pdata->rep = !!of_get_property(node, "autorepeat", NULL);
>   of_property_read_u32(node, "poll-interval", &pdata->poll_interval);
> -- 
> 1.7.10
> 

-- 
Dmitry
--
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/


Re: [PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

2012-09-12 Thread Dmitry Torokhov
On Wed, Sep 05, 2012 at 10:09:57PM +0200, Vojtěch Boček wrote:
> Hi,
> 
> 2012/9/5 Dmitry Torokhov 
> > So what guarantees that joystick events will arrive in time, before
> > joydev_generate_startup_event() is called? It looks like your solution
> > is racy...
> >
> > I wonder if we should not generate the startup event until we have seen
> > at least one EV_SYN, i.e. entire device state has been transmitted to
> > us.
> 
> I just tried to delay read() until first EV_SYN arrives as you suggested,
> but that was not the problem, at least not the main one. First joystick
> input events arrives immediately after it is plugged in and driver is loaded,
> but when that happens, joydev may not be (and most likely is not)
> opened yet, which means the events will never reach joydev because of
> "if (!handle->open)" check in input_pass_event.
> 
> I wonder how to handle this. Is "if(!handle->open)" valid check? I think so,
> my guess is that joydev is the only handler with internal buffer, and it
> is useless to update that buffer when the device is not opened.
> I think reloading abs values on joydev open should be okay.
> 
> I suppose that if we reload abs values on joydev open, waiting for first
> sync is not needed, yes?
> 
> So the patch could look like this:

This makes sense. Just to confirm - have you tried the patch and
verified it works for you?

Thanks.

-- 
Dmitry
--
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/


Re: [PATCH v3 01/20] Input: Break out MT data

2012-09-12 Thread Dmitry Torokhov
On Sat, Sep 01, 2012 at 09:46:56PM +0200, Henrik Rydberg wrote:
> Move all MT-related things to a separate place. This saves some
> bytes for non-mt input devices, and prepares for new MT features.
> 
> Signed-off-by: Henrik Rydberg 
...
> @@ -1287,10 +1284,8 @@ struct input_dev {
>  
>   int rep[REP_CNT];
>  
> - struct input_mt_slot *mt;
> - int mtsize;
> + struct input_mt *mt;
>   int slot;
> - int trkid;
>  
>   struct input_absinfo *absinfo;

Shouldn't 'slot' go into struct input_mt as well?

Thanks.

-- 
Dmitry
--
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/


  1   2   3   4   5   6   7   8   9   10   >