* [PATCH 1/4] eeprom: at24: don't assign the i2c ID table to at24_driver
2018-04-10 13:12 [PATCH 0/4] eeprom: at24: last bits of the big refactoring Bartosz Golaszewski
@ 2018-04-10 13:12 ` Bartosz Golaszewski
2018-04-11 9:56 ` Peter Rosin
2018-04-10 13:12 ` [PATCH 2/4] eeprom: at24: use devm_nvmem_register() Bartosz Golaszewski
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-04-10 13:12 UTC (permalink / raw)
To: Andy Shevchenko, Peter Rosin, Sven Van Asbroeck
Cc: linux-i2c, linux-kernel, Bartosz Golaszewski
We switched to using probe_new(), so this is no longer used
by i2c core.
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
drivers/misc/eeprom/at24.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 0c125f207aea..2310f26ac4f3 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -737,7 +737,6 @@ static struct i2c_driver at24_driver = {
},
.probe_new = at24_probe,
.remove = at24_remove,
- .id_table = at24_ids,
};
static int __init at24_init(void)
--
2.17.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] eeprom: at24: don't assign the i2c ID table to at24_driver
2018-04-10 13:12 ` [PATCH 1/4] eeprom: at24: don't assign the i2c ID table to at24_driver Bartosz Golaszewski
@ 2018-04-11 9:56 ` Peter Rosin
2018-04-11 10:09 ` Bartosz Golaszewski
0 siblings, 1 reply; 10+ messages in thread
From: Peter Rosin @ 2018-04-11 9:56 UTC (permalink / raw)
To: Bartosz Golaszewski, Andy Shevchenko, Sven Van Asbroeck
Cc: linux-i2c, linux-kernel
On 2018-04-10 15:12, Bartosz Golaszewski wrote:
> We switched to using probe_new(), so this is no longer used
> by i2c core.
It seems to be used in i2c_device_match() ???
This could easily be me not understanding something...
Cheers,
Peter
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> ---
> drivers/misc/eeprom/at24.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 0c125f207aea..2310f26ac4f3 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -737,7 +737,6 @@ static struct i2c_driver at24_driver = {
> },
> .probe_new = at24_probe,
> .remove = at24_remove,
> - .id_table = at24_ids,
> };
>
> static int __init at24_init(void)
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] eeprom: at24: don't assign the i2c ID table to at24_driver
2018-04-11 9:56 ` Peter Rosin
@ 2018-04-11 10:09 ` Bartosz Golaszewski
2018-04-11 10:12 ` Peter Rosin
0 siblings, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-04-11 10:09 UTC (permalink / raw)
To: Peter Rosin
Cc: Andy Shevchenko, Sven Van Asbroeck, linux-i2c, Linux Kernel Mailing List
2018-04-11 11:56 GMT+02:00 Peter Rosin <peda@axentia.se>:
> On 2018-04-10 15:12, Bartosz Golaszewski wrote:
>> We switched to using probe_new(), so this is no longer used
>> by i2c core.
>
> It seems to be used in i2c_device_match() ???
>
> This could easily be me not understanding something...
>
Yes, but i2c core no longer uses the id_table field in struct
i2c_driver. We call i2c_device_match() ourselves instead of letting
i2c core do it and pass the driver data to probe().
Hope that helps,
Bartosz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] eeprom: at24: don't assign the i2c ID table to at24_driver
2018-04-11 10:09 ` Bartosz Golaszewski
@ 2018-04-11 10:12 ` Peter Rosin
2018-04-11 11:14 ` Bartosz Golaszewski
0 siblings, 1 reply; 10+ messages in thread
From: Peter Rosin @ 2018-04-11 10:12 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andy Shevchenko, Sven Van Asbroeck, linux-i2c, Linux Kernel Mailing List
On 2018-04-11 12:09, Bartosz Golaszewski wrote:
> 2018-04-11 11:56 GMT+02:00 Peter Rosin <peda@axentia.se>:
>> On 2018-04-10 15:12, Bartosz Golaszewski wrote:
>>> We switched to using probe_new(), so this is no longer used
>>> by i2c core.
>>
>> It seems to be used in i2c_device_match() ???
>>
>> This could easily be me not understanding something...
>>
>
> Yes, but i2c core no longer uses the id_table field in struct
> i2c_driver. We call i2c_device_match() ourselves instead of letting
> i2c core do it and pass the driver data to probe().
>
> Hope that helps,
> Bartosz
>
But, i2c_device_match is a static function in the core. I think you
are confusing it with i2c_match_id, which is in fact called from
the static i2c_device_match (with is a bus operation).
So, no, it didn't really help...
Cheers,
Peter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] eeprom: at24: don't assign the i2c ID table to at24_driver
2018-04-11 10:12 ` Peter Rosin
@ 2018-04-11 11:14 ` Bartosz Golaszewski
0 siblings, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-04-11 11:14 UTC (permalink / raw)
To: Peter Rosin
Cc: Andy Shevchenko, Sven Van Asbroeck, linux-i2c, Linux Kernel Mailing List
2018-04-11 12:12 GMT+02:00 Peter Rosin <peda@axentia.se>:
> On 2018-04-11 12:09, Bartosz Golaszewski wrote:
>> 2018-04-11 11:56 GMT+02:00 Peter Rosin <peda@axentia.se>:
>>> On 2018-04-10 15:12, Bartosz Golaszewski wrote:
>>>> We switched to using probe_new(), so this is no longer used
>>>> by i2c core.
>>>
>>> It seems to be used in i2c_device_match() ???
>>>
>>> This could easily be me not understanding something...
>>>
>>
>> Yes, but i2c core no longer uses the id_table field in struct
>> i2c_driver. We call i2c_device_match() ourselves instead of letting
>> i2c core do it and pass the driver data to probe().
>>
>> Hope that helps,
>> Bartosz
>>
>
> But, i2c_device_match is a static function in the core. I think you
> are confusing it with i2c_match_id, which is in fact called from
> the static i2c_device_match (with is a bus operation).
>
Right. So probe_new() in i2c no longer has the id parameter and the
users are supposed to call i2c_match_id() themselves but
i2c_device_match() still expects the id_table to be present in struct
i2c_driver. I should have tested it with some board that uses platform
data.
Consider this patch dropped.
Thanks for spotting that,
Bartosz
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] eeprom: at24: use devm_nvmem_register()
2018-04-10 13:12 [PATCH 0/4] eeprom: at24: last bits of the big refactoring Bartosz Golaszewski
2018-04-10 13:12 ` [PATCH 1/4] eeprom: at24: don't assign the i2c ID table to at24_driver Bartosz Golaszewski
@ 2018-04-10 13:12 ` Bartosz Golaszewski
2018-04-10 13:12 ` [PATCH 3/4] eeprom: at24: provide and use a helper for releasing dummy i2c clients Bartosz Golaszewski
2018-04-10 13:12 ` [PATCH 4/4] eeprom: at24: provide a separate routine for creating " Bartosz Golaszewski
3 siblings, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-04-10 13:12 UTC (permalink / raw)
To: Andy Shevchenko, Peter Rosin, Sven Van Asbroeck
Cc: linux-i2c, linux-kernel, Bartosz Golaszewski
We now have a managed variant of nvmem_register(). Use it
in at24_probe().
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
drivers/misc/eeprom/at24.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 2310f26ac4f3..5e56a99435c6 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -685,7 +685,7 @@ static int at24_probe(struct i2c_client *client)
nvmem_config.word_size = 1;
nvmem_config.size = pdata.byte_len;
- at24->nvmem = nvmem_register(&nvmem_config);
+ at24->nvmem = devm_nvmem_register(dev, &nvmem_config);
if (IS_ERR(at24->nvmem)) {
err = PTR_ERR(at24->nvmem);
goto err_clients;
@@ -718,8 +718,6 @@ static int at24_remove(struct i2c_client *client)
at24 = i2c_get_clientdata(client);
- nvmem_unregister(at24->nvmem);
-
for (i = 1; i < at24->num_addresses; i++)
i2c_unregister_device(at24->client[i].client);
--
2.17.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] eeprom: at24: provide and use a helper for releasing dummy i2c clients
2018-04-10 13:12 [PATCH 0/4] eeprom: at24: last bits of the big refactoring Bartosz Golaszewski
2018-04-10 13:12 ` [PATCH 1/4] eeprom: at24: don't assign the i2c ID table to at24_driver Bartosz Golaszewski
2018-04-10 13:12 ` [PATCH 2/4] eeprom: at24: use devm_nvmem_register() Bartosz Golaszewski
@ 2018-04-10 13:12 ` Bartosz Golaszewski
2018-04-10 13:12 ` [PATCH 4/4] eeprom: at24: provide a separate routine for creating " Bartosz Golaszewski
3 siblings, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-04-10 13:12 UTC (permalink / raw)
To: Andy Shevchenko, Peter Rosin, Sven Van Asbroeck
Cc: linux-i2c, linux-kernel, Bartosz Golaszewski
This allows us to drop two opencoded for loops. We also don't need to
check if the i2c client is NULL before calling i2c_unregister_device().
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
drivers/misc/eeprom/at24.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 5e56a99435c6..c57eaea181d8 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -532,6 +532,14 @@ static int at24_get_pdata(struct device *dev, struct at24_platform_data *pdata)
return 0;
}
+static void at24_remove_dummy_clients(struct at24_data *at24)
+{
+ int i;
+
+ for (i = 1; i < at24->num_addresses; i++)
+ i2c_unregister_device(at24->client[i].client);
+}
+
static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len)
{
if (flags & AT24_FLAG_MAC) {
@@ -702,10 +710,7 @@ static int at24_probe(struct i2c_client *client)
return 0;
err_clients:
- for (i = 1; i < num_addresses; i++)
- if (at24->client[i].client)
- i2c_unregister_device(at24->client[i].client);
-
+ at24_remove_dummy_clients(at24);
pm_runtime_disable(dev);
return err;
@@ -714,13 +719,10 @@ static int at24_probe(struct i2c_client *client)
static int at24_remove(struct i2c_client *client)
{
struct at24_data *at24;
- int i;
at24 = i2c_get_clientdata(client);
- for (i = 1; i < at24->num_addresses; i++)
- i2c_unregister_device(at24->client[i].client);
-
+ at24_remove_dummy_clients(at24);
pm_runtime_disable(&client->dev);
pm_runtime_set_suspended(&client->dev);
--
2.17.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] eeprom: at24: provide a separate routine for creating dummy i2c clients
2018-04-10 13:12 [PATCH 0/4] eeprom: at24: last bits of the big refactoring Bartosz Golaszewski
` (2 preceding siblings ...)
2018-04-10 13:12 ` [PATCH 3/4] eeprom: at24: provide and use a helper for releasing dummy i2c clients Bartosz Golaszewski
@ 2018-04-10 13:12 ` Bartosz Golaszewski
2018-04-11 11:44 ` Peter Rosin
3 siblings, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-04-10 13:12 UTC (permalink / raw)
To: Andy Shevchenko, Peter Rosin, Sven Van Asbroeck
Cc: linux-i2c, linux-kernel, Bartosz Golaszewski
Move the code responsible for creating the dummy i2c clients used by
chips taking multiple slave addresses to a separate function.
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
drivers/misc/eeprom/at24.c | 47 ++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index c57eaea181d8..0cf79ca4f1bf 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -540,6 +540,35 @@ static void at24_remove_dummy_clients(struct at24_data *at24)
i2c_unregister_device(at24->client[i].client);
}
+static int at24_make_dummy_client(struct at24_data *at24, unsigned int index,
+ struct regmap_config *regmap_config)
+{
+ struct i2c_client *base_client, *dummy_client;
+ unsigned short int addr;
+ struct regmap *regmap;
+ struct device *dev;
+
+ base_client = at24->client[0].client;
+ dev = &base_client->dev;
+ addr = base_client->addr + index;
+
+ dummy_client = i2c_new_dummy(base_client->adapter,
+ base_client->addr + index);
+ if (!dummy_client) {
+ dev_err(dev, "address 0x%02x unavailable\n", addr);
+ return -EADDRINUSE;
+ }
+
+ regmap = devm_regmap_init_i2c(dummy_client, regmap_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ at24->client[index].client = dummy_client;
+ at24->client[index].regmap = regmap;
+
+ return 0;
+}
+
static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len)
{
if (flags & AT24_FLAG_MAC) {
@@ -645,20 +674,10 @@ static int at24_probe(struct i2c_client *client)
/* use dummy devices for multiple-address chips */
for (i = 1; i < num_addresses; i++) {
- at24->client[i].client = i2c_new_dummy(client->adapter,
- client->addr + i);
- if (!at24->client[i].client) {
- dev_err(dev, "address 0x%02x unavailable\n",
- client->addr + i);
- err = -EADDRINUSE;
- goto err_clients;
- }
- at24->client[i].regmap = devm_regmap_init_i2c(
- at24->client[i].client,
- ®map_config);
- if (IS_ERR(at24->client[i].regmap)) {
- err = PTR_ERR(at24->client[i].regmap);
- goto err_clients;
+ err = at24_make_dummy_client(at24, i, ®map_config);
+ if (err) {
+ at24_remove_dummy_clients(at24);
+ return err;
}
}
--
2.17.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] eeprom: at24: provide a separate routine for creating dummy i2c clients
2018-04-10 13:12 ` [PATCH 4/4] eeprom: at24: provide a separate routine for creating " Bartosz Golaszewski
@ 2018-04-11 11:44 ` Peter Rosin
0 siblings, 0 replies; 10+ messages in thread
From: Peter Rosin @ 2018-04-11 11:44 UTC (permalink / raw)
To: Bartosz Golaszewski, Andy Shevchenko, Sven Van Asbroeck
Cc: linux-i2c, linux-kernel
On 2018-04-10 15:12, Bartosz Golaszewski wrote:
> Move the code responsible for creating the dummy i2c clients used by
> chips taking multiple slave addresses to a separate function.
>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> ---
> drivers/misc/eeprom/at24.c | 47 ++++++++++++++++++++++++++------------
> 1 file changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index c57eaea181d8..0cf79ca4f1bf 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -540,6 +540,35 @@ static void at24_remove_dummy_clients(struct at24_data *at24)
> i2c_unregister_device(at24->client[i].client);
> }
>
> +static int at24_make_dummy_client(struct at24_data *at24, unsigned int index,
> + struct regmap_config *regmap_config)
> +{
> + struct i2c_client *base_client, *dummy_client;
> + unsigned short int addr;
> + struct regmap *regmap;
> + struct device *dev;
> +
> + base_client = at24->client[0].client;
> + dev = &base_client->dev;
> + addr = base_client->addr + index;
> +
> + dummy_client = i2c_new_dummy(base_client->adapter,
> + base_client->addr + index);
> + if (!dummy_client) {
> + dev_err(dev, "address 0x%02x unavailable\n", addr);
> + return -EADDRINUSE;
> + }
> +
> + regmap = devm_regmap_init_i2c(dummy_client, regmap_config);
> + if (IS_ERR(regmap))
AFAICT, if regmap init fails and you return early, dummy_client is leaked.
> + return PTR_ERR(regmap);
> +
> + at24->client[index].client = dummy_client;
> + at24->client[index].regmap = regmap;
> +
> + return 0;
> +}
> +
> static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len)
> {
> if (flags & AT24_FLAG_MAC) {
> @@ -645,20 +674,10 @@ static int at24_probe(struct i2c_client *client)
>
> /* use dummy devices for multiple-address chips */
> for (i = 1; i < num_addresses; i++) {
> - at24->client[i].client = i2c_new_dummy(client->adapter,
> - client->addr + i);
> - if (!at24->client[i].client) {
> - dev_err(dev, "address 0x%02x unavailable\n",
> - client->addr + i);
> - err = -EADDRINUSE;
> - goto err_clients;
> - }
> - at24->client[i].regmap = devm_regmap_init_i2c(
> - at24->client[i].client,
> - ®map_config);
> - if (IS_ERR(at24->client[i].regmap)) {
> - err = PTR_ERR(at24->client[i].regmap);
> - goto err_clients;
> + err = at24_make_dummy_client(at24, i, ®map_config);
> + if (err) {
> + at24_remove_dummy_clients(at24);
> + return err;
> }
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread