[PATCH 00/21] eeprom: at24: driver refactoring

[PATCH 00/21] eeprom: at24: driver refactoring

[Bartosz Golaszewski]

This series contains what I hope to be a non-controversial refactoring
of the at24 eeprom driver.

Most changes revolve around at24_probe() which became quite complicated
and hard to read.

The only functional changes are: disabling the internal locking
mechanisms of regmap (since we already take care of that in the driver)
and removing an if checking if byte_len is a power of 2 (as we do
support models for which it's not true).

All other patches affect readability and code structure.

Tested with a couple models and different both for device tree and
platform data modes.

Bartosz Golaszewski (21):
  eeprom: at24: disable regmap locking
  eeprom: at24: remove nvmem_config from at24_data
  eeprom: at24: arrange local variables
  eeprom: at24: use SPDX identifier instead of GPL boiler-plate
  eeprom: at24: remove code separators
  eeprom: at24: drop redundant variable in at24_read()
  eeprom: at24: drop redundant variable in at24_write()
  eeprom: at24: make struct initialization uniform in at24_probe()
  eeprom: at24: don't check if byte_len is a power of 2
  eeprom: at24: rename at24_get_pdata()
  eeprom: at24: rename chip to pdata in at24_probe()
  eeprom: at24: use a helper variable for dev
  eeprom: at24: readability tweak in at24_probe()
  eeprom: at24: provide and use at24_base_client_dev()
  eeprom: at24: switch to using probe_new() from the i2c framework
  eeprom: at24: move platform data processing into a separate routine
  eeprom: at24: remove at24_platform_data from at24_data
  eeprom: at24: refactor at24_probe()
  eeprom: at24: tweak newlines
  eeprom: at24: fix a line break
  eeprom: at24: simplify the i2c functionality checking

 drivers/misc/eeprom/at24.c | 293 ++++++++++++++++++++++++---------------------
 1 file changed, 156 insertions(+), 137 deletions(-)

-- 
2.16.1

[PATCH 01/21] eeprom: at24: disable regmap locking

[Bartosz Golaszewski]

We use our own mutex for locking. Disable the regmap-specific locking.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 01f9c4921c50..73af6c5a2d73 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -577,6 +577,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 	regmap_config.val_bits = 8;
 	regmap_config.reg_bits = (chip.flags & AT24_FLAG_ADDR16) ? 16 : 8;
+	regmap_config.disable_locking = true;
 
 	at24 = devm_kzalloc(&client->dev, sizeof(struct at24_data) +
 		num_addresses * sizeof(struct at24_client), GFP_KERNEL);
-- 
2.16.1

[PATCH 02/21] eeprom: at24: remove nvmem_config from at24_data

[Bartosz Golaszewski]

This structure only needs to exist during the call to nvmem_register().

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 73af6c5a2d73..2600657c7034 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -75,7 +75,6 @@ struct at24_data {
 	unsigned int num_addresses;
 	unsigned int offset_adj;
 
-	struct nvmem_config nvmem_config;
 	struct nvmem_device *nvmem;
 
 	struct gpio_desc *wp_gpio;
@@ -523,6 +522,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	int err;
 	unsigned int i, num_addresses;
 	struct regmap_config regmap_config = { };
+	struct nvmem_config nvmem_config = { };
 	u8 test_byte;
 
 	if (client->dev.platform_data) {
@@ -650,21 +650,21 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		goto err_clients;
 	}
 
-	at24->nvmem_config.name = dev_name(&client->dev);
-	at24->nvmem_config.dev = &client->dev;
-	at24->nvmem_config.read_only = !writable;
-	at24->nvmem_config.root_only = true;
-	at24->nvmem_config.owner = THIS_MODULE;
-	at24->nvmem_config.compat = true;
-	at24->nvmem_config.base_dev = &client->dev;
-	at24->nvmem_config.reg_read = at24_read;
-	at24->nvmem_config.reg_write = at24_write;
-	at24->nvmem_config.priv = at24;
-	at24->nvmem_config.stride = 1;
-	at24->nvmem_config.word_size = 1;
-	at24->nvmem_config.size = chip.byte_len;
-
-	at24->nvmem = nvmem_register(&at24->nvmem_config);
+	nvmem_config.name = dev_name(&client->dev);
+	nvmem_config.dev = &client->dev;
+	nvmem_config.read_only = !writable;
+	nvmem_config.root_only = true;
+	nvmem_config.owner = THIS_MODULE;
+	nvmem_config.compat = true;
+	nvmem_config.base_dev = &client->dev;
+	nvmem_config.reg_read = at24_read;
+	nvmem_config.reg_write = at24_write;
+	nvmem_config.priv = at24;
+	nvmem_config.stride = 1;
+	nvmem_config.word_size = 1;
+	nvmem_config.size = chip.byte_len;
+
+	at24->nvmem = nvmem_register(&nvmem_config);
 
 	if (IS_ERR(at24->nvmem)) {
 		err = PTR_ERR(at24->nvmem);
-- 
2.16.1

[PATCH 03/21] eeprom: at24: arrange local variables

[Bartosz Golaszewski]

Arrange declarations of local variables by line length as visually
it's easier to read.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 2600657c7034..2a4154eeb1bd 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -370,11 +370,14 @@ static ssize_t at24_regmap_write(struct at24_data *at24, const char *buf,
 
 static int at24_read(void *priv, unsigned int off, void *val, size_t count)
 {
-	struct at24_data *at24 = priv;
-	struct device *dev = &at24->client[0].client->dev;
+	struct at24_data *at24;
+	struct device *dev;
 	char *buf = val;
 	int ret;
 
+	at24 = priv;
+	dev = &at24->client[0].client->dev;
+
 	if (unlikely(!count))
 		return count;
 
@@ -416,11 +419,14 @@ static int at24_read(void *priv, unsigned int off, void *val, size_t count)
 
 static int at24_write(void *priv, unsigned int off, void *val, size_t count)
 {
-	struct at24_data *at24 = priv;
-	struct device *dev = &at24->client[0].client->dev;
+	struct at24_data *at24;
+	struct device *dev;
 	char *buf = val;
 	int ret;
 
+	at24 = priv;
+	dev = &at24->client[0].client->dev;
+
 	if (unlikely(!count))
 		return -EINVAL;
 
@@ -515,15 +521,15 @@ static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len)
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
-	struct at24_platform_data chip = { 0 };
-	const struct at24_chip_data *cd = NULL;
-	bool writable;
-	struct at24_data *at24;
-	int err;
-	unsigned int i, num_addresses;
 	struct regmap_config regmap_config = { };
 	struct nvmem_config nvmem_config = { };
+	const struct at24_chip_data *cd = NULL;
+	struct at24_platform_data chip = { 0 };
+	unsigned int i, num_addresses;
+	struct at24_data *at24;
+	bool writable;
 	u8 test_byte;
+	int err;
 
 	if (client->dev.platform_data) {
 		chip = *(struct at24_platform_data *)client->dev.platform_data;
-- 
2.16.1

Re: [PATCH 03/21] eeprom: at24: arrange local variables

[Greg Kroah-Hartman]

On Mon, Mar 19, 2018 at 10:17:03AM +0100, Bartosz Golaszewski wrote:
> Arrange declarations of local variables by line length as visually
> it's easier to read.

That's the craziest thing I've seen in a while...

ok, if it makes it easier for you :)

greg k-h

Re: [PATCH 03/21] eeprom: at24: arrange local variables

[Bartosz Golaszewski]

2018-03-23 16:21 GMT+01:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> On Mon, Mar 19, 2018 at 10:17:03AM +0100, Bartosz Golaszewski wrote:
>> Arrange declarations of local variables by line length as visually
>> it's easier to read.
>
> That's the craziest thing I've seen in a while...
>
> ok, if it makes it easier for you :)
>
> greg k-h

Hi Greg,

this isn't something unusual, see for example:

bef6b1b7a6ff ("nfp: reorder variables in nfp_net_tx()")
5318c53d5b4b ("crypto: s5p-sss - Use consistent indentation for
variables and members")

Thanks,
Bart

Re: [PATCH 03/21] eeprom: at24: arrange local variables

[Greg Kroah-Hartman]

On Fri, Mar 23, 2018 at 05:06:30PM +0100, Bartosz Golaszewski wrote:
> 2018-03-23 16:21 GMT+01:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> > On Mon, Mar 19, 2018 at 10:17:03AM +0100, Bartosz Golaszewski wrote:
> >> Arrange declarations of local variables by line length as visually
> >> it's easier to read.
> >
> > That's the craziest thing I've seen in a while...
> >
> > ok, if it makes it easier for you :)
> >
> > greg k-h
> 
> Hi Greg,
> 
> this isn't something unusual, see for example:
> 
> bef6b1b7a6ff ("nfp: reorder variables in nfp_net_tx()")
> 5318c53d5b4b ("crypto: s5p-sss - Use consistent indentation for
> variables and members")

I still say it is crazy :)

[PATCH 04/21] eeprom: at24: use SPDX identifier instead of GPL boiler-plate

[Bartosz Golaszewski]

Replace the GPL header with an SPDX identifier for GPL-2.0.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/misc/eeprom/at24.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 2a4154eeb1bd..0e06201d33c9 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -1,14 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  * at24.c - handle most I2C EEPROMs
  *
  * Copyright (C) 2005-2007 David Brownell
  * Copyright (C) 2008 Wolfram Sang, Pengutronix
- *
- * 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.
  */
+
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/module.h>
-- 
2.16.1

Re: [PATCH 04/21] eeprom: at24: use SPDX identifier instead of GPL boiler-plate

[Peter Rosin]

On 2018-03-19 10:17, Bartosz Golaszewski wrote:
> Replace the GPL header with an SPDX identifier for GPL-2.0.

Great care should be exercised when dealing with copyrights of others.

> 
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> ---
>  drivers/misc/eeprom/at24.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 2a4154eeb1bd..0e06201d33c9 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -1,14 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

This should be 2.0+

Also, use a // style comment for the SPDX line in C files.

Cheers,
Peter

>  /*
>   * at24.c - handle most I2C EEPROMs
>   *
>   * Copyright (C) 2005-2007 David Brownell
>   * Copyright (C) 2008 Wolfram Sang, Pengutronix
> - *
> - * 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.
>   */
> +
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/module.h>
> 

Re: [PATCH 04/21] eeprom: at24: use SPDX identifier instead of GPL boiler-plate

[Bartosz Golaszewski]

2018-03-19 12:03 GMT+01:00 Peter Rosin <peda@axentia.se>:
> On 2018-03-19 10:17, Bartosz Golaszewski wrote:
>> Replace the GPL header with an SPDX identifier for GPL-2.0.
>
> Great care should be exercised when dealing with copyrights of others.
>
>>
>> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
>> ---
>>  drivers/misc/eeprom/at24.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
>> index 2a4154eeb1bd..0e06201d33c9 100644
>> --- a/drivers/misc/eeprom/at24.c
>> +++ b/drivers/misc/eeprom/at24.c
>> @@ -1,14 +1,11 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>
> This should be 2.0+

Indeed, thanks.

>
> Also, use a // style comment for the SPDX line in C files.
>
> Cheers,
> Peter

I'm seeing both /* */ and // style comments used for SPDX headers - is
there any reason not to use /* */ here?

Bart

Re: [PATCH 04/21] eeprom: at24: use SPDX identifier instead of GPL boiler-plate

[Peter Rosin]

On 2018-03-19 13:12, Bartosz Golaszewski wrote:
> 2018-03-19 12:03 GMT+01:00 Peter Rosin <peda@axentia.se>:
>> Also, use a // style comment for the SPDX line in C files.
> 
> I'm seeing both /* */ and // style comments used for SPDX headers - is
> there any reason not to use /* */ here?

Documentation/process/license-rules.rst states:

2. Style:

   The SPDX license identifier is added in form of a comment.  The comment
   style depends on the file type::

      C source: // SPDX-License-Identifier: <SPDX License Expression>
      C header: /* SPDX-License-Identifier: <SPDX License Expression> */
      ASM:      /* SPDX-License-Identifier: <SPDX License Expression> */
      scripts:  # SPDX-License-Identifier: <SPDX License Expression>
      .rst:     .. SPDX-License-Identifier: <SPDX License Expression>
      .dts{i}:  // SPDX-License-Identifier: <SPDX License Expression>

Read more in that file for reasons. If there are none, I personally
think the reason is that "Linus said so". Or something like that?

Cheers,
Peter

Re: [PATCH 04/21] eeprom: at24: use SPDX identifier instead of GPL boiler-plate

[Bartosz Golaszewski]

2018-03-19 13:51 GMT+01:00 Peter Rosin <peda@axentia.se>:
> On 2018-03-19 13:12, Bartosz Golaszewski wrote:
>> 2018-03-19 12:03 GMT+01:00 Peter Rosin <peda@axentia.se>:
>>> Also, use a // style comment for the SPDX line in C files.
>>
>> I'm seeing both /* */ and // style comments used for SPDX headers - is
>> there any reason not to use /* */ here?
>
> Documentation/process/license-rules.rst states:
>
> 2. Style:
>
>    The SPDX license identifier is added in form of a comment.  The comment
>    style depends on the file type::
>
>       C source: // SPDX-License-Identifier: <SPDX License Expression>
>       C header: /* SPDX-License-Identifier: <SPDX License Expression> */
>       ASM:      /* SPDX-License-Identifier: <SPDX License Expression> */
>       scripts:  # SPDX-License-Identifier: <SPDX License Expression>
>       .rst:     .. SPDX-License-Identifier: <SPDX License Expression>
>       .dts{i}:  // SPDX-License-Identifier: <SPDX License Expression>
>
> Read more in that file for reasons. If there are none, I personally
> think the reason is that "Linus said so". Or something like that?
>
> Cheers,
> Peter

Makes sense, thanks.

I'm thinking about dropping this file from this series and submitting
it separately for Greg to Ack.

Unless he sees our exchange and acks it here. :)

Thanks,
Bart

Re: [PATCH 04/21] eeprom: at24: use SPDX identifier instead of GPL boiler-plate

[Greg Kroah-Hartman]

On Mon, Mar 19, 2018 at 01:56:28PM +0100, Bartosz Golaszewski wrote:
> 2018-03-19 13:51 GMT+01:00 Peter Rosin <peda@axentia.se>:
> > On 2018-03-19 13:12, Bartosz Golaszewski wrote:
> >> 2018-03-19 12:03 GMT+01:00 Peter Rosin <peda@axentia.se>:
> >>> Also, use a // style comment for the SPDX line in C files.
> >>
> >> I'm seeing both /* */ and // style comments used for SPDX headers - is
> >> there any reason not to use /* */ here?
> >
> > Documentation/process/license-rules.rst states:
> >
> > 2. Style:
> >
> >    The SPDX license identifier is added in form of a comment.  The comment
> >    style depends on the file type::
> >
> >       C source: // SPDX-License-Identifier: <SPDX License Expression>
> >       C header: /* SPDX-License-Identifier: <SPDX License Expression> */
> >       ASM:      /* SPDX-License-Identifier: <SPDX License Expression> */
> >       scripts:  # SPDX-License-Identifier: <SPDX License Expression>
> >       .rst:     .. SPDX-License-Identifier: <SPDX License Expression>
> >       .dts{i}:  // SPDX-License-Identifier: <SPDX License Expression>
> >
> > Read more in that file for reasons. If there are none, I personally
> > think the reason is that "Linus said so". Or something like that?
> >
> > Cheers,
> > Peter
> 
> Makes sense, thanks.
> 
> I'm thinking about dropping this file from this series and submitting
> it separately for Greg to Ack.
> 
> Unless he sees our exchange and acks it here. :)

I can't ack a patch that is incorrect :(

Please fix it up and resend...

thanks,

greg k-h

Re: [PATCH 04/21] eeprom: at24: use SPDX identifier instead of GPL boiler-plate

[Bartosz Golaszewski]

2018-03-19 16:38 GMT+01:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> On Mon, Mar 19, 2018 at 01:56:28PM +0100, Bartosz Golaszewski wrote:
>> 2018-03-19 13:51 GMT+01:00 Peter Rosin <peda@axentia.se>:
>> > On 2018-03-19 13:12, Bartosz Golaszewski wrote:
>> >> 2018-03-19 12:03 GMT+01:00 Peter Rosin <peda@axentia.se>:
>> >>> Also, use a // style comment for the SPDX line in C files.
>> >>
>> >> I'm seeing both /* */ and // style comments used for SPDX headers - is
>> >> there any reason not to use /* */ here?
>> >
>> > Documentation/process/license-rules.rst states:
>> >
>> > 2. Style:
>> >
>> >    The SPDX license identifier is added in form of a comment.  The comment
>> >    style depends on the file type::
>> >
>> >       C source: // SPDX-License-Identifier: <SPDX License Expression>
>> >       C header: /* SPDX-License-Identifier: <SPDX License Expression> */
>> >       ASM:      /* SPDX-License-Identifier: <SPDX License Expression> */
>> >       scripts:  # SPDX-License-Identifier: <SPDX License Expression>
>> >       .rst:     .. SPDX-License-Identifier: <SPDX License Expression>
>> >       .dts{i}:  // SPDX-License-Identifier: <SPDX License Expression>
>> >
>> > Read more in that file for reasons. If there are none, I personally
>> > think the reason is that "Linus said so". Or something like that?
>> >
>> > Cheers,
>> > Peter
>>
>> Makes sense, thanks.
>>
>> I'm thinking about dropping this file from this series and submitting
>> it separately for Greg to Ack.
>>
>> Unless he sees our exchange and acks it here. :)
>
> I can't ack a patch that is incorrect :(
>
> Please fix it up and resend...
>

Oh yes, sure, I was just waiting for more reviews before resending v2.
It's 4.18 material anyway.

Thanks,
Bartosz

[PATCH 05/21] eeprom: at24: remove code separators

[Bartosz Golaszewski]

These are just two left-overs from times when this driver was bigger.

They are not really useful anymore. Remove them.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 0e06201d33c9..d1ad73d94fa8 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -235,8 +235,6 @@ static const struct acpi_device_id at24_acpi_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, at24_acpi_ids);
 
-/*-------------------------------------------------------------------------*/
-
 /*
  * This routine supports chips which consume multiple I2C addresses. It
  * computes the addressing information to be used for a given r/w request.
@@ -712,8 +710,6 @@ static int at24_remove(struct i2c_client *client)
 	return 0;
 }
 
-/*-------------------------------------------------------------------------*/
-
 static struct i2c_driver at24_driver = {
 	.driver = {
 		.name = "at24",
-- 
2.16.1

[PATCH 06/21] eeprom: at24: drop redundant variable in at24_read()

[Bartosz Golaszewski]

We can reuse ret instead of defining a loop-local status variable.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index d1ad73d94fa8..ff7eb8e382c2 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -392,17 +392,15 @@ static int at24_read(void *priv, unsigned int off, void *val, size_t count)
 	mutex_lock(&at24->lock);
 
 	while (count) {
-		int	status;
-
-		status = at24_regmap_read(at24, buf, off, count);
-		if (status < 0) {
+		ret = at24_regmap_read(at24, buf, off, count);
+		if (ret < 0) {
 			mutex_unlock(&at24->lock);
 			pm_runtime_put(dev);
-			return status;
+			return ret;
 		}
-		buf += status;
-		off += status;
-		count -= status;
+		buf += ret;
+		off += ret;
+		count -= ret;
 	}
 
 	mutex_unlock(&at24->lock);
-- 
2.16.1

[PATCH 07/21] eeprom: at24: drop redundant variable in at24_write()

[Bartosz Golaszewski]

We can reuse ret instead of defining a loop-local status variable.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index ff7eb8e382c2..d9d55310ffa4 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -440,18 +440,16 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
 	gpiod_set_value_cansleep(at24->wp_gpio, 0);
 
 	while (count) {
-		int status;
-
-		status = at24_regmap_write(at24, buf, off, count);
-		if (status < 0) {
+		ret = at24_regmap_write(at24, buf, off, count);
+		if (ret < 0) {
 			gpiod_set_value_cansleep(at24->wp_gpio, 1);
 			mutex_unlock(&at24->lock);
 			pm_runtime_put(dev);
-			return status;
+			return ret;
 		}
-		buf += status;
-		off += status;
-		count -= status;
+		buf += ret;
+		off += ret;
+		count -= ret;
 	}
 
 	gpiod_set_value_cansleep(at24->wp_gpio, 1);
-- 
2.16.1

[PATCH 08/21] eeprom: at24: make struct initialization uniform in at24_probe()

[Bartosz Golaszewski]

When zeroing structs, use "{ }" everywhere.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index d9d55310ffa4..83a9223e62fb 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -515,7 +515,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct regmap_config regmap_config = { };
 	struct nvmem_config nvmem_config = { };
 	const struct at24_chip_data *cd = NULL;
-	struct at24_platform_data chip = { 0 };
+	struct at24_platform_data chip = { };
 	unsigned int i, num_addresses;
 	struct at24_data *at24;
 	bool writable;
-- 
2.16.1

[PATCH 09/21] eeprom: at24: don't check if byte_len is a power of 2

[Bartosz Golaszewski]

We support certain models the size of which is not a power of 2. This
is not a reason to emit a warning.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 83a9223e62fb..d386df6b6100 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -550,9 +550,6 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		at24_get_pdata(&client->dev, &chip);
 	}
 
-	if (!is_power_of_2(chip.byte_len))
-		dev_warn(&client->dev,
-			"byte_len looks suspicious (no power of 2)!\n");
 	if (!chip.page_size) {
 		dev_err(&client->dev, "page_size must not be 0!\n");
 		return -EINVAL;
-- 
2.16.1

[PATCH 10/21] eeprom: at24: rename at24_get_pdata()

[Bartosz Golaszewski]

As preparation for at24_probe() refactoring: rename at24_get_pdata()
to at24_properties_to_pdata(). We're doing it because we'll move the
pdata parsing code into a separate function which will be called
at24_get_pdata(). Current routine with that name actually parses
the device properties so change its name to reflect its purpose.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index d386df6b6100..dad5b0f0606e 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -460,7 +460,8 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
 	return 0;
 }
 
-static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
+static void at24_properties_to_pdata(struct device *dev,
+				     struct at24_platform_data *chip)
 {
 	int err;
 	u32 val;
@@ -547,7 +548,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 		chip.byte_len = cd->byte_len;
 		chip.flags = cd->flags;
-		at24_get_pdata(&client->dev, &chip);
+		at24_properties_to_pdata(&client->dev, &chip);
 	}
 
 	if (!chip.page_size) {
-- 
2.16.1

[PATCH 11/21] eeprom: at24: rename chip to pdata in at24_probe()

[Bartosz Golaszewski]

Reflect the purpose of this variable: it contains platform data so name
it such.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index dad5b0f0606e..93431d1296d9 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -516,7 +516,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct regmap_config regmap_config = { };
 	struct nvmem_config nvmem_config = { };
 	const struct at24_chip_data *cd = NULL;
-	struct at24_platform_data chip = { };
+	struct at24_platform_data pdata = { };
 	unsigned int i, num_addresses;
 	struct at24_data *at24;
 	bool writable;
@@ -524,7 +524,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	int err;
 
 	if (client->dev.platform_data) {
-		chip = *(struct at24_platform_data *)client->dev.platform_data;
+		pdata = *(struct at24_platform_data *)client->dev.platform_data;
 	} else {
 		/*
 		 * The I2C core allows OF nodes compatibles to match against the
@@ -546,32 +546,32 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		if (!cd)
 			return -ENODEV;
 
-		chip.byte_len = cd->byte_len;
-		chip.flags = cd->flags;
-		at24_properties_to_pdata(&client->dev, &chip);
+		pdata.byte_len = cd->byte_len;
+		pdata.flags = cd->flags;
+		at24_properties_to_pdata(&client->dev, &pdata);
 	}
 
-	if (!chip.page_size) {
+	if (!pdata.page_size) {
 		dev_err(&client->dev, "page_size must not be 0!\n");
 		return -EINVAL;
 	}
-	if (!is_power_of_2(chip.page_size))
+	if (!is_power_of_2(pdata.page_size))
 		dev_warn(&client->dev,
 			"page_size looks suspicious (no power of 2)!\n");
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
 	    !i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_WRITE_I2C_BLOCK))
-		chip.page_size = 1;
+		pdata.page_size = 1;
 
-	if (chip.flags & AT24_FLAG_TAKE8ADDR)
+	if (pdata.flags & AT24_FLAG_TAKE8ADDR)
 		num_addresses = 8;
 	else
-		num_addresses =	DIV_ROUND_UP(chip.byte_len,
-			(chip.flags & AT24_FLAG_ADDR16) ? 65536 : 256);
+		num_addresses =	DIV_ROUND_UP(pdata.byte_len,
+			(pdata.flags & AT24_FLAG_ADDR16) ? 65536 : 256);
 
 	regmap_config.val_bits = 8;
-	regmap_config.reg_bits = (chip.flags & AT24_FLAG_ADDR16) ? 16 : 8;
+	regmap_config.reg_bits = (pdata.flags & AT24_FLAG_ADDR16) ? 16 : 8;
 	regmap_config.disable_locking = true;
 
 	at24 = devm_kzalloc(&client->dev, sizeof(struct at24_data) +
@@ -580,9 +580,9 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return -ENOMEM;
 
 	mutex_init(&at24->lock);
-	at24->chip = chip;
+	at24->chip = pdata;
 	at24->num_addresses = num_addresses;
-	at24->offset_adj = at24_get_offset_adj(chip.flags, chip.byte_len);
+	at24->offset_adj = at24_get_offset_adj(pdata.flags, pdata.byte_len);
 
 	at24->wp_gpio = devm_gpiod_get_optional(&client->dev,
 						"wp", GPIOD_OUT_HIGH);
@@ -594,16 +594,16 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (IS_ERR(at24->client[0].regmap))
 		return PTR_ERR(at24->client[0].regmap);
 
-	if ((chip.flags & AT24_FLAG_SERIAL) && (chip.flags & AT24_FLAG_MAC)) {
+	if ((pdata.flags & AT24_FLAG_SERIAL) && (pdata.flags & AT24_FLAG_MAC)) {
 		dev_err(&client->dev,
 			"invalid device data - cannot have both AT24_FLAG_SERIAL & AT24_FLAG_MAC.");
 		return -EINVAL;
 	}
 
-	writable = !(chip.flags & AT24_FLAG_READONLY);
+	writable = !(pdata.flags & AT24_FLAG_READONLY);
 	if (writable) {
 		at24->write_max = min_t(unsigned int,
-					chip.page_size, at24_io_limit);
+					pdata.page_size, at24_io_limit);
 		if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
 		    at24->write_max > I2C_SMBUS_BLOCK_MAX)
 			at24->write_max = I2C_SMBUS_BLOCK_MAX;
@@ -657,7 +657,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	nvmem_config.priv = at24;
 	nvmem_config.stride = 1;
 	nvmem_config.word_size = 1;
-	nvmem_config.size = chip.byte_len;
+	nvmem_config.size = pdata.byte_len;
 
 	at24->nvmem = nvmem_register(&nvmem_config);
 
@@ -667,12 +667,12 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	}
 
 	dev_info(&client->dev, "%u byte %s EEPROM, %s, %u bytes/write\n",
-		chip.byte_len, client->name,
+		pdata.byte_len, client->name,
 		writable ? "writable" : "read-only", at24->write_max);
 
 	/* export data to kernel code */
-	if (chip.setup)
-		chip.setup(at24->nvmem, chip.context);
+	if (pdata.setup)
+		pdata.setup(at24->nvmem, pdata.context);
 
 	return 0;
 
-- 
2.16.1

[PATCH 12/21] eeprom: at24: use a helper variable for dev

[Bartosz Golaszewski]

We use the &client->dev construct all over in at24_probe(). Use
a helper variable which is more readable and allows to avoid a couple
unnecessary line breaks.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 48 ++++++++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 93431d1296d9..e4c1997c2b56 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -517,29 +517,29 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct nvmem_config nvmem_config = { };
 	const struct at24_chip_data *cd = NULL;
 	struct at24_platform_data pdata = { };
+	struct device *dev = &client->dev;
 	unsigned int i, num_addresses;
 	struct at24_data *at24;
 	bool writable;
 	u8 test_byte;
 	int err;
 
-	if (client->dev.platform_data) {
-		pdata = *(struct at24_platform_data *)client->dev.platform_data;
+	if (dev->platform_data) {
+		pdata = *(struct at24_platform_data *)dev->platform_data;
 	} else {
 		/*
 		 * The I2C core allows OF nodes compatibles to match against the
 		 * I2C device ID table as a fallback, so check not only if an OF
 		 * node is present but also if it matches an OF device ID entry.
 		 */
-		if (client->dev.of_node &&
-		    of_match_device(at24_of_match, &client->dev)) {
-			cd = of_device_get_match_data(&client->dev);
+		if (dev->of_node && of_match_device(at24_of_match, dev)) {
+			cd = of_device_get_match_data(dev);
 		} else if (id) {
 			cd = (void *)id->driver_data;
 		} else {
 			const struct acpi_device_id *aid;
 
-			aid = acpi_match_device(at24_acpi_ids, &client->dev);
+			aid = acpi_match_device(at24_acpi_ids, dev);
 			if (aid)
 				cd = (void *)aid->driver_data;
 		}
@@ -548,16 +548,15 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 		pdata.byte_len = cd->byte_len;
 		pdata.flags = cd->flags;
-		at24_properties_to_pdata(&client->dev, &pdata);
+		at24_properties_to_pdata(dev, &pdata);
 	}
 
 	if (!pdata.page_size) {
-		dev_err(&client->dev, "page_size must not be 0!\n");
+		dev_err(dev, "page_size must not be 0!\n");
 		return -EINVAL;
 	}
 	if (!is_power_of_2(pdata.page_size))
-		dev_warn(&client->dev,
-			"page_size looks suspicious (no power of 2)!\n");
+		dev_warn(dev, "page_size looks suspicious (no power of 2)!\n");
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
 	    !i2c_check_functionality(client->adapter,
@@ -574,8 +573,8 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	regmap_config.reg_bits = (pdata.flags & AT24_FLAG_ADDR16) ? 16 : 8;
 	regmap_config.disable_locking = true;
 
-	at24 = devm_kzalloc(&client->dev, sizeof(struct at24_data) +
-		num_addresses * sizeof(struct at24_client), GFP_KERNEL);
+	at24 = devm_kzalloc(dev, sizeof(struct at24_data) + num_addresses *
+			    sizeof(struct at24_client), GFP_KERNEL);
 	if (!at24)
 		return -ENOMEM;
 
@@ -584,8 +583,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	at24->num_addresses = num_addresses;
 	at24->offset_adj = at24_get_offset_adj(pdata.flags, pdata.byte_len);
 
-	at24->wp_gpio = devm_gpiod_get_optional(&client->dev,
-						"wp", GPIOD_OUT_HIGH);
+	at24->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_HIGH);
 	if (IS_ERR(at24->wp_gpio))
 		return PTR_ERR(at24->wp_gpio);
 
@@ -595,7 +593,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return PTR_ERR(at24->client[0].regmap);
 
 	if ((pdata.flags & AT24_FLAG_SERIAL) && (pdata.flags & AT24_FLAG_MAC)) {
-		dev_err(&client->dev,
+		dev_err(dev,
 			"invalid device data - cannot have both AT24_FLAG_SERIAL & AT24_FLAG_MAC.");
 		return -EINVAL;
 	}
@@ -614,8 +612,8 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		at24->client[i].client = i2c_new_dummy(client->adapter,
 						       client->addr + i);
 		if (!at24->client[i].client) {
-			dev_err(&client->dev, "address 0x%02x unavailable\n",
-					client->addr + i);
+			dev_err(dev, "address 0x%02x unavailable\n",
+				client->addr + i);
 			err = -EADDRINUSE;
 			goto err_clients;
 		}
@@ -631,27 +629,27 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	i2c_set_clientdata(client, at24);
 
 	/* enable runtime pm */
-	pm_runtime_set_active(&client->dev);
-	pm_runtime_enable(&client->dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
 
 	/*
 	 * Perform a one-byte test read to verify that the
 	 * chip is functional.
 	 */
 	err = at24_read(at24, 0, &test_byte, 1);
-	pm_runtime_idle(&client->dev);
+	pm_runtime_idle(dev);
 	if (err) {
 		err = -ENODEV;
 		goto err_clients;
 	}
 
-	nvmem_config.name = dev_name(&client->dev);
-	nvmem_config.dev = &client->dev;
+	nvmem_config.name = dev_name(dev);
+	nvmem_config.dev = dev;
 	nvmem_config.read_only = !writable;
 	nvmem_config.root_only = true;
 	nvmem_config.owner = THIS_MODULE;
 	nvmem_config.compat = true;
-	nvmem_config.base_dev = &client->dev;
+	nvmem_config.base_dev = dev;
 	nvmem_config.reg_read = at24_read;
 	nvmem_config.reg_write = at24_write;
 	nvmem_config.priv = at24;
@@ -666,7 +664,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		goto err_clients;
 	}
 
-	dev_info(&client->dev, "%u byte %s EEPROM, %s, %u bytes/write\n",
+	dev_info(dev, "%u byte %s EEPROM, %s, %u bytes/write\n",
 		pdata.byte_len, client->name,
 		writable ? "writable" : "read-only", at24->write_max);
 
@@ -681,7 +679,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		if (at24->client[i].client)
 			i2c_unregister_device(at24->client[i].client);
 
-	pm_runtime_disable(&client->dev);
+	pm_runtime_disable(dev);
 
 	return err;
 }
-- 
2.16.1

[PATCH 13/21] eeprom: at24: readability tweak in at24_probe()

[Bartosz Golaszewski]

Use a helper variable for the size we want to allocate with
devm_kzalloc() and save an ugly line break.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index e4c1997c2b56..ea5e81cb8e8c 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -520,6 +520,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct device *dev = &client->dev;
 	unsigned int i, num_addresses;
 	struct at24_data *at24;
+	size_t at24_size;
 	bool writable;
 	u8 test_byte;
 	int err;
@@ -573,8 +574,8 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	regmap_config.reg_bits = (pdata.flags & AT24_FLAG_ADDR16) ? 16 : 8;
 	regmap_config.disable_locking = true;
 
-	at24 = devm_kzalloc(dev, sizeof(struct at24_data) + num_addresses *
-			    sizeof(struct at24_client), GFP_KERNEL);
+	at24_size = sizeof(*at24) + num_addresses * sizeof(struct at24_client);
+	at24 = devm_kzalloc(dev, at24_size, GFP_KERNEL);
 	if (!at24)
 		return -ENOMEM;
 
-- 
2.16.1

[PATCH 14/21] eeprom: at24: provide and use at24_base_client_dev()

[Bartosz Golaszewski]

Use a helper function for accessing the device struct of the base
i2c client. This routine is named in a way that reflects its purpose
unlike the previously hand-coded dereferencing.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index ea5e81cb8e8c..02b710919b8b 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -260,6 +260,11 @@ static struct at24_client *at24_translate_offset(struct at24_data *at24,
 	return &at24->client[i];
 }
 
+static struct device *at24_base_client_dev(struct at24_data *at24)
+{
+	return &at24->client[0].client->dev;
+}
+
 static size_t at24_adjust_read_count(struct at24_data *at24,
 				      unsigned int offset, size_t count)
 {
@@ -371,7 +376,7 @@ static int at24_read(void *priv, unsigned int off, void *val, size_t count)
 	int ret;
 
 	at24 = priv;
-	dev = &at24->client[0].client->dev;
+	dev = at24_base_client_dev(at24);
 
 	if (unlikely(!count))
 		return count;
@@ -418,7 +423,7 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
 	int ret;
 
 	at24 = priv;
-	dev = &at24->client[0].client->dev;
+	dev = at24_base_client_dev(at24);
 
 	if (unlikely(!count))
 		return -EINVAL;
-- 
2.16.1

[PATCH 15/21] eeprom: at24: switch to using probe_new() from the i2c framework

[Bartosz Golaszewski]

Use the new probe() style for i2c drivers.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 02b710919b8b..243d46912f29 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -516,13 +516,14 @@ static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len)
 	}
 }
 
-static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
+static int at24_probe(struct i2c_client *client)
 {
 	struct regmap_config regmap_config = { };
 	struct nvmem_config nvmem_config = { };
 	const struct at24_chip_data *cd = NULL;
 	struct at24_platform_data pdata = { };
 	struct device *dev = &client->dev;
+	const struct i2c_device_id *id;
 	unsigned int i, num_addresses;
 	struct at24_data *at24;
 	size_t at24_size;
@@ -530,6 +531,8 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	u8 test_byte;
 	int err;
 
+	id = i2c_match_id(at24_ids, client);
+
 	if (dev->platform_data) {
 		pdata = *(struct at24_platform_data *)dev->platform_data;
 	} else {
@@ -714,7 +717,7 @@ static struct i2c_driver at24_driver = {
 		.of_match_table = at24_of_match,
 		.acpi_match_table = ACPI_PTR(at24_acpi_ids),
 	},
-	.probe = at24_probe,
+	.probe_new = at24_probe,
 	.remove = at24_remove,
 	.id_table = at24_ids,
 };
-- 
2.16.1

[PATCH 16/21] eeprom: at24: move platform data processing into a separate routine

[Bartosz Golaszewski]

This driver can receive its device data from different sources
depending on the system. Move the entire code processing platform data,
device tree and acpi into a separate function.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 70 ++++++++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 243d46912f29..bed6917468e1 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -493,6 +493,43 @@ static void at24_properties_to_pdata(struct device *dev,
 	}
 }
 
+static int at24_get_pdata(struct device *dev, struct at24_platform_data *pdata)
+{
+	struct device_node *of_node = dev->of_node;
+	const struct at24_chip_data *cdata;
+	const struct i2c_device_id *id;
+	struct at24_platform_data *pd;
+
+	pd = dev_get_platdata(dev);
+	if (pd) {
+		memcpy(pdata, pd, sizeof(*pdata));
+		return 0;
+	}
+
+	id = i2c_match_id(at24_ids, to_i2c_client(dev));
+
+	/*
+	 * The I2C core allows OF nodes compatibles to match against the
+	 * I2C device ID table as a fallback, so check not only if an OF
+	 * node is present but also if it matches an OF device ID entry.
+	 */
+	if (of_node && of_match_device(at24_of_match, dev))
+		cdata = of_device_get_match_data(dev);
+	else if (id)
+		cdata = (void *)&id->driver_data;
+	else
+		cdata = acpi_device_get_match_data(dev);
+
+	if (!cdata)
+		return -ENODEV;
+
+	pdata->byte_len = cdata->byte_len;
+	pdata->flags = cdata->flags;
+	at24_properties_to_pdata(dev, pdata);
+
+	return 0;
+}
+
 static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len)
 {
 	if (flags & AT24_FLAG_MAC) {
@@ -520,10 +557,8 @@ static int at24_probe(struct i2c_client *client)
 {
 	struct regmap_config regmap_config = { };
 	struct nvmem_config nvmem_config = { };
-	const struct at24_chip_data *cd = NULL;
 	struct at24_platform_data pdata = { };
 	struct device *dev = &client->dev;
-	const struct i2c_device_id *id;
 	unsigned int i, num_addresses;
 	struct at24_data *at24;
 	size_t at24_size;
@@ -531,34 +566,9 @@ static int at24_probe(struct i2c_client *client)
 	u8 test_byte;
 	int err;
 
-	id = i2c_match_id(at24_ids, client);
-
-	if (dev->platform_data) {
-		pdata = *(struct at24_platform_data *)dev->platform_data;
-	} else {
-		/*
-		 * The I2C core allows OF nodes compatibles to match against the
-		 * I2C device ID table as a fallback, so check not only if an OF
-		 * node is present but also if it matches an OF device ID entry.
-		 */
-		if (dev->of_node && of_match_device(at24_of_match, dev)) {
-			cd = of_device_get_match_data(dev);
-		} else if (id) {
-			cd = (void *)id->driver_data;
-		} else {
-			const struct acpi_device_id *aid;
-
-			aid = acpi_match_device(at24_acpi_ids, dev);
-			if (aid)
-				cd = (void *)aid->driver_data;
-		}
-		if (!cd)
-			return -ENODEV;
-
-		pdata.byte_len = cd->byte_len;
-		pdata.flags = cd->flags;
-		at24_properties_to_pdata(dev, &pdata);
-	}
+	err = at24_get_pdata(dev, &pdata);
+	if (err)
+		return err;
 
 	if (!pdata.page_size) {
 		dev_err(dev, "page_size must not be 0!\n");
-- 
2.16.1

[PATCH 17/21] eeprom: at24: remove at24_platform_data from at24_data

[Bartosz Golaszewski]

Not all fields from at24_platform_data are needed in at24_data. Let's
keep just the ones we need and not carry the whole platform_data
structure all the time.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index bed6917468e1..4888999a62cc 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -60,8 +60,6 @@ struct at24_client {
 };
 
 struct at24_data {
-	struct at24_platform_data chip;
-
 	/*
 	 * Lock protects against activities from other Linux tasks,
 	 * but not from changes by other I2C masters.
@@ -72,6 +70,10 @@ struct at24_data {
 	unsigned int num_addresses;
 	unsigned int offset_adj;
 
+	u32 byte_len;
+	u16 page_size;
+	u8 flags;
+
 	struct nvmem_device *nvmem;
 
 	struct gpio_desc *wp_gpio;
@@ -249,7 +251,7 @@ static struct at24_client *at24_translate_offset(struct at24_data *at24,
 {
 	unsigned int i;
 
-	if (at24->chip.flags & AT24_FLAG_ADDR16) {
+	if (at24->flags & AT24_FLAG_ADDR16) {
 		i = *offset >> 16;
 		*offset &= 0xffff;
 	} else {
@@ -276,8 +278,8 @@ static size_t at24_adjust_read_count(struct at24_data *at24,
 	 * the next slave address: truncate the count to the slave boundary,
 	 * so that the read never straddles slaves.
 	 */
-	if (at24->chip.flags & AT24_FLAG_NO_RDROL) {
-		bits = (at24->chip.flags & AT24_FLAG_ADDR16) ? 16 : 8;
+	if (at24->flags & AT24_FLAG_NO_RDROL) {
+		bits = (at24->flags & AT24_FLAG_ADDR16) ? 16 : 8;
 		remainder = BIT(bits) - offset;
 		if (count > remainder)
 			count = remainder;
@@ -336,7 +338,7 @@ static size_t at24_adjust_write_count(struct at24_data *at24,
 		count = at24->write_max;
 
 	/* Never roll over backwards, to the start of this page */
-	next_page = roundup(offset + 1, at24->chip.page_size);
+	next_page = roundup(offset + 1, at24->page_size);
 	if (offset + count > next_page)
 		count = next_page - offset;
 
@@ -381,7 +383,7 @@ static int at24_read(void *priv, unsigned int off, void *val, size_t count)
 	if (unlikely(!count))
 		return count;
 
-	if (off + count > at24->chip.byte_len)
+	if (off + count > at24->byte_len)
 		return -EINVAL;
 
 	ret = pm_runtime_get_sync(dev);
@@ -428,7 +430,7 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
 	if (unlikely(!count))
 		return -EINVAL;
 
-	if (off + count > at24->chip.byte_len)
+	if (off + count > at24->byte_len)
 		return -EINVAL;
 
 	ret = pm_runtime_get_sync(dev);
@@ -598,7 +600,9 @@ static int at24_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	mutex_init(&at24->lock);
-	at24->chip = pdata;
+	at24->byte_len = pdata.byte_len;
+	at24->page_size = pdata.page_size;
+	at24->flags = pdata.flags;
 	at24->num_addresses = num_addresses;
 	at24->offset_adj = at24_get_offset_adj(pdata.flags, pdata.byte_len);
 
-- 
2.16.1

[PATCH 18/21] eeprom: at24: refactor at24_probe()

[Bartosz Golaszewski]

The code in at24_probe() is pretty mangled. It can be cleaned up a bit
by doing things one by one.

Let's group the code by logic: parse and verify pdata, initialize the
regmap, allocate and fill the fields of at24_data, allocate dummy i2c
devices, initialize pm & register with nvmem.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 4888999a62cc..f67f5f626617 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -563,6 +563,7 @@ static int at24_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	unsigned int i, num_addresses;
 	struct at24_data *at24;
+	struct regmap *regmap;
 	size_t at24_size;
 	bool writable;
 	u8 test_byte;
@@ -572,6 +573,11 @@ static int at24_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
+	    !i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_WRITE_I2C_BLOCK))
+		pdata.page_size = 1;
+
 	if (!pdata.page_size) {
 		dev_err(dev, "page_size must not be 0!\n");
 		return -EINVAL;
@@ -579,21 +585,26 @@ static int at24_probe(struct i2c_client *client)
 	if (!is_power_of_2(pdata.page_size))
 		dev_warn(dev, "page_size looks suspicious (no power of 2)!\n");
 
-	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
-	    !i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_WRITE_I2C_BLOCK))
-		pdata.page_size = 1;
-
 	if (pdata.flags & AT24_FLAG_TAKE8ADDR)
 		num_addresses = 8;
 	else
 		num_addresses =	DIV_ROUND_UP(pdata.byte_len,
 			(pdata.flags & AT24_FLAG_ADDR16) ? 65536 : 256);
 
+	if ((pdata.flags & AT24_FLAG_SERIAL) && (pdata.flags & AT24_FLAG_MAC)) {
+		dev_err(dev,
+			"invalid device data - cannot have both AT24_FLAG_SERIAL & AT24_FLAG_MAC.");
+		return -EINVAL;
+	}
+
 	regmap_config.val_bits = 8;
 	regmap_config.reg_bits = (pdata.flags & AT24_FLAG_ADDR16) ? 16 : 8;
 	regmap_config.disable_locking = true;
 
+	regmap = devm_regmap_init_i2c(client, &regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
 	at24_size = sizeof(*at24) + num_addresses * sizeof(struct at24_client);
 	at24 = devm_kzalloc(dev, at24_size, GFP_KERNEL);
 	if (!at24)
@@ -605,22 +616,13 @@ static int at24_probe(struct i2c_client *client)
 	at24->flags = pdata.flags;
 	at24->num_addresses = num_addresses;
 	at24->offset_adj = at24_get_offset_adj(pdata.flags, pdata.byte_len);
+	at24->client[0].client = client;
+	at24->client[0].regmap = regmap;
 
 	at24->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_HIGH);
 	if (IS_ERR(at24->wp_gpio))
 		return PTR_ERR(at24->wp_gpio);
 
-	at24->client[0].client = client;
-	at24->client[0].regmap = devm_regmap_init_i2c(client, &regmap_config);
-	if (IS_ERR(at24->client[0].regmap))
-		return PTR_ERR(at24->client[0].regmap);
-
-	if ((pdata.flags & AT24_FLAG_SERIAL) && (pdata.flags & AT24_FLAG_MAC)) {
-		dev_err(dev,
-			"invalid device data - cannot have both AT24_FLAG_SERIAL & AT24_FLAG_MAC.");
-		return -EINVAL;
-	}
-
 	writable = !(pdata.flags & AT24_FLAG_READONLY);
 	if (writable) {
 		at24->write_max = min_t(unsigned int,
-- 
2.16.1

[PATCH 19/21] eeprom: at24: tweak newlines

[Bartosz Golaszewski]

Remove the newline between the nvmem registration and its return value
check. This is consistent with the rest of the driver code.

Add a missing newline between two pdata checks to stay consistent with
all the others.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index f67f5f626617..b4233d914500 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -582,6 +582,7 @@ static int at24_probe(struct i2c_client *client)
 		dev_err(dev, "page_size must not be 0!\n");
 		return -EINVAL;
 	}
+
 	if (!is_power_of_2(pdata.page_size))
 		dev_warn(dev, "page_size looks suspicious (no power of 2)!\n");
 
@@ -683,7 +684,6 @@ static int at24_probe(struct i2c_client *client)
 	nvmem_config.size = pdata.byte_len;
 
 	at24->nvmem = nvmem_register(&nvmem_config);
-
 	if (IS_ERR(at24->nvmem)) {
 		err = PTR_ERR(at24->nvmem);
 		goto err_clients;
-- 
2.16.1

[PATCH 20/21] eeprom: at24: fix a line break

[Bartosz Golaszewski]

Align the broken line with the opening parenthesis to stay consistent
with the rest of the driver code.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index b4233d914500..c42e479b880a 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -690,8 +690,8 @@ static int at24_probe(struct i2c_client *client)
 	}
 
 	dev_info(dev, "%u byte %s EEPROM, %s, %u bytes/write\n",
-		pdata.byte_len, client->name,
-		writable ? "writable" : "read-only", at24->write_max);
+		 pdata.byte_len, client->name,
+		 writable ? "writable" : "read-only", at24->write_max);
 
 	/* export data to kernel code */
 	if (pdata.setup)
-- 
2.16.1

[PATCH 21/21] eeprom: at24: simplify the i2c functionality checking

[Bartosz Golaszewski]

Save one call and make code prettier by checking the i2c functionality
in the beginning of at24_probe(), saving the relevant values and
reusing them later.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/misc/eeprom/at24.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index c42e479b880a..d15934b876c1 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -561,6 +561,7 @@ static int at24_probe(struct i2c_client *client)
 	struct nvmem_config nvmem_config = { };
 	struct at24_platform_data pdata = { };
 	struct device *dev = &client->dev;
+	bool i2c_fn_i2c, i2c_fn_block;
 	unsigned int i, num_addresses;
 	struct at24_data *at24;
 	struct regmap *regmap;
@@ -569,13 +570,15 @@ static int at24_probe(struct i2c_client *client)
 	u8 test_byte;
 	int err;
 
+	i2c_fn_i2c = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
+	i2c_fn_block = i2c_check_functionality(client->adapter,
+					       I2C_FUNC_SMBUS_WRITE_I2C_BLOCK);
+
 	err = at24_get_pdata(dev, &pdata);
 	if (err)
 		return err;
 
-	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
-	    !i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_WRITE_I2C_BLOCK))
+	if (!i2c_fn_i2c && !i2c_fn_block)
 		pdata.page_size = 1;
 
 	if (!pdata.page_size) {
@@ -628,8 +631,7 @@ static int at24_probe(struct i2c_client *client)
 	if (writable) {
 		at24->write_max = min_t(unsigned int,
 					pdata.page_size, at24_io_limit);
-		if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
-		    at24->write_max > I2C_SMBUS_BLOCK_MAX)
+		if (!i2c_fn_i2c && at24->write_max > I2C_SMBUS_BLOCK_MAX)
 			at24->write_max = I2C_SMBUS_BLOCK_MAX;
 	}
 
-- 
2.16.1

Re: [PATCH 00/21] eeprom: at24: driver refactoring

[Andy Shevchenko]

On Mon, Mar 19, 2018 at 11:17 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> This series contains what I hope to be a non-controversial refactoring
> of the at24 eeprom driver.
>
> Most changes revolve around at24_probe() which became quite complicated
> and hard to read.
>
> The only functional changes are: disabling the internal locking
> mechanisms of regmap (since we already take care of that in the driver)
> and removing an if checking if byte_len is a power of 2 (as we do
> support models for which it's not true).
>
> All other patches affect readability and code structure.
>
> Tested with a couple models and different both for device tree and
> platform data modes.

Is there any available tree with that series applied?
I would test it on Intel Galileo Gen 2 which has ACPI enumerated AT24
EEPROM attached.

>
> Bartosz Golaszewski (21):
>   eeprom: at24: disable regmap locking
>   eeprom: at24: remove nvmem_config from at24_data
>   eeprom: at24: arrange local variables
>   eeprom: at24: use SPDX identifier instead of GPL boiler-plate
>   eeprom: at24: remove code separators
>   eeprom: at24: drop redundant variable in at24_read()
>   eeprom: at24: drop redundant variable in at24_write()
>   eeprom: at24: make struct initialization uniform in at24_probe()
>   eeprom: at24: don't check if byte_len is a power of 2
>   eeprom: at24: rename at24_get_pdata()
>   eeprom: at24: rename chip to pdata in at24_probe()
>   eeprom: at24: use a helper variable for dev
>   eeprom: at24: readability tweak in at24_probe()
>   eeprom: at24: provide and use at24_base_client_dev()
>   eeprom: at24: switch to using probe_new() from the i2c framework
>   eeprom: at24: move platform data processing into a separate routine
>   eeprom: at24: remove at24_platform_data from at24_data
>   eeprom: at24: refactor at24_probe()
>   eeprom: at24: tweak newlines
>   eeprom: at24: fix a line break
>   eeprom: at24: simplify the i2c functionality checking
>
>  drivers/misc/eeprom/at24.c | 293 ++++++++++++++++++++++++---------------------
>  1 file changed, 156 insertions(+), 137 deletions(-)
>
> --
> 2.16.1
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 00/21] eeprom: at24: driver refactoring

[Bartosz Golaszewski]

2018-03-19 15:43 GMT+01:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
> On Mon, Mar 19, 2018 at 11:17 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> This series contains what I hope to be a non-controversial refactoring
>> of the at24 eeprom driver.
>>
>> Most changes revolve around at24_probe() which became quite complicated
>> and hard to read.
>>
>> The only functional changes are: disabling the internal locking
>> mechanisms of regmap (since we already take care of that in the driver)
>> and removing an if checking if byte_len is a power of 2 (as we do
>> support models for which it's not true).
>>
>> All other patches affect readability and code structure.
>>
>> Tested with a couple models and different both for device tree and
>> platform data modes.
>
> Is there any available tree with that series applied?
> I would test it on Intel Galileo Gen 2 which has ACPI enumerated AT24
> EEPROM attached.
>

Yes, it's in my github tree:

    https://github.com/brgl/linux    topic/at24/refactoring

Thanks in advance for testing it!

Best regards,
Bartosz

Re: [PATCH 00/21] eeprom: at24: driver refactoring

[Andy Shevchenko]

On Mon, Mar 19, 2018 at 5:21 PM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 2018-03-19 15:43 GMT+01:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
>> On Mon, Mar 19, 2018 at 11:17 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>> This series contains what I hope to be a non-controversial refactoring
>>> of the at24 eeprom driver.
>>>
>>> Most changes revolve around at24_probe() which became quite complicated
>>> and hard to read.
>>>
>>> The only functional changes are: disabling the internal locking
>>> mechanisms of regmap (since we already take care of that in the driver)
>>> and removing an if checking if byte_len is a power of 2 (as we do
>>> support models for which it's not true).
>>>
>>> All other patches affect readability and code structure.
>>>
>>> Tested with a couple models and different both for device tree and
>>> platform data modes.
>>
>> Is there any available tree with that series applied?
>> I would test it on Intel Galileo Gen 2 which has ACPI enumerated AT24
>> EEPROM attached.
>>
>
> Yes, it's in my github tree:
>
>     https://github.com/brgl/linux    topic/at24/refactoring
>
> Thanks in advance for testing it!

At least this didn't break AT24 on Intel Galileo Gen 2 board in ACPI mode.

Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 00/21] eeprom: at24: driver refactoring

[Greg Kroah-Hartman]

On Wed, Mar 21, 2018 at 04:52:19PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 19, 2018 at 5:21 PM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > 2018-03-19 15:43 GMT+01:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
> >> On Mon, Mar 19, 2018 at 11:17 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >>> This series contains what I hope to be a non-controversial refactoring
> >>> of the at24 eeprom driver.
> >>>
> >>> Most changes revolve around at24_probe() which became quite complicated
> >>> and hard to read.
> >>>
> >>> The only functional changes are: disabling the internal locking
> >>> mechanisms of regmap (since we already take care of that in the driver)
> >>> and removing an if checking if byte_len is a power of 2 (as we do
> >>> support models for which it's not true).
> >>>
> >>> All other patches affect readability and code structure.
> >>>
> >>> Tested with a couple models and different both for device tree and
> >>> platform data modes.
> >>
> >> Is there any available tree with that series applied?
> >> I would test it on Intel Galileo Gen 2 which has ACPI enumerated AT24
> >> EEPROM attached.
> >>
> >
> > Yes, it's in my github tree:
> >
> >     https://github.com/brgl/linux    topic/at24/refactoring
> >
> > Thanks in advance for testing it!
> 
> At least this didn't break AT24 on Intel Galileo Gen 2 board in ACPI mode.
> 
> Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>

All applied except for patch 4.

thanks,

greg k-h

Re: [PATCH 00/21] eeprom: at24: driver refactoring

[Bartosz Golaszewski]

2018-03-23 16:26 GMT+01:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> On Wed, Mar 21, 2018 at 04:52:19PM +0200, Andy Shevchenko wrote:
>> On Mon, Mar 19, 2018 at 5:21 PM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> > 2018-03-19 15:43 GMT+01:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
>> >> On Mon, Mar 19, 2018 at 11:17 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> >>> This series contains what I hope to be a non-controversial refactoring
>> >>> of the at24 eeprom driver.
>> >>>
>> >>> Most changes revolve around at24_probe() which became quite complicated
>> >>> and hard to read.
>> >>>
>> >>> The only functional changes are: disabling the internal locking
>> >>> mechanisms of regmap (since we already take care of that in the driver)
>> >>> and removing an if checking if byte_len is a power of 2 (as we do
>> >>> support models for which it's not true).
>> >>>
>> >>> All other patches affect readability and code structure.
>> >>>
>> >>> Tested with a couple models and different both for device tree and
>> >>> platform data modes.
>> >>
>> >> Is there any available tree with that series applied?
>> >> I would test it on Intel Galileo Gen 2 which has ACPI enumerated AT24
>> >> EEPROM attached.
>> >>
>> >
>> > Yes, it's in my github tree:
>> >
>> >     https://github.com/brgl/linux    topic/at24/refactoring
>> >
>> > Thanks in advance for testing it!
>>
>> At least this didn't break AT24 on Intel Galileo Gen 2 board in ACPI mode.
>>
>> Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> All applied except for patch 4.
>
> thanks,
>
> greg k-h

Hi Greg,

I maintain this driver now and my pull requests usually go through
Wolfram's i2c tree (Cc'ed). I initially intended for this series to be
applied for 4.18. For 4.17 luckily the only other changes for at24 are
device tree bindings, so I guess we can keep it in misc-char, since
there are no conflicts.

Let me resend the corrected patch 4.

Best regards,
Bartosz Golaszewski