2010-06-17 12:47:48

by Dmitry Baryshkov

Subject: [PATCH] pcf857x: support working w/o platform data

Provide sane defaults for pcf857x, so the driver can be used w/o
providing platform data (and thus can be simply bound via OF tree).

Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
---
drivers/gpio/pcf857x.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/pcf857x.c b/drivers/gpio/pcf857x.c
index 29f19ce..879b473 100644
--- a/drivers/gpio/pcf857x.c
+++ b/drivers/gpio/pcf857x.c
@@ -190,7 +190,6 @@ static int pcf857x_probe(struct i2c_client *client,
pdata = client->dev.platform_data;
if (!pdata) {
dev_dbg(&client->dev, "no platform data\n");
- return -EINVAL;
}

/* Allocate, initialize, and register this gpio_chip. */
@@ -200,7 +199,7 @@ static int pcf857x_probe(struct i2c_client *client,

mutex_init(&gpio->lock);

- gpio->chip.base = pdata->gpio_base;
+ gpio->chip.base = pdata ? pdata->gpio_base : -1;
gpio->chip.can_sleep = 1;
gpio->chip.dev = &client->dev;
gpio->chip.owner = THIS_MODULE;
@@ -278,7 +277,7 @@ static int pcf857x_probe(struct i2c_client *client,
* to zero, our software copy of the "latch" then matches the chip's
* all-ones reset state. Otherwise it flags pins to be driven low.
*/
- gpio->out = ~pdata->n_latch;
+ gpio->out = pdata ? ~pdata->n_latch : ~0;

status = gpiochip_add(&gpio->chip);
if (status < 0)
@@ -299,7 +298,7 @@ static int pcf857x_probe(struct i2c_client *client,
/* Let platform code set up the GPIOs and their users.
* Now is the first time anyone could use them.
*/
- if (pdata->setup) {
+ if (pdata && pdata->setup) {
status = pdata->setup(client,
gpio->chip.base, gpio->chip.ngpio,
pdata->context);
@@ -322,7 +321,7 @@ static int pcf857x_remove(struct i2c_client *client)
struct pcf857x *gpio = i2c_get_clientdata(client);
int status = 0;

- if (pdata->teardown) {
+ if (pdata && pdata->teardown) {
status = pdata->teardown(client,
gpio->chip.base, gpio->chip.ngpio,
pdata->context);
--
1.7.1

2010-06-25 10:17:14

by David Brownell

Subject: Re: [PATCH] pcf857x: support working w/o platform data

--- On Thu, 6/17/10, Dmitry Eremin-Solenikov <[email protected]> wrote:

> Provide sane defaults for pcf857x, so
> the driver can be used w/o
> providing platform data (and thus can be simply bound via OF tree).


Maybe we can get an ack from some OF folk
who are using it in that way?



> Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
> ---
> drivers/gpio/pcf857x.c |? ? 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpio/pcf857x.c
> b/drivers/gpio/pcf857x.c
> index 29f19ce..879b473 100644
> --- a/drivers/gpio/pcf857x.c
> +++ b/drivers/gpio/pcf857x.c
> @@ -190,7 +190,6 @@ static int pcf857x_probe(struct
> i2c_client *client,
> ??? pdata = client->dev.platform_data;
> ??? if (!pdata) {
> ??? ???
> dev_dbg(&client->dev, "no platform data\n");
> -??? ??? return -EINVAL;
> ??? }
>
> ??? /* Allocate, initialize, and register
> this gpio_chip. */
> @@ -200,7 +199,7 @@ static int pcf857x_probe(struct
> i2c_client *client,
>
> ??? mutex_init(&gpio->lock);
>
> -??? gpio->chip.base =
> pdata->gpio_base;
> +??? gpio->chip.base = pdata ?
> pdata->gpio_base : -1;
> ??? gpio->chip.can_sleep = 1;
> ??? gpio->chip.dev =
> &client->dev;
> ??? gpio->chip.owner = THIS_MODULE;
> @@ -278,7 +277,7 @@ static int pcf857x_probe(struct
> i2c_client *client,
> ?????* to zero, our software copy
> of the "latch" then matches the chip's
> ?????* all-ones reset
> state.? Otherwise it flags pins to be driven low.
> ?????*/
> -??? gpio->out = ~pdata->n_latch;
> +??? gpio->out = pdata ?
> ~pdata->n_latch : ~0;
>
> ??? status =
> gpiochip_add(&gpio->chip);
> ??? if (status < 0)
> @@ -299,7 +298,7 @@ static int pcf857x_probe(struct
> i2c_client *client,
> ??? /* Let platform code set up the GPIOs
> and their users.
> ?????* Now is the first time
> anyone could use them.
> ?????*/
> -??? if (pdata->setup) {
> +??? if (pdata && pdata->setup)
> {
> ??? ??? status =
> pdata->setup(client,
> ??? ??? ???
> ??? gpio->chip.base, gpio->chip.ngpio,
> ??? ??? ???
> ??? pdata->context);
> @@ -322,7 +321,7 @@ static int pcf857x_remove(struct
> i2c_client *client)
> ??? struct pcf857x???
> ??? ??? *gpio =
> i2c_get_clientdata(client);
> ??? int???
> ??? ??? ???
> status = 0;
>
> -??? if (pdata->teardown) {
> +??? if (pdata &&
> pdata->teardown) {
> ??? ??? status =
> pdata->teardown(client,
> ??? ??? ???
> ??? gpio->chip.base, gpio->chip.ngpio,
> ??? ??? ???
> ??? pdata->context);
> --
> 1.7.1
>
>

2010-06-27 06:38:46

by Dmitry Baryshkov

Subject: Re: [PATCH] pcf857x: support working w/o platform data

Hello,

On 6/25/10, David Brownell <[email protected]> wrote:
>
> --- On Thu, 6/17/10, Dmitry Eremin-Solenikov <[email protected]> wrote:
>
>> Provide sane defaults for pcf857x, so
>> the driver can be used w/o
>> providing platform data (and thus can be simply bound via OF tree).
>
>
> Maybe we can get an ack from some OF folk
> who are using it in that way?

Please see arch/powerpc/boot//dts/mpc8349emitx.dts (the platform I'm using it).

>> Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
>> ---
>> drivers/gpio/pcf857x.c | 9 ++++-----
>> 1 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpio/pcf857x.c
>> b/drivers/gpio/pcf857x.c
>> index 29f19ce..879b473 100644
>> --- a/drivers/gpio/pcf857x.c
>> +++ b/drivers/gpio/pcf857x.c
>> @@ -190,7 +190,6 @@ static int pcf857x_probe(struct
>> i2c_client *client,
>> pdata = client->dev.platform_data;
>> if (!pdata) {
>>
>> dev_dbg(&client->dev, "no platform data\n");
>> - return -EINVAL;
>> }
>>
>> /* Allocate, initialize, and register
>> this gpio_chip. */
>> @@ -200,7 +199,7 @@ static int pcf857x_probe(struct
>> i2c_client *client,
>>
>> mutex_init(&gpio->lock);
>>
>> - gpio->chip.base =
>> pdata->gpio_base;
>> + gpio->chip.base = pdata ?
>> pdata->gpio_base : -1;
>> gpio->chip.can_sleep = 1;
>> gpio->chip.dev =
>> &client->dev;
>> gpio->chip.owner = THIS_MODULE;
>> @@ -278,7 +277,7 @@ static int pcf857x_probe(struct
>> i2c_client *client,
>> * to zero, our software copy
>> of the "latch" then matches the chip's
>> * all-ones reset
>> state. Otherwise it flags pins to be driven low.
>> */
>> - gpio->out = ~pdata->n_latch;
>> + gpio->out = pdata ?
>> ~pdata->n_latch : ~0;
>>
>> status =
>> gpiochip_add(&gpio->chip);
>> if (status < 0)
>> @@ -299,7 +298,7 @@ static int pcf857x_probe(struct
>> i2c_client *client,
>> /* Let platform code set up the GPIOs
>> and their users.
>> * Now is the first time
>> anyone could use them.
>> */
>> - if (pdata->setup) {
>> + if (pdata && pdata->setup)
>> {
>> status =
>> pdata->setup(client,
>>
>> gpio->chip.base, gpio->chip.ngpio,
>>
>> pdata->context);
>> @@ -322,7 +321,7 @@ static int pcf857x_remove(struct
>> i2c_client *client)
>> struct pcf857x
>> *gpio =
>> i2c_get_clientdata(client);
>> int
>>
>> status = 0;
>>
>> - if (pdata->teardown) {
>> + if (pdata &&
>> pdata->teardown) {
>> status =
>> pdata->teardown(client,
>>
>> gpio->chip.base, gpio->chip.ngpio,
>>
>> pdata->context);
>> --
>> 1.7.1
>>
>>
>
>


--
With best wishes
Dmitry

2010-06-27 07:06:10

by Grant Likely

Subject: Re: [PATCH] pcf857x: support working w/o platform data

On Fri, Jun 25, 2010 at 4:17 AM, David Brownell <[email protected]> wrote:
>
>
> --- On Thu, 6/17/10, Dmitry Eremin-Solenikov <[email protected]> wrote:
>
>> Provide sane defaults for pcf857x, so
>> the driver can be used w/o
>> providing platform data (and thus can be simply bound via OF tree).
>
>
> Maybe we can get an ack from some OF folk
> who are using it in that way?

I'm not actually using this chip, but the change looks sane to me.

g.