public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] i2c, core: optimze i2c_set_bus_num()
@ 2013-10-04  5:36 Heiko Schocher
  2013-10-04  8:42 ` Lukasz Majewski
  2013-10-17  6:24 ` Heiko Schocher
  0 siblings, 2 replies; 5+ messages in thread
From: Heiko Schocher @ 2013-10-04  5:36 UTC (permalink / raw)
  To: u-boot

check first, if we are on the bus, we want to enable. If so,
return immediately, do not calc max adapter number, nor check
other things.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Lukasz Majewski <l.majewski@samsung.com>
---
 drivers/i2c/i2c_core.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/i2c_core.c b/drivers/i2c/i2c_core.c
index d1072e8..170423a 100644
--- a/drivers/i2c/i2c_core.c
+++ b/drivers/i2c/i2c_core.c
@@ -278,20 +278,22 @@ unsigned int i2c_get_bus_num(void)
  */
 int i2c_set_bus_num(unsigned int bus)
 {
-	int max = ll_entry_count(struct i2c_adapter, i2c);
+	int max;
+
+	if ((bus == I2C_BUS) && (I2C_ADAP->init_done > 0))
+		return 0;
 
-	if (I2C_ADAPTER(bus) >= max) {
-		printf("Error, wrong i2c adapter %d max %d possible\n",
-		       I2C_ADAPTER(bus), max);
-		return -2;
-	}
 #ifndef CONFIG_SYS_I2C_DIRECT_BUS
 	if (bus >= CONFIG_SYS_NUM_I2C_BUSES)
 		return -1;
 #endif
 
-	if ((bus == I2C_BUS) && (I2C_ADAP->init_done > 0))
-		return 0;
+	max = ll_entry_count(struct i2c_adapter, i2c);
+	if (I2C_ADAPTER(bus) >= max) {
+		printf("Error, wrong i2c adapter %d max %d possible\n",
+		       I2C_ADAPTER(bus), max);
+		return -2;
+	}
 
 #ifndef CONFIG_SYS_I2C_DIRECT_BUS
 	i2c_mux_disconnet_all();
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] i2c, core: optimze i2c_set_bus_num()
  2013-10-04  5:36 [U-Boot] [PATCH] i2c, core: optimze i2c_set_bus_num() Heiko Schocher
@ 2013-10-04  8:42 ` Lukasz Majewski
  2013-10-04  8:59   ` Heiko Schocher
  2013-10-17  6:24 ` Heiko Schocher
  1 sibling, 1 reply; 5+ messages in thread
From: Lukasz Majewski @ 2013-10-04  8:42 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

> check first, if we are on the bus, we want to enable. If so,
> return immediately, do not calc max adapter number, nor check
> other things.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> ---
>  drivers/i2c/i2c_core.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/i2c_core.c b/drivers/i2c/i2c_core.c
> index d1072e8..170423a 100644
> --- a/drivers/i2c/i2c_core.c
> +++ b/drivers/i2c/i2c_core.c
> @@ -278,20 +278,22 @@ unsigned int i2c_get_bus_num(void)
>   */
>  int i2c_set_bus_num(unsigned int bus)
>  {
> -	int max = ll_entry_count(struct i2c_adapter, i2c);
> +	int max;
> +
> +	if ((bus == I2C_BUS) && (I2C_ADAP->init_done > 0))
> +		return 0;
>  
> -	if (I2C_ADAPTER(bus) >= max) {
> -		printf("Error, wrong i2c adapter %d max %d
> possible\n",
> -		       I2C_ADAPTER(bus), max);
> -		return -2;
> -	}
>  #ifndef CONFIG_SYS_I2C_DIRECT_BUS
>  	if (bus >= CONFIG_SYS_NUM_I2C_BUSES)
>  		return -1;
>  #endif
>  
> -	if ((bus == I2C_BUS) && (I2C_ADAP->init_done > 0))
> -		return 0;
> +	max = ll_entry_count(struct i2c_adapter, i2c);
> +	if (I2C_ADAPTER(bus) >= max) {
> +		printf("Error, wrong i2c adapter %d max %d
> possible\n",

Since you are the maintainer of the i2c code, you will decide if
those changes shall be applied (they are really cosmetic) :-).

My suggestion: printf -> error() macro @ common.h

> +		       I2C_ADAPTER(bus), max);
> +		return -2;

I've noticed that i2c_core uses -1/-2 return values for errors
globaly.

So for a new code we could start using defines from errno.h (-2 ->
-ENODEV) ?


> +	}
>  
>  #ifndef CONFIG_SYS_I2C_DIRECT_BUS
>  	i2c_mux_disconnet_all();



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] i2c, core: optimze i2c_set_bus_num()
  2013-10-04  8:42 ` Lukasz Majewski
@ 2013-10-04  8:59   ` Heiko Schocher
  2013-10-04 10:41     ` Lukasz Majewski
  0 siblings, 1 reply; 5+ messages in thread
From: Heiko Schocher @ 2013-10-04  8:59 UTC (permalink / raw)
  To: u-boot

Hello Lukasz,

Am 04.10.2013 10:42, schrieb Lukasz Majewski:
> Hi Heiko,
>
>> check first, if we are on the bus, we want to enable. If so,
>> return immediately, do not calc max adapter number, nor check
>> other things.
>>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> Cc: Lukasz Majewski<l.majewski@samsung.com>
>> ---
>>   drivers/i2c/i2c_core.c | 18 ++++++++++--------
>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c_core.c b/drivers/i2c/i2c_core.c
>> index d1072e8..170423a 100644
>> --- a/drivers/i2c/i2c_core.c
>> +++ b/drivers/i2c/i2c_core.c
>> @@ -278,20 +278,22 @@ unsigned int i2c_get_bus_num(void)
>>    */
>>   int i2c_set_bus_num(unsigned int bus)
>>   {
>> -	int max = ll_entry_count(struct i2c_adapter, i2c);
>> +	int max;
>> +
>> +	if ((bus == I2C_BUS)&&  (I2C_ADAP->init_done>  0))
>> +		return 0;
>>
>> -	if (I2C_ADAPTER(bus)>= max) {
>> -		printf("Error, wrong i2c adapter %d max %d
>> possible\n",
>> -		       I2C_ADAPTER(bus), max);
>> -		return -2;
>> -	}
>>   #ifndef CONFIG_SYS_I2C_DIRECT_BUS
>>   	if (bus>= CONFIG_SYS_NUM_I2C_BUSES)
>>   		return -1;
>>   #endif
>>
>> -	if ((bus == I2C_BUS)&&  (I2C_ADAP->init_done>  0))
>> -		return 0;
>> +	max = ll_entry_count(struct i2c_adapter, i2c);
>> +	if (I2C_ADAPTER(bus)>= max) {
>> +		printf("Error, wrong i2c adapter %d max %d
>> possible\n",
>
> Since you are the maintainer of the i2c code, you will decide if
> those changes shall be applied (they are really cosmetic) :-).

Exactly for this reason a "Acked-by" would not disturb me ;-)

> My suggestion: printf ->  error() macro @ common.h

Yep ... but this should be a seperate patch.

>> +		       I2C_ADAPTER(bus), max);
>> +		return -2;
>
> I've noticed that i2c_core uses -1/-2 return values for errors
> globaly.
>
> So for a new code we could start using defines from errno.h (-2 ->
> -ENODEV) ?

Yes, good point!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] i2c, core: optimze i2c_set_bus_num()
  2013-10-04  8:59   ` Heiko Schocher
@ 2013-10-04 10:41     ` Lukasz Majewski
  0 siblings, 0 replies; 5+ messages in thread
From: Lukasz Majewski @ 2013-10-04 10:41 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

> Hello Lukasz,
> 
> Am 04.10.2013 10:42, schrieb Lukasz Majewski:
> > Hi Heiko,
> >
> >> check first, if we are on the bus, we want to enable. If so,
> >> return immediately, do not calc max adapter number, nor check
> >> other things.
> >>
> >> Signed-off-by: Heiko Schocher<hs@denx.de>
> >> Cc: Lukasz Majewski<l.majewski@samsung.com>
> >> ---
> >>   drivers/i2c/i2c_core.c | 18 ++++++++++--------
> >>   1 file changed, 10 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/i2c/i2c_core.c b/drivers/i2c/i2c_core.c
> >> index d1072e8..170423a 100644
> >> --- a/drivers/i2c/i2c_core.c
> >> +++ b/drivers/i2c/i2c_core.c
> >> @@ -278,20 +278,22 @@ unsigned int i2c_get_bus_num(void)
> >>    */
> >>   int i2c_set_bus_num(unsigned int bus)
> >>   {
> >> -	int max = ll_entry_count(struct i2c_adapter, i2c);
> >> +	int max;
> >> +
> >> +	if ((bus == I2C_BUS)&&  (I2C_ADAP->init_done>  0))
> >> +		return 0;
> >>
> >> -	if (I2C_ADAPTER(bus)>= max) {
> >> -		printf("Error, wrong i2c adapter %d max %d
> >> possible\n",
> >> -		       I2C_ADAPTER(bus), max);
> >> -		return -2;
> >> -	}
> >>   #ifndef CONFIG_SYS_I2C_DIRECT_BUS
> >>   	if (bus>= CONFIG_SYS_NUM_I2C_BUSES)
> >>   		return -1;
> >>   #endif
> >>
> >> -	if ((bus == I2C_BUS)&&  (I2C_ADAP->init_done>  0))
> >> -		return 0;
> >> +	max = ll_entry_count(struct i2c_adapter, i2c);
> >> +	if (I2C_ADAPTER(bus)>= max) {
> >> +		printf("Error, wrong i2c adapter %d max %d
> >> possible\n",
> >
> > Since you are the maintainer of the i2c code, you will decide if
> > those changes shall be applied (they are really cosmetic) :-).
> 
> Exactly for this reason a "Acked-by" would not disturb me ;-)

So as requested :-) :

Acked-by: Lukasz Majewski <l.majewski@samsung.com>

> 
> > My suggestion: printf ->  error() macro @ common.h
> 
> Yep ... but this should be a seperate patch.
> 
> >> +		       I2C_ADAPTER(bus), max);
> >> +		return -2;
> >
> > I've noticed that i2c_core uses -1/-2 return values for errors
> > globaly.
> >
> > So for a new code we could start using defines from errno.h (-2 ->
> > -ENODEV) ?
> 
> Yes, good point!
> 
> bye,
> Heiko



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] i2c, core: optimze i2c_set_bus_num()
  2013-10-04  5:36 [U-Boot] [PATCH] i2c, core: optimze i2c_set_bus_num() Heiko Schocher
  2013-10-04  8:42 ` Lukasz Majewski
@ 2013-10-17  6:24 ` Heiko Schocher
  1 sibling, 0 replies; 5+ messages in thread
From: Heiko Schocher @ 2013-10-17  6:24 UTC (permalink / raw)
  To: u-boot

Hello Heiko,

Am 04.10.2013 07:36, schrieb Heiko Schocher:
> check first, if we are on the bus, we want to enable. If so,
> return immediately, do not calc max adapter number, nor check
> other things.
>
> Signed-off-by: Heiko Schocher<hs@denx.de>
> Cc: Lukasz Majewski<l.majewski@samsung.com>
> ---
>   drivers/i2c/i2c_core.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)

Applied to u-boot-i2c.git

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-10-17  6:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-04  5:36 [U-Boot] [PATCH] i2c, core: optimze i2c_set_bus_num() Heiko Schocher
2013-10-04  8:42 ` Lukasz Majewski
2013-10-04  8:59   ` Heiko Schocher
2013-10-04 10:41     ` Lukasz Majewski
2013-10-17  6:24 ` Heiko Schocher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox