Re: [PATCH] i2c-eg20t: use dynamically registered adapter number

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2016年08月26日 23:30, Jean Delvare wrote:
Hi Yadi,

On Tue, 23 Aug 2016 17:05:58 +0800, Yadi Hu wrote:
From: Hu Yadi <Yadi.hu@xxxxxxxxxxxxx>

The eg20t driver uses i2c_add_numbered_adapter() to register adapter:

     pch_adap->nr = i;
     ret = i2c_add_numbered_adapter(pch_adap);

Variable i is assigned to 0, it means that i2c_eg20t is the first adapter
by default. if another adapter registers before eg20t, above code return
error for index conflict:

     i2c_eg20t 0000:05:0c.2: pch_i2c_probe :i2c_add_adapter[ch:0] FAILED
     i2c_eg20t: probe of 0000:05:0c.2 failed with error -16

So, we can replace i2c_add_numbered_adapter() with i2c_add_adapter()
interface.since it dynamically allocates the index number.
This does the exact opposite of:

commit 07e8a51ff68353e01d795cceafbac9f54c49132b
Author: Feng Tang <feng.tang@xxxxxxxxx>
Date:   Thu Jan 12 15:38:02 2012 +0800

     i2c-eg20t: use i2c_add_numbered_adapter to get a fixed bus number

So my understanding is that you are not supposed to register another
i2c bus before the eg20t buses. What is the conflicting driver? Is it
creating the buses with static number or not?
I am using one Kontron M2M, on which both i2c-eg20t and i2c-kempld are used simultaneously. since kempld always is firstly initialized and configured to register dynamically, it always occupied the first bus-id. i.e. 0. consequently, eg20t will complain to fail to register for no space for "0" i2c-bus.



Looking at

commit 03bde7c31a360f814ca42101d60563b1b803dca1
Author: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
Date:   Thu Mar 12 17:17:59 2015 +0100

     i2c: busses with dynamic ids should start after fixed ids for DT

it could be that you need to set some OF attribute to reserve i2c bus
numbers <= 1 for static usage. Assuming you use OF. Or is it automatic,
Wolfram?

If not, it may make sense to add a helper function exposing
__i2c_first_dynamic_bus_num to drivers (something like
i2c_is_dynamic_bus_num().) After all, i2c_add_numbered_adapter() mostly
makes sense if static i2c device definitions exist. If not,
i2c_add_adapter() is just as good. So something like:

	if (i2c_is_dynamic_bus_num(i))
		ret = i2c_add_adapter(pch_adap);
	else {
		pch_adap->nr = i;
		ret = i2c_add_numbered_adapter(pch_adap);
	}

may make sense. Unless someone has a better idea.

I totally agree with you and send off V2 patch soon per your suggestion.


Yadi

Signed-off-by: Hu Yadi <Yadi.hu@xxxxxxxxxxxxx>
---
  drivers/i2c/busses/i2c-eg20t.c |    3 +--
  1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c
index 7a51ddc..2f4c2af 100644
--- a/drivers/i2c/busses/i2c-eg20t.c
+++ b/drivers/i2c/busses/i2c-eg20t.c
@@ -913,8 +913,7 @@ static int __devinit pch_i2c_probe(struct pci_dev *pdev,
pch_i2c_init(&adap_info->pch_data[i]); - pch_adap->nr = i;
-		ret = i2c_add_numbered_adapter(pch_adap);
+               ret = i2c_add_adapter(pch_adap);
Coding style is wrong here. Please use tabs, as ./scripts/checkpatch.pl
tells you. Always check your patches with this script before you post
them, thanks.

  		if (ret) {
  			pch_pci_err(pdev, "i2c_add_adapter[ch:%d] FAILED\n", i);
  			goto err_add_adapter;


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux