* [PATCH] i2c: change the id to let the i2c device work
@ 2012-10-12 2:34 Bo Shen
2012-10-12 4:40 ` Mark Brown
0 siblings, 1 reply; 12+ messages in thread
From: Bo Shen @ 2012-10-12 2:34 UTC (permalink / raw)
To: linux-arm-kernel
As the old method will use platform device id, change the id to
let the i2c device work
Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
arch/arm/mach-at91/at91sam9260_devices.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-at91/at91sam9260_devices.c b/arch/arm/mach-at91/at91sam9260_devices.c
index 5c5966a..aad76b7 100644
--- a/arch/arm/mach-at91/at91sam9260_devices.c
+++ b/arch/arm/mach-at91/at91sam9260_devices.c
@@ -471,7 +471,7 @@ static struct i2c_gpio_platform_data pdata = {
static struct platform_device at91sam9260_twi_device = {
.name = "i2c-gpio",
- .id = -1,
+ .id = 0,
.dev.platform_data = &pdata,
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 2:34 [PATCH] i2c: change the id to let the i2c device work Bo Shen
@ 2012-10-12 4:40 ` Mark Brown
2012-10-12 4:57 ` Bo Shen
0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2012-10-12 4:40 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 12, 2012 at 10:34:18AM +0800, Bo Shen wrote:
> As the old method will use platform device id, change the id to
> let the i2c device work
What are "the old method" and new method? You're not explaining why
this is needed...
> static struct platform_device at91sam9260_twi_device = {
> .name = "i2c-gpio",
> - .id = -1,
> + .id = 0,
> .dev.platform_data = &pdata,
This looks like a regression, if there's only one of a given type of
device in the system it should have no numeric ID in the display name so
having -1 is appropriate.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 4:40 ` Mark Brown
@ 2012-10-12 4:57 ` Bo Shen
2012-10-12 5:14 ` Mark Brown
0 siblings, 1 reply; 12+ messages in thread
From: Bo Shen @ 2012-10-12 4:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mark,
On 10/12/2012 12:40, Mark Brown wrote:
> On Fri, Oct 12, 2012 at 10:34:18AM +0800, Bo Shen wrote:
>> As the old method will use platform device id, change the id to
>> let the i2c device work
>
> What are "the old method" and new method? You're not explaining why
> this is needed...
Maybe use the 'legacy method' will be better (I am not sure). Now, the
Linux kernel is transferring to device tree which doesn't use platform
device id. So, change the id to let the legacy code work.
May you understand.
>
>> static struct platform_device at91sam9260_twi_device = {
>> .name = "i2c-gpio",
>> - .id = -1,
>> + .id = 0,
>> .dev.platform_data = &pdata,
>
> This looks like a regression, if there's only one of a given type of
> device in the system it should have no numeric ID in the display name so
> having -1 is appropriate.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 4:57 ` Bo Shen
@ 2012-10-12 5:14 ` Mark Brown
2012-10-12 5:45 ` Bo Shen
0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2012-10-12 5:14 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 12, 2012 at 12:57:34PM +0800, Bo Shen wrote:
> On 10/12/2012 12:40, Mark Brown wrote:
> >On Fri, Oct 12, 2012 at 10:34:18AM +0800, Bo Shen wrote:
> >>As the old method will use platform device id, change the id to
> >>let the i2c device work
> >What are "the old method" and new method? You're not explaining why
> >this is needed...
> Maybe use the 'legacy method' will be better (I am not sure). Now,
> the Linux kernel is transferring to device tree which doesn't use
> platform device id. So, change the id to let the legacy code work.
> May you understand.
No, this still makes no sense to me. This is clearly a non-DT device
registration, what does DT have to do with anything here? What is the
practical problem you are seeing in your system and how does this help
with it?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 5:14 ` Mark Brown
@ 2012-10-12 5:45 ` Bo Shen
2012-10-12 5:53 ` Mark Brown
0 siblings, 1 reply; 12+ messages in thread
From: Bo Shen @ 2012-10-12 5:45 UTC (permalink / raw)
To: linux-arm-kernel
On 10/12/2012 13:14, Mark Brown wrote:
> On Fri, Oct 12, 2012 at 12:57:34PM +0800, Bo Shen wrote:
>> On 10/12/2012 12:40, Mark Brown wrote:
>>> On Fri, Oct 12, 2012 at 10:34:18AM +0800, Bo Shen wrote:
>
>>>> As the old method will use platform device id, change the id to
>>>> let the i2c device work
>
>>> What are "the old method" and new method? You're not explaining why
>>> this is needed...
>
>> Maybe use the 'legacy method' will be better (I am not sure). Now,
>> the Linux kernel is transferring to device tree which doesn't use
>> platform device id. So, change the id to let the legacy code work.
>
>> May you understand.
>
> No, this still makes no sense to me. This is clearly a non-DT device
> registration, what does DT have to do with anything here? What is the
Yes, this is non-DT device registration.
As the Linux kernel code is transferring to DT which doesn't use
platform device id. however here, modified unconsciously. So correct
this error.
> practical problem you are seeing in your system and how does this help
> with it?
In non-DT kernel, we use platform device id to distinguish between each
device.
For example,
if register i2c device on i2c bus 0, using:
i2c_register_board_info(0, ...)
if register i2c device on i2c bus 1, using:
i2c_register_board_info(1, ...)
So, in this case, if the id = -1, i2c core will dynamically assign a bus
id to this bus when running, the dynamically assigned bus id is unknown
when writing the code. So, when we use i2c_register_board_info(int
busnum, ...) to register device on busnum. we don't know which value to
be use.
So, this patch will fix this issue.
BRs,
Bo Shen
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 5:45 ` Bo Shen
@ 2012-10-12 5:53 ` Mark Brown
2012-10-12 6:42 ` Bo Shen
2012-10-12 7:14 ` Jean Delvare
0 siblings, 2 replies; 12+ messages in thread
From: Mark Brown @ 2012-10-12 5:53 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 12, 2012 at 01:45:48PM +0800, Bo Shen wrote:
> On 10/12/2012 13:14, Mark Brown wrote:
> >No, this still makes no sense to me. This is clearly a non-DT device
> >registration, what does DT have to do with anything here? What is the
> Yes, this is non-DT device registration.
> As the Linux kernel code is transferring to DT which doesn't use
> platform device id. however here, modified unconsciously. So correct
> this error.
What error and what does DT have to do with any of this? DT has no
impact on non-DT systems.
> >practical problem you are seeing in your system and how does this help
> >with it?
> In non-DT kernel, we use platform device id to distinguish between
> each device.
> So, in this case, if the id = -1, i2c core will dynamically assign a
> bus id to this bus when running, the dynamically assigned bus id is
> unknown when writing the code. So, when we use
> i2c_register_board_info(int busnum, ...) to register device on
> busnum. we don't know which value to be use.
The I2C bus number assigned to the controller should be independant of
the platform device ID used to register the device; a better fix if this
is an issue would be to update the i2c-gpio driver to allow a fixed bus
number to be specified in platform data.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 5:53 ` Mark Brown
@ 2012-10-12 6:42 ` Bo Shen
2012-10-12 7:14 ` Jean Delvare
1 sibling, 0 replies; 12+ messages in thread
From: Bo Shen @ 2012-10-12 6:42 UTC (permalink / raw)
To: linux-arm-kernel
On 10/12/2012 13:53, Mark Brown wrote:
> On Fri, Oct 12, 2012 at 01:45:48PM +0800, Bo Shen wrote:
>> On 10/12/2012 13:14, Mark Brown wrote:
>
>>> No, this still makes no sense to me. This is clearly a non-DT device
>>> registration, what does DT have to do with anything here? What is the
>
>> Yes, this is non-DT device registration.
>> As the Linux kernel code is transferring to DT which doesn't use
>> platform device id. however here, modified unconsciously. So correct
>> this error.
>
> What error and what does DT have to do with any of this? DT has no
> impact on non-DT systems.
Yes. This has nothing to do with DT.
>>> practical problem you are seeing in your system and how does this help
>>> with it?
>
>> In non-DT kernel, we use platform device id to distinguish between
>> each device.
>
>> So, in this case, if the id = -1, i2c core will dynamically assign a
>> bus id to this bus when running, the dynamically assigned bus id is
>> unknown when writing the code. So, when we use
>> i2c_register_board_info(int busnum, ...) to register device on
>> busnum. we don't know which value to be use.
>
> The I2C bus number assigned to the controller should be independant of
> the platform device ID used to register the device; a better fix if this
> is an issue would be to update the i2c-gpio driver to allow a fixed bus
> number to be specified in platform data.
Hi Haavard Skinnemoen,
Do you have any suggestions about this?
Thanks.
BRs,
Bo Shen
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 5:53 ` Mark Brown
2012-10-12 6:42 ` Bo Shen
@ 2012-10-12 7:14 ` Jean Delvare
2012-10-12 7:55 ` Bo Shen
1 sibling, 1 reply; 12+ messages in thread
From: Jean Delvare @ 2012-10-12 7:14 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 12 Oct 2012 14:53:02 +0900, Mark Brown wrote:
> On Fri, Oct 12, 2012 at 01:45:48PM +0800, Bo Shen wrote:
> > So, in this case, if the id = -1, i2c core will dynamically assign a
> > bus id to this bus when running, the dynamically assigned bus id is
> > unknown when writing the code. So, when we use
> > i2c_register_board_info(int busnum, ...) to register device on
> > busnum. we don't know which value to be use.
>
> The I2C bus number assigned to the controller should be independant of
> the platform device ID used to register the device; a better fix if this
> is an issue would be to update the i2c-gpio driver to allow a fixed bus
> number to be specified in platform data.
i2c-gpio does support this already.
--
Jean Delvare
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 7:14 ` Jean Delvare
@ 2012-10-12 7:55 ` Bo Shen
2012-10-12 8:05 ` Jean Delvare
0 siblings, 1 reply; 12+ messages in thread
From: Bo Shen @ 2012-10-12 7:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jean Delvare,
On 10/12/2012 15:14, Jean Delvare wrote:
> On Fri, 12 Oct 2012 14:53:02 +0900, Mark Brown wrote:
>> On Fri, Oct 12, 2012 at 01:45:48PM +0800, Bo Shen wrote:
>>> So, in this case, if the id = -1, i2c core will dynamically assign a
>>> bus id to this bus when running, the dynamically assigned bus id is
>>> unknown when writing the code. So, when we use
>>> i2c_register_board_info(int busnum, ...) to register device on
>>> busnum. we don't know which value to be use.
>>
>> The I2C bus number assigned to the controller should be independant of
>> the platform device ID used to register the device; a better fix if this
>> is an issue would be to update the i2c-gpio driver to allow a fixed bus
>> number to be specified in platform data.
>
> i2c-gpio does support this already.
vim I only see the i2c-gpio platform data structure as following:
--<---------------------
struct i2c_gpio_platform_data {
unsigned int sda_pin;
unsigned int scl_pin;
int udelay;
int timeout;
unsigned int sda_is_open_drain:1;
unsigned int scl_is_open_drain:1;
unsigned int scl_is_output_only:1;
};
-->---------------------
So, how to allow a fixed bus number to be specified in platform data?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 7:55 ` Bo Shen
@ 2012-10-12 8:05 ` Jean Delvare
2012-10-12 8:21 ` Mark Brown
0 siblings, 1 reply; 12+ messages in thread
From: Jean Delvare @ 2012-10-12 8:05 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 12 Oct 2012 15:55:32 +0800, Bo Shen wrote:
> vim I only see the i2c-gpio platform data structure as following:
> --<---------------------
> struct i2c_gpio_platform_data {
> unsigned int sda_pin;
> unsigned int scl_pin;
> int udelay;
> int timeout;
> unsigned int sda_is_open_drain:1;
> unsigned int scl_is_open_drain:1;
> unsigned int scl_is_output_only:1;
> };
> -->---------------------
Ah sorry I misread Mark's request. i2c-gpio will turn the platform
device ID into bus number, it can indeed not be forced through platform
data. But I don't think any other i2c bus driver allows this either. I
don't quite see the problem with setting a platform device ID even if
there's only one instance of the platform device. I have many examples
of this on my machine:
Fixed MDIO bus.0
coretemp.0
vesafb.0
So please just set the platform device ID to 0 (or whatever i2c adapter
number you want) and your problem is solved. As you just proposed
initially, actually :)
--
Jean Delvare
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 8:05 ` Jean Delvare
@ 2012-10-12 8:21 ` Mark Brown
2012-10-12 9:02 ` Bo Shen
0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2012-10-12 8:21 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 12, 2012 at 10:05:21AM +0200, Jean Delvare wrote:
> Ah sorry I misread Mark's request. i2c-gpio will turn the platform
> device ID into bus number, it can indeed not be forced through platform
> data. But I don't think any other i2c bus driver allows this either. I
> don't quite see the problem with setting a platform device ID even if
> there's only one instance of the platform device. I have many examples
> of this on my machine:
> Fixed MDIO bus.0
> coretemp.0
> vesafb.0
This is generally bad style; if it's required by APIs we really should
be fixing the APIs to remove this sort of dependency. Aside from the
ugliness it tends to be fragile.
> So please just set the platform device ID to 0 (or whatever i2c adapter
> number you want) and your problem is solved. As you just proposed
> initially, actually :)
Though it *does* need a comprehensible commit message so people can
understand what on earth the change is intended to do.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] i2c: change the id to let the i2c device work
2012-10-12 8:21 ` Mark Brown
@ 2012-10-12 9:02 ` Bo Shen
0 siblings, 0 replies; 12+ messages in thread
From: Bo Shen @ 2012-10-12 9:02 UTC (permalink / raw)
To: linux-arm-kernel
On 10/12/2012 16:21, Mark Brown wrote:
> On Fri, Oct 12, 2012 at 10:05:21AM +0200, Jean Delvare wrote:
>
>> Ah sorry I misread Mark's request. i2c-gpio will turn the platform
>> device ID into bus number, it can indeed not be forced through platform
>> data. But I don't think any other i2c bus driver allows this either. I
>> don't quite see the problem with setting a platform device ID even if
>> there's only one instance of the platform device. I have many examples
>> of this on my machine:
>> Fixed MDIO bus.0
>> coretemp.0
>> vesafb.0
>
> This is generally bad style; if it's required by APIs we really should
> be fixing the APIs to remove this sort of dependency. Aside from the
> ugliness it tends to be fragile.
>
>> So please just set the platform device ID to 0 (or whatever i2c adapter
>> number you want) and your problem is solved. As you just proposed
>> initially, actually :)
>
> Though it *does* need a comprehensible commit message so people can
> understand what on earth the change is intended to do.
I will update the commit message and send v2 patch.
Thanks
BR,
Bo Shen
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-10-12 9:02 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-12 2:34 [PATCH] i2c: change the id to let the i2c device work Bo Shen
2012-10-12 4:40 ` Mark Brown
2012-10-12 4:57 ` Bo Shen
2012-10-12 5:14 ` Mark Brown
2012-10-12 5:45 ` Bo Shen
2012-10-12 5:53 ` Mark Brown
2012-10-12 6:42 ` Bo Shen
2012-10-12 7:14 ` Jean Delvare
2012-10-12 7:55 ` Bo Shen
2012-10-12 8:05 ` Jean Delvare
2012-10-12 8:21 ` Mark Brown
2012-10-12 9:02 ` Bo Shen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).