| Message ID | 1552985370-13598-1-git-send-email-tklauser@distanz.ch (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Delegated to: | Andy Shevchenko |
| Headers | show |
| Series | x86: apuv2: fix spelling in comment | expand |
On Tue, Mar 19, 2019 at 10:57 AM Tobias Klauser <tklauser@distanz.ch> wrote: > > s/maainline/mainline/ > AFAIR it's went through other tree, so, please include the respective maintainers and mailing lists. Enrico, can you fix the driver to provide proper name of LEDs? We have already driver for LEDs in upstream for two years, Sorry, I have missed that. Perhaps you may remove the old driver. > Signed-off-by: Tobias Klauser <tklauser@distanz.ch> > --- > drivers/platform/x86/pcengines-apuv2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c > index c1ca931e1fab..98e2dd28e0a5 100644 > --- a/drivers/platform/x86/pcengines-apuv2.c > +++ b/drivers/platform/x86/pcengines-apuv2.c > @@ -141,7 +141,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = { > }, > .driver_data = (void *)&board_apu2, > }, > - /* APU2 w/ maainline bios */ > + /* APU2 w/ mainline bios */ > { > .ident = "apu2", > .matches = { > -- > 2.20.0 >
On 05.04.19 16:18, Andy Shevchenko wrote: > On Tue, Mar 19, 2019 at 10:57 AM Tobias Klauser <tklauser@distanz.ch> wrote: >> >> s/maainline/mainline/ >> > > AFAIR it's went through other tree, so, please include the respective > maintainers and mailing lists. IIRC it went through gpio tree. @Tobias: please use ./scripts/get_maintainer.pl to fetch the maintainers for certain files / files affected by a patch > Enrico, can you fix the driver to provide proper name of LEDs? We have > already driver for LEDs in upstream for two years, Sorry, I have > missed that. hmm, I could do that ... (with a bit reluctance, as I've already got devices with my naming scheme in the field :o). > Perhaps you may remove the old driver. Ok. But we should find a proper way to inform potential remaining users. --mtx
On 2019-04-08 at 22:34:00 +0200, Enrico Weigelt, metux IT consult <lkml@metux.net> wrote: > On 05.04.19 16:18, Andy Shevchenko wrote: > > On Tue, Mar 19, 2019 at 10:57 AM Tobias Klauser <tklauser@distanz.ch> wrote: > >> > >> s/maainline/mainline/ > >> > > > > AFAIR it's went through other tree, so, please include the respective > > maintainers and mailing lists. > > IIRC it went through gpio tree. > > @Tobias: please use ./scripts/get_maintainer.pl to fetch the maintainers > for certain files / files affected by a patch That's exactly what I did FWIW.
Cc: LED ML and maintainers On Mon, Apr 8, 2019 at 11:34 PM Enrico Weigelt, metux IT consult <lkml@metux.net> wrote: > On 05.04.19 16:18, Andy Shevchenko wrote: > > On Tue, Mar 19, 2019 at 10:57 AM Tobias Klauser <tklauser@distanz.ch> wrote: > > Enrico, can you fix the driver to provide proper name of LEDs? We have > > already driver for LEDs in upstream for two years, Sorry, I have > > missed that. > > hmm, I could do that ... (with a bit reluctance, as I've already got > devices with my naming scheme in the field :o). > > > Perhaps you may remove the old driver. > > Ok. But we should find a proper way to inform potential remaining users. Pavel, Jacek, we got a duplicate driver for some devices here, the old one was submited couple of years ago and went to upstream thru LED subsystem. What would be your recommendation on this case?
On 4/9/19 10:39 AM, Andy Shevchenko wrote: > Cc: LED ML and maintainers > > On Mon, Apr 8, 2019 at 11:34 PM Enrico Weigelt, metux IT consult > <lkml@metux.net> wrote: >> On 05.04.19 16:18, Andy Shevchenko wrote: >>> On Tue, Mar 19, 2019 at 10:57 AM Tobias Klauser <tklauser@distanz.ch> wrote: > >>> Enrico, can you fix the driver to provide proper name of LEDs? We have >>> already driver for LEDs in upstream for two years, Sorry, I have >>> missed that. >> >> hmm, I could do that ... (with a bit reluctance, as I've already got >> devices with my naming scheme in the field :o). >> >>> Perhaps you may remove the old driver. >> >> Ok. But we should find a proper way to inform potential remaining users. > > Pavel, Jacek, we got a duplicate driver for some devices here, the old > one was submited couple of years ago and went to upstream thru LED > subsystem. > What would be your recommendation on this case? Remove that recent duplicate?
On 09.04.19 20:20, Jacek Anaszewski wrote:
>> Pavel, Jacek, we got a duplicate driver for some devices here, the old>> one was submited couple of years ago and went to upstream thru LED>>
subsystem.>> What would be your recommendation on this case?> > Remove
that recent duplicate?
It's not duplicate, it's more complete.
The old driver is just for LEDs on apu board, and blocks all the others
pins, eg. front key.
OTOH, the new drivers are:
#1: generic gpio driver for the amd-fch (at least generic for all
G-112* boards, probably for lots of other SoCs w/ the same FCH)
#2: minimal platform driver that wires up gpios to led and input
subsystems, so it provides leds as well as front key.
They now provide full support for the APU2/3 front peripherals,
LEDs as well as front key. The key cannot be used w/ the old led-only
driver.
--mtx
On 4/12/19 5:14 PM, Enrico Weigelt, metux IT consult wrote: > On 09.04.19 20:20, Jacek Anaszewski wrote: >>> Pavel, Jacek, we got a duplicate driver for some devices here, the old>> one was submited couple of years ago and went to upstream thru LED>> > subsystem.>> What would be your recommendation on this case?> > Remove > that recent duplicate? > It's not duplicate, it's more complete. > > The old driver is just for LEDs on apu board, and blocks all the others > pins, eg. front key. > > OTOH, the new drivers are: > > #1: generic gpio driver for the amd-fch (at least generic for all > G-112* boards, probably for lots of other SoCs w/ the same FCH) > #2: minimal platform driver that wires up gpios to led and input > subsystems, so it provides leds as well as front key. > > They now provide full support for the APU2/3 front peripherals, > LEDs as well as front key. The key cannot be used w/ the old led-only > driver. Then I propose to add missing functionalities to the LED subsystem driver.
On 12.04.19 20:20, Jacek Anaszewski wrote: >> They now provide full support for the APU2/3 front peripherals, >> LEDs as well as front key. The key cannot be used w/ the old led-only >> driver. > > Then I propose to add missing functionalities to the LED subsystem > driver. So, the led driver should also register an generic gpio, and an input device as well - and so duplicate code that naturally belongs into different subsystems ? Maybe I should be a bit more clear on the situation here: * the AMD G-series SoCs have an integrated gpio controller, which is used in various ways, depending on the actual board. * apu2/apu3 is one of many boards using G-series SoCs. * on apu2/apu3 the gpio's are used for different things, eg. leds, keys, simcard slots, etc. According to the LDM, the gpio controller is one device, and the other things, eg. leds or keys, are their own devices, that just happen to be connected to the gpio controller. That's also the usual way we do it in embedded world: we don't write specific-purpose drivers for each single board, but instead generic ones that are just configured for in the individual board's oftree. Unfotunately, we don't have the luxury of oftree on x86, nor automatic overlays on dmi-basis, yet - that's why the platform driver. OTOH, if you wanna rewrite the old leds-only driver to support all the other functions, you'd end up w/ pretty much a complete rewrite: at least you'd have to factor-out the gpio stuff, so it can be called from separate sub-drivers, and then write several sub-drivers based on that. --mtx
On 4/15/19 1:12 PM, Enrico Weigelt, metux IT consult wrote: > On 12.04.19 20:20, Jacek Anaszewski wrote: > >>> They now provide full support for the APU2/3 front peripherals, >>> LEDs as well as front key. The key cannot be used w/ the old led-only >>> driver. >> >> Then I propose to add missing functionalities to the LED subsystem >> driver. > > So, the led driver should also register an generic gpio, and an > input device as well - and so duplicate code that naturally belongs > into different subsystems ? > > Maybe I should be a bit more clear on the situation here: > > * the AMD G-series SoCs have an integrated gpio controller, which is > used in various ways, depending on the actual board. > * apu2/apu3 is one of many boards using G-series SoCs. > * on apu2/apu3 the gpio's are used for different things, eg. leds, > keys, simcard slots, etc. > > According to the LDM, the gpio controller is one device, and the other > things, eg. leds or keys, are their own devices, that just happen to > be connected to the gpio controller. > > That's also the usual way we do it in embedded world: we don't write > specific-purpose drivers for each single board, but instead generic > ones that are just configured for in the individual board's oftree. > Unfotunately, we don't have the luxury of oftree on x86, nor automatic > overlays on dmi-basis, yet - that's why the platform driver. > > > OTOH, if you wanna rewrite the old leds-only driver to support all the > other functions, you'd end up w/ pretty much a complete rewrite: at > least you'd have to factor-out the gpio stuff, so it can be called from > separate sub-drivers, and then write several sub-drivers based on that. I have no problem with doing it the other way round, i.e. moving all functionality provided by the LED subsystem driver to the x86 platform one. Just make sure that from userspace perspective nothing will change.
On 4/15/19 1:12 PM, Enrico Weigelt, metux IT consult wrote: > On 12.04.19 20:20, Jacek Anaszewski wrote: > >>> They now provide full support for the APU2/3 front peripherals, >>> LEDs as well as front key. The key cannot be used w/ the old led-only >>> driver. >> >> Then I propose to add missing functionalities to the LED subsystem >> driver. > > So, the led driver should also register an generic gpio, and an > input device as well - and so duplicate code that naturally belongs > into different subsystems ? > > Maybe I should be a bit more clear on the situation here: > > * the AMD G-series SoCs have an integrated gpio controller, which is > used in various ways, depending on the actual board. > * apu2/apu3 is one of many boards using G-series SoCs. > * on apu2/apu3 the gpio's are used for different things, eg. leds, > keys, simcard slots, etc. > > According to the LDM, the gpio controller is one device, and the other > things, eg. leds or keys, are their own devices, that just happen to > be connected to the gpio controller. > > That's also the usual way we do it in embedded world: we don't write > specific-purpose drivers for each single board, but instead generic > ones that are just configured for in the individual board's oftree. > Unfotunately, we don't have the luxury of oftree on x86, nor automatic > overlays on dmi-basis, yet - that's why the platform driver. > > > OTOH, if you wanna rewrite the old leds-only driver to support all the > other functions, you'd end up w/ pretty much a complete rewrite: at > least you'd have to factor-out the gpio stuff, so it can be called from > separate sub-drivers, and then write several sub-drivers based on that. OK, I have no problem with moving the support provided by the LED subsystem driver to the x86 platform one. Just make sure that from userspace perspective nothing will change.
On Mon 2019-04-15 13:12:59, Enrico Weigelt, metux IT consult wrote: > On 12.04.19 20:20, Jacek Anaszewski wrote: > > >> They now provide full support for the APU2/3 front peripherals, > >> LEDs as well as front key. The key cannot be used w/ the old led-only > >> driver. > > > > Then I propose to add missing functionalities to the LED subsystem > > driver. > > So, the led driver should also register an generic gpio, and an > input device as well - and so duplicate code that naturally belongs > into different subsystems ? No, other way around. > Maybe I should be a bit more clear on the situation here: > > * the AMD G-series SoCs have an integrated gpio controller, which is > used in various ways, depending on the actual board. > * apu2/apu3 is one of many boards using G-series SoCs. > * on apu2/apu3 the gpio's are used for different things, eg. leds, > keys, simcard slots, etc. > > According to the LDM, the gpio controller is one device, and the other > things, eg. leds or keys, are their own devices, that just happen to > be connected to the gpio controller. Yup. > That's also the usual way we do it in embedded world: we don't write > specific-purpose drivers for each single board, but instead generic > ones that are just configured for in the individual board's oftree. > Unfotunately, we don't have the luxury of oftree on x86, nor automatic > overlays on dmi-basis, yet - that's why the platform driver. Well, usualy vendor mask these details using ACPI BIOS on x86. But yes, device tree would make sense for this. Pavel
On Mon, Apr 15, 2019 at 2:13 PM Enrico Weigelt, metux IT consult <lkml@metux.net> wrote: > On 12.04.19 20:20, Jacek Anaszewski wrote: > That's also the usual way we do it in embedded world: we don't write > specific-purpose drivers for each single board, but instead generic > ones that are just configured for in the individual board's oftree. > Unfotunately, we don't have the luxury of oftree on x86, nor automatic > overlays on dmi-basis, yet - that's why the platform driver. You may utilize swnode API instead and have a generic driver beneath which resource provider agnostic (via device property / fwnode API).
On 15.04.19 22:20, Andy Shevchenko wrote:
>> That's also the usual way we do it in embedded world: we don't write>> specific-purpose drivers for each single board, but instead generic>>
ones that are just configured for in the individual board's oftree.>>
Unfotunately, we don't have the luxury of oftree on x86, nor automatic>>
overlays on dmi-basis, yet - that's why the platform driver.> > You may
utilize swnode API instead and have a generic driver beneath> which
resource provider agnostic (via device property / fwnode API).
Well, when we add oftree probing to the driver, then using fwnode api
for that would seems to make sense. But for now, I haven't seen a single
board with that SoC that uses either oftree, or has proper acpi tables.
For the apu2/3 I don't see anything like that on the horizon - and here
it would only help us, if all existing devices would get a fw upgrade.
I'm not even sure, whether the whole thing can be expressed via ACPI
tables in the way we need it.
Remember, we have several layers here:
a) the gpio device within the SoC (base address, gpio registers - they
are NOT linear, ...)
b) assignment between invididual gpio's to the functional (virtual)
devices - leds, keys, rfkill, ...
I'm really not up to date on recent acpi specs, but gpio entries only
(assuming the fw in the field actually supports it) won't be sufficient.
We'd need to express leds, keys, etc _connected_ to gpios.
--mtx
On Tue, Apr 16, 2019 at 7:36 PM Enrico Weigelt, metux IT consult <lkml@metux.net> wrote: > On 15.04.19 22:20, Andy Shevchenko wrote: ... [something wrong with text formatting in your MUA] > > You may utilize swnode API instead and have a generic driver beneath which resource provider agnostic (via device property / fwnode API). > Well, when we add oftree probing to the driver, then using fwnode api oftree -> unified device property API (if we are talking about driver) fwnode API -> swnode API (if we are talking about platform code) > for that would seems to make sense. But for now, I haven't seen a single > board with that SoC that uses either oftree, or has proper acpi tables. Make them. swnode API and preparing structures allows you to mock up the thing and test. > For the apu2/3 I don't see anything like that on the horizon - and here > it would only help us, if all existing devices would get a fw upgrade. > I'm not even sure, whether the whole thing can be expressed via ACPI > tables in the way we need it. It may. Just Do It. > Remember, we have several layers here: > > a) the gpio device within the SoC (base address, gpio registers - they > are NOT linear, ...) > b) assignment between invididual gpio's to the functional (virtual) > devices - leds, keys, rfkill, ... > > I'm really not up to date on recent acpi specs, but gpio entries only > (assuming the fw in the field actually supports it) won't be sufficient. > We'd need to express leds, keys, etc _connected_ to gpios. I do not see any issue here. Have you got familiar with meta-acpi [1] repository? There are plenty examples how to describe DT enabled drivers in ACPI tables, including gpio-keys, LEDs [2], and so on. [1]: https://github.com/westeri/meta-acpi [2]: https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/leds.asli
diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c index c1ca931e1fab..98e2dd28e0a5 100644 --- a/drivers/platform/x86/pcengines-apuv2.c +++ b/drivers/platform/x86/pcengines-apuv2.c @@ -141,7 +141,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = { }, .driver_data = (void *)&board_apu2, }, - /* APU2 w/ maainline bios */ + /* APU2 w/ mainline bios */ { .ident = "apu2", .matches = {
s/maainline/mainline/ Signed-off-by: Tobias Klauser <tklauser@distanz.ch> --- drivers/platform/x86/pcengines-apuv2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)