* Re: [RFC PATCH] staging: ak8975: add platform data. [not found] ` <643E69AA4436674C8F39DCC2C05F763816BD96CFB5@HQMAIL03.nvidia.com> @ 2011-02-21 16:10 ` Jonathan Cameron 2011-02-22 9:29 ` Jean Delvare 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2011-02-21 16:10 UTC (permalink / raw) To: Andrew Chew Cc: 'chinyeow.sim.xt@renesas.com', gregkh@suse.de, devel@driverdev.osuosl.org, linux-iio@vger.kernel.org, Jean Delvare On 02/15/11 20:49, Andrew Chew wrote: >> As some of the platform not support irq_to_gpio, we pass gpio port >> by platform data. > > Looks good to me. > > Signed-off-by: Andrew Chew <achew@nvidia.com> Firstly, please cc linux-iio for patches effecting iio drivers. Hmm.. It's a bit of a pain that i2c devices don't take platform resources or we could avoid doing this entirely. I have a vague recollection of this being suggested in the past, but never happening (might have been for spi rather than i2c though...) Jean, can you comment on this? Dmitry isn't going to want that header in the linux/input directory. What we've done for other drivers that need simple platform data like this is to use a pointer to an int rather than creating a specific structure for passing the data. It's clunky but politically a lot simpler than having a driver with parts in staging and parts not (which won't happen). There were quite a few cases of this that we had to clean up before the subsystem was acceptable for staging in the first place. For more complex cases, we have cases that really need platform data and have a chunk of of their local header in the tree that is commented to be moved to linux/iio/ when that exists and people are just cutting that out as necessary for their local development trees. We could add a directory under drivers/staging/iio for these to make this even clearer. This whole patch highlights a point I'd originally missed when reviewing this driver. The irq is never used as an actual interrupt and hence should never have been passed like that in the first place. Andrew, would you be happy with a patch that removed the miss use of the irq field entirely in favour of Tony's platform data route (with a simple int rather than the structure)? The interrupt passing would go back in if anyone ever added interrupt handling to this driver. Jonathan > >> Signed-off-by: Tony SIM <chinyeow.sim.xt@renesas.com> >> --- >> drivers/staging/iio/magnetometer/ak8975.c | 8 +++++++- >> include/linux/input/ak8975.h | 20 ++++++++++++++++++++ >> 2 files changed, 27 insertions(+), 1 deletions(-) >> create mode 100644 include/linux/input/ak8975.h >> >> diff --git a/drivers/staging/iio/magnetometer/ak8975.c >> b/drivers/staging/iio/magnetometer/ak8975.c >> index 420f206..80c0f41 100644 >> --- a/drivers/staging/iio/magnetometer/ak8975.c >> +++ b/drivers/staging/iio/magnetometer/ak8975.c >> @@ -29,6 +29,7 @@ >> #include <linux/delay.h> >> >> #include <linux/gpio.h> >> +#include <linux/input/ak8975.h> >> >> #include "../iio.h" >> #include "magnet.h" >> @@ -435,6 +436,7 @@ static int ak8975_probe(struct i2c_client *client, >> const struct i2c_device_id *id) >> { >> struct ak8975_data *data; >> + struct ak8975_platform_data *pdata; >> int err; >> >> /* Allocate our device context. */ >> @@ -452,7 +454,11 @@ static int ak8975_probe(struct >> i2c_client *client, >> >> /* Grab and set up the supplied GPIO. */ >> data->eoc_irq = client->irq; >> - data->eoc_gpio = irq_to_gpio(client->irq); >> + pdata = client->dev.platform_data; >> + if (pdata) >> + data->eoc_gpio = pdata->gpio; >> + else >> + data->eoc_gpio = irq_to_gpio(client->irq); >> >> if (!data->eoc_gpio) { >> dev_err(&client->dev, "failed, no valid GPIO\n"); >> diff --git a/include/linux/input/ak8975.h >> b/include/linux/input/ak8975.h >> new file mode 100644 >> index 0000000..25d41eb >> --- /dev/null >> +++ b/include/linux/input/ak8975.h >> @@ -0,0 +1,20 @@ >> +/* >> + * ak8975 platform support >> + * >> + * Copyright (C) 2010 Renesas Solutions Corp. >> + * >> + * Author: Tony SIM <chinyeow.sim.xt@renesas.com> >> + * >> + * This program is free software; you can redistribute it >> and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#ifndef _AK8975_H >> +#define _AK8975_H >> + >> +struct ak8975_platform_data { >> + int gpio; >> +}; >> + >> +#endif >> -- >> 1.7.2.3 >> >> > ----------------------------------------------------------------------------------- > This email message is for the sole use of the intended recipient(s) and may contain > confidential information. Any unauthorized review, use, disclosure or distribution > is prohibited. If you are not the intended recipient, please contact the sender by > reply email and destroy all copies of the original message. > ----------------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] staging: ak8975: add platform data. 2011-02-21 16:10 ` [RFC PATCH] staging: ak8975: add platform data Jonathan Cameron @ 2011-02-22 9:29 ` Jean Delvare 2011-02-22 12:35 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: Jean Delvare @ 2011-02-22 9:29 UTC (permalink / raw) To: Jonathan Cameron; +Cc: Andrew Chew, chinyeow.sim.xt, gregkh, devel, linux-iio Hi Jonathan, On Mon, 21 Feb 2011 16:10:14 +0000, Jonathan Cameron wrote: > On 02/15/11 20:49, Andrew Chew wrote: > >> As some of the platform not support irq_to_gpio, we pass gpio port > >> by platform data. > > > > Looks good to me. > > > > Signed-off-by: Andrew Chew <achew@nvidia.com> > Firstly, please cc linux-iio for patches effecting iio drivers. > > Hmm.. It's a bit of a pain that i2c devices don't take platform resources > or we could avoid doing this entirely. I have a vague recollection of this > being suggested in the past, but never happening (might have been for spi > rather than i2c though...) Jean, can you comment on this? I fear I don't have the slightest idea of what you're talking about. "Taking platform resources"? > Dmitry isn't going to want that header in the linux/input directory. And he would be right. Header files containing platform data structures for i2c device drivers go to include/linux/i2c. -- Jean Delvare ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] staging: ak8975: add platform data. 2011-02-22 9:29 ` Jean Delvare @ 2011-02-22 12:35 ` Jonathan Cameron 2011-02-22 21:55 ` Mark Brown 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2011-02-22 12:35 UTC (permalink / raw) To: Jean Delvare; +Cc: Andrew Chew, chinyeow.sim.xt, gregkh, devel, linux-iio On 02/22/11 09:29, Jean Delvare wrote: > Hi Jonathan, > > On Mon, 21 Feb 2011 16:10:14 +0000, Jonathan Cameron wrote: >> On 02/15/11 20:49, Andrew Chew wrote: >>>> As some of the platform not support irq_to_gpio, we pass gpio port >>>> by platform data. >>> >>> Looks good to me. >>> >>> Signed-off-by: Andrew Chew <achew@nvidia.com> >> Firstly, please cc linux-iio for patches effecting iio drivers. >> >> Hmm.. It's a bit of a pain that i2c devices don't take platform resources >> or we could avoid doing this entirely. I have a vague recollection of this >> being suggested in the past, but never happening (might have been for spi >> rather than i2c though...) Jean, can you comment on this? > > I fear I don't have the slightest idea of what you're talking about. > "Taking platform resources"? Sorry, I should have written a proper description of what I meant. Add to i2c_board_info the following struct resource *resource; which is defined in ioport.h I thought this actually allowed for gpio's but right now can't figure out how to handle them. What it definitely does handle cleanly is devices with multiple irq's. You basically get to query for resources by name so can pass lots of 'platform data' type stuff without having to define a specific structure for passing it in. > >> Dmitry isn't going to want that header in the linux/input directory. > > And he would be right. Header files containing platform data structures > for i2c device drivers go to include/linux/i2c. Cool. I'd kind of assumed they moved out when i2c/chips went away, but never actually checked! > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] staging: ak8975: add platform data. 2011-02-22 12:35 ` Jonathan Cameron @ 2011-02-22 21:55 ` Mark Brown 2011-02-23 8:19 ` Jean Delvare 0 siblings, 1 reply; 7+ messages in thread From: Mark Brown @ 2011-02-22 21:55 UTC (permalink / raw) To: Jonathan Cameron Cc: Jean Delvare, Andrew Chew, chinyeow.sim.xt, gregkh, devel, linux-iio On Tue, Feb 22, 2011 at 12:35:13PM +0000, Jonathan Cameron wrote: > I thought this actually allowed for gpio's but right now can't figure out how > to handle them. What it definitely does handle cleanly is devices with multiple > irq's. > You basically get to query for resources by name so can pass lots of 'platform data' > type stuff without having to define a specific structure for passing it in. There's no standard support for GPIOs as resources - this is always done using platform data these days. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] staging: ak8975: add platform data. 2011-02-22 21:55 ` Mark Brown @ 2011-02-23 8:19 ` Jean Delvare 2011-02-23 15:41 ` Bill Gatliff 2011-02-23 20:10 ` Mark Brown 0 siblings, 2 replies; 7+ messages in thread From: Jean Delvare @ 2011-02-23 8:19 UTC (permalink / raw) To: Mark Brown Cc: Jonathan Cameron, Andrew Chew, chinyeow.sim.xt, gregkh, devel, linux-iio On Tue, 22 Feb 2011 21:55:55 +0000, Mark Brown wrote: > On Tue, Feb 22, 2011 at 12:35:13PM +0000, Jonathan Cameron wrote: > > > I thought this actually allowed for gpio's but right now can't figure out how > > to handle them. What it definitely does handle cleanly is devices with multiple > > irq's. > > > You basically get to query for resources by name so can pass lots of 'platform data' > > type stuff without having to define a specific structure for passing it in. > > There's no standard support for GPIOs as resources - this is always done > using platform data these days. Would it make sense to add this? I have no opinion on the question, not being a big user of GPIO, just wondering. -- Jean Delvare ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] staging: ak8975: add platform data. 2011-02-23 8:19 ` Jean Delvare @ 2011-02-23 15:41 ` Bill Gatliff 2011-02-23 20:10 ` Mark Brown 1 sibling, 0 replies; 7+ messages in thread From: Bill Gatliff @ 2011-02-23 15:41 UTC (permalink / raw) To: Jean Delvare Cc: Mark Brown, Jonathan Cameron, Andrew Chew, chinyeow.sim.xt, gregkh, devel, linux-iio Guys: On Wed, Feb 23, 2011 at 2:19 AM, Jean Delvare <khali@linux-fr.org> wrote: > On Tue, 22 Feb 2011 21:55:55 +0000, Mark Brown wrote: >> There's no standard support for GPIOs as resources - this is always done >> using platform data these days. > > Would it make sense to add this? I have no opinion on the question, not > being a big user of GPIO, just wondering. I for one would welcome an IORESOURCE_GPIO. b.g. -- Bill Gatliff bgat@billgatliff.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] staging: ak8975: add platform data. 2011-02-23 8:19 ` Jean Delvare 2011-02-23 15:41 ` Bill Gatliff @ 2011-02-23 20:10 ` Mark Brown 1 sibling, 0 replies; 7+ messages in thread From: Mark Brown @ 2011-02-23 20:10 UTC (permalink / raw) To: Jean Delvare Cc: Jonathan Cameron, Andrew Chew, chinyeow.sim.xt, gregkh, devel, linux-iio On Wed, Feb 23, 2011 at 09:19:39AM +0100, Jean Delvare wrote: > On Tue, 22 Feb 2011 21:55:55 +0000, Mark Brown wrote: > > There's no standard support for GPIOs as resources - this is always done > > using platform data these days. > Would it make sense to add this? I have no opinion on the question, not > being a big user of GPIO, just wondering. Possibly, yes. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-02-23 20:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1297764627-21077-1-git-send-email-chinyeow.sim.xt@renesas.com>
[not found] ` <643E69AA4436674C8F39DCC2C05F763816BD96CFB5@HQMAIL03.nvidia.com>
2011-02-21 16:10 ` [RFC PATCH] staging: ak8975: add platform data Jonathan Cameron
2011-02-22 9:29 ` Jean Delvare
2011-02-22 12:35 ` Jonathan Cameron
2011-02-22 21:55 ` Mark Brown
2011-02-23 8:19 ` Jean Delvare
2011-02-23 15:41 ` Bill Gatliff
2011-02-23 20:10 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox