Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Randy Dunlap]

On 4/29/19 2:03 AM, Stephen Rothwell wrote:
> Hi all,
> 
> Changes since 20190426:
> 

on i386:

when CONFIG_LEDS_TURRIS_OMNIA=y and CONFIG_I2C=m:

Probably should also depend on I2C.


ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_remove':
leds-turris-omnia.c:(.text+0xb): undefined reference to `i2c_smbus_write_byte_data'
ld: drivers/leds/leds-turris-omnia.o: in function `omnia_led_brightness_set_blocking':
leds-turris-omnia.c:(.text+0x62): undefined reference to `i2c_smbus_write_byte_data'
ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_probe':
leds-turris-omnia.c:(.text+0x1d8): undefined reference to `i2c_smbus_write_byte_data'
ld: leds-turris-omnia.c:(.text+0x1f6): undefined reference to `i2c_smbus_write_byte_data'
ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_driver_init':
leds-turris-omnia.c:(.init.text+0x8): undefined reference to `i2c_register_driver'
ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_driver_exit':
leds-turris-omnia.c:(.exit.text+0x6): undefined reference to `i2c_del_driver'



-- 
~Randy

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

On Mon 2019-04-29 08:03:02, Randy Dunlap wrote:
> On 4/29/19 2:03 AM, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Changes since 20190426:
> > 
> 
> on i386:
> 
> when CONFIG_LEDS_TURRIS_OMNIA=y and CONFIG_I2C=m:
> 
> Probably should also depend on I2C.
> 
> 
> ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_remove':
> leds-turris-omnia.c:(.text+0xb): undefined reference to `i2c_smbus_write_byte_data'
> ld: drivers/leds/leds-turris-omnia.o: in function `omnia_led_brightness_set_blocking':
> leds-turris-omnia.c:(.text+0x62): undefined reference to `i2c_smbus_write_byte_data'
> ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_probe':
> leds-turris-omnia.c:(.text+0x1d8): undefined reference to `i2c_smbus_write_byte_data'
> ld: leds-turris-omnia.c:(.text+0x1f6): undefined reference to `i2c_smbus_write_byte_data'
> ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_driver_init':
> leds-turris-omnia.c:(.init.text+0x8): undefined reference to `i2c_register_driver'
> ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_driver_exit':
> leds-turris-omnia.c:(.exit.text+0x6): undefined reference to `i2c_del_driver'

Ack.

It can also depend on ARM or even specific ARM soc... unless we are
compile testing. This is for specific router...

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Marek Behun]

I am sending patch only adding the I2C dep. Theoretically it is
possible that someone uses the same I2C API in their microcontroller on
another architecture.

On Mon, 29 Apr 2019 17:32:00 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> On Mon 2019-04-29 08:03:02, Randy Dunlap wrote:
> > On 4/29/19 2:03 AM, Stephen Rothwell wrote:  
> > > Hi all,
> > > 
> > > Changes since 20190426:
> > >   
> > 
> > on i386:
> > 
> > when CONFIG_LEDS_TURRIS_OMNIA=y and CONFIG_I2C=m:
> > 
> > Probably should also depend on I2C.
> > 
> > 
> > ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_remove':
> > leds-turris-omnia.c:(.text+0xb): undefined reference to `i2c_smbus_write_byte_data'
> > ld: drivers/leds/leds-turris-omnia.o: in function `omnia_led_brightness_set_blocking':
> > leds-turris-omnia.c:(.text+0x62): undefined reference to `i2c_smbus_write_byte_data'
> > ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_probe':
> > leds-turris-omnia.c:(.text+0x1d8): undefined reference to `i2c_smbus_write_byte_data'
> > ld: leds-turris-omnia.c:(.text+0x1f6): undefined reference to `i2c_smbus_write_byte_data'
> > ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_driver_init':
> > leds-turris-omnia.c:(.init.text+0x8): undefined reference to `i2c_register_driver'
> > ld: drivers/leds/leds-turris-omnia.o: in function `omnia_leds_driver_exit':
> > leds-turris-omnia.c:(.exit.text+0x6): undefined reference to `i2c_del_driver'  
> 
> Ack.
> 
> It can also depend on ARM or even specific ARM soc... unless we are
> compile testing. This is for specific router...
> 
> 									Pavel

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

On Mon 2019-04-29 17:38:42, Marek Behun wrote:
> I am sending patch only adding the I2C dep. Theoretically it is
> possible that someone uses the same I2C API in their microcontroller on
> another architecture.

Theoretically. But we both now that probability of that is very low,
and that likely driver would need other updates, too... right?

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Marek Behun]

On Mon, 29 Apr 2019 18:37:53 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> On Mon 2019-04-29 17:38:42, Marek Behun wrote:
> > I am sending patch only adding the I2C dep. Theoretically it is
> > possible that someone uses the same I2C API in their microcontroller on
> > another architecture.  
> 
> Theoretically. But we both now that probability of that is very low,
> and that likely driver would need other updates, too... right?
> 
> 								Pavel

What would be the benefit to add ARM dependency? So that distro
compilations don't ship the turris_omnia driver unnecesarily?

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

On Mon 2019-04-29 18:44:39, Marek Behun wrote:
> On Mon, 29 Apr 2019 18:37:53 +0200
> Pavel Machek <pavel@ucw.cz> wrote:
> 
> > On Mon 2019-04-29 17:38:42, Marek Behun wrote:
> > > I am sending patch only adding the I2C dep. Theoretically it is
> > > possible that someone uses the same I2C API in their microcontroller on
> > > another architecture.  
> > 
> > Theoretically. But we both now that probability of that is very low,
> > and that likely driver would need other updates, too... right?
> 
> What would be the benefit to add ARM dependency? So that distro
> compilations don't ship the turris_omnia driver unnecesarily?

That, and so that people are not asked "do you want to enable omnia
LEDs?" when they update their kernel on i386.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Enrico Weigelt, metux IT consult]

On 29.04.19 18:53, Pavel Machek wrote:
>>> Theoretically. But we both now that probability of that is very low,
>>> and that likely driver would need other updates, too... right?
>>
>> What would be the benefit to add ARM dependency? So that distro
>> compilations don't ship the turris_omnia driver unnecesarily?
> 
> That, and so that people are not asked "do you want to enable omnia
> LEDs?" when they update their kernel on i386.

Is that controller only built-in into some SoCs, or also available
as a separate chip ?

--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

On Mon 2019-04-29 19:51:40, Enrico Weigelt, metux IT consult wrote:
> On 29.04.19 18:53, Pavel Machek wrote:
> >>> Theoretically. But we both now that probability of that is very low,
> >>> and that likely driver would need other updates, too... right?
> >>
> >> What would be the benefit to add ARM dependency? So that distro
> >> compilations don't ship the turris_omnia driver unnecesarily?
> > 
> > That, and so that people are not asked "do you want to enable omnia
> > LEDs?" when they update their kernel on i386.
> 
> Is that controller only built-in into some SoCs, or also available
> as a separate chip ?

AFAIU.. separate chip, but runs firmware not likely to be available
outside Turris routers.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Enrico Weigelt, metux IT consult]

On 29.04.19 20:12, Pavel Machek wrote:

>> Is that controller only built-in into some SoCs, or also available
>> as a separate chip ?
> 
> AFAIU.. separate chip, but runs firmware not likely to be available
> outside Turris routers.

hmm, if it's a separate chip, IMHO it should be selectable, so that
anybody who puts that chip on his board can directly use it.

--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: linux-next: Tree for Apr 29 (drivers/leds/leds-turris-omnia)

[Marek Behun]

On Mon, 29 Apr 2019 20:49:59 +0200
"Enrico Weigelt, metux IT consult" <lkml@metux.net> wrote:

> On 29.04.19 20:12, Pavel Machek wrote:
> 
> >> Is that controller only built-in into some SoCs, or also available
> >> as a separate chip ?  
> > 
> > AFAIU.. separate chip, but runs firmware not likely to be available
> > outside Turris routers.  
> 
> hmm, if it's a separate chip, IMHO it should be selectable, so that
> anybody who puts that chip on his board can directly use it.
> 
> --mtx
> 

The chip is a cortex-m3 or something like that. What makes the LEDs
work in this specific way this driver uses is the firmware on the chip,
and that firmware is specific for Omnia.

It is possible that in the future someone makes a I2C controller
compatible with the API the Omnia firmware provides, but very unlikely.

I think it is reasonable to make the driver depend on MACH_ARMADA_38X
and in the unlikely scenario that someone makes a compatible
controller, the dependency can always be removed.

Marek
situation it is possible to