Subject: [PATCH 1/6] leds: apu: drop superseeded apu2/3 led support

From: Enrico Weigelt <[email protected]>

This driver only supports gpio-attached LEDs on apu1/2/3 boards,
but neither gpio's themselves, nor other gpio-attached devices
(eg. front button).

For apu2+ a newer, more complete driver exists, based on a generic
driver for the AMD SOCs gpio-controller, supporting LEDs as well
other devices. Therefore these boards don't need legacy support
from this driver anymore.

Both drivers claim the same device, so only one driver may exist
in a system, putting distros in the position to decide between
either apu2+ keys or apu1 led support.

Therefore drop the apu2+ led support from the old driver, solve
the conflict and reduce unnecessary code.

Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/leds/Kconfig | 5 +-
drivers/leds/leds-apu.c | 124 +++---------------------------------------------
2 files changed, 10 insertions(+), 119 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b0fdeef..f7a3dd7 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -74,9 +74,12 @@ config LEDS_APU
depends on LEDS_CLASS
depends on X86 && DMI
help
- This driver makes the PC Engines APU/APU2/APU3 front panel LEDs
+ This driver makes the PC Engines APU1 front panel LEDs
accessible from userspace programs through the LED subsystem.

+ If you're looking for APU2/3, use the pcengines-apu2 driver.
+ (symbol CONFIG_PCENGINES_APU2)
+
To compile this driver as a module, choose M here: the
module will be called leds-apu.

diff --git a/drivers/leds/leds-apu.c b/drivers/leds/leds-apu.c
index 8d42e46..140093a 100644
--- a/drivers/leds/leds-apu.c
+++ b/drivers/leds/leds-apu.c
@@ -47,12 +47,6 @@
#define APU1_NUM_GPIO 3
#define APU1_IOSIZE sizeof(u8)

-#define APU2_FCH_ACPI_MMIO_BASE 0xFED80000
-#define APU2_FCH_GPIO_BASE (APU2_FCH_ACPI_MMIO_BASE + 0x1500)
-#define APU2_GPIO_BIT_WRITE 22
-#define APU2_APU2_NUM_GPIO 4
-#define APU2_IOSIZE sizeof(u32)
-
/* LED access parameters */
struct apu_param {
void __iomem *addr; /* for ioread/iowrite */
@@ -75,7 +69,6 @@ struct apu_led_profile {
/* Supported platform types */
enum apu_led_platform_types {
APU1_LED_PLATFORM,
- APU2_LED_PLATFORM,
};

struct apu_led_pdata {
@@ -96,19 +89,6 @@ struct apu_led_pdata {
{ "apu:green:3", LED_OFF, APU1_FCH_GPIO_BASE + 2 * APU1_IOSIZE },
};

-static const struct apu_led_profile apu2_led_profile[] = {
- { "apu2:green:1", LED_ON, APU2_FCH_GPIO_BASE + 68 * APU2_IOSIZE },
- { "apu2:green:2", LED_OFF, APU2_FCH_GPIO_BASE + 69 * APU2_IOSIZE },
- { "apu2:green:3", LED_OFF, APU2_FCH_GPIO_BASE + 70 * APU2_IOSIZE },
-};
-
-/* Same as apu2_led_profile, but with "3" in the LED names. */
-static const struct apu_led_profile apu3_led_profile[] = {
- { "apu3:green:1", LED_ON, APU2_FCH_GPIO_BASE + 68 * APU2_IOSIZE },
- { "apu3:green:2", LED_OFF, APU2_FCH_GPIO_BASE + 69 * APU2_IOSIZE },
- { "apu3:green:3", LED_OFF, APU2_FCH_GPIO_BASE + 70 * APU2_IOSIZE },
-};
-
static const struct dmi_system_id apu_led_dmi_table[] __initconst = {
{
.ident = "apu",
@@ -117,54 +97,6 @@ struct apu_led_pdata {
DMI_MATCH(DMI_PRODUCT_NAME, "APU")
}
},
- /* PC Engines APU2 with "Legacy" bios < 4.0.8 */
- {
- .ident = "apu2",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
- DMI_MATCH(DMI_BOARD_NAME, "APU2")
- }
- },
- /* PC Engines APU2 with "Legacy" bios >= 4.0.8 */
- {
- .ident = "apu2",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
- DMI_MATCH(DMI_BOARD_NAME, "apu2")
- }
- },
- /* PC Engines APU2 with "Mainline" bios */
- {
- .ident = "apu2",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
- DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu2")
- }
- },
- /* PC Engines APU3 with "Legacy" bios < 4.0.8 */
- {
- .ident = "apu3",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
- DMI_MATCH(DMI_BOARD_NAME, "APU3")
- }
- },
- /* PC Engines APU3 with "Legacy" bios >= 4.0.8 */
- {
- .ident = "apu3",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
- DMI_MATCH(DMI_BOARD_NAME, "apu3")
- }
- },
- /* PC Engines APU2 with "Mainline" bios */
- {
- .ident = "apu3",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
- DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu3")
- }
- },
{}
};
MODULE_DEVICE_TABLE(dmi, apu_led_dmi_table);
@@ -178,25 +110,6 @@ static void apu1_led_brightness_set(struct led_classdev *led, enum led_brightnes
spin_unlock(&apu_led->lock);
}

-static void apu2_led_brightness_set(struct led_classdev *led, enum led_brightness value)
-{
- struct apu_led_priv *pled = cdev_to_priv(led);
- u32 value_new;
-
- spin_lock(&apu_led->lock);
-
- value_new = ioread32(pled->param.addr);
-
- if (value)
- value_new &= ~BIT(APU2_GPIO_BIT_WRITE);
- else
- value_new |= BIT(APU2_GPIO_BIT_WRITE);
-
- iowrite32(value_new, pled->param.addr);
-
- spin_unlock(&apu_led->lock);
-}
-
static int apu_led_config(struct device *dev, struct apu_led_pdata *apuld)
{
int i;
@@ -219,8 +132,6 @@ static int apu_led_config(struct device *dev, struct apu_led_pdata *apuld)
led_cdev->flags = LED_CORE_SUSPENDRESUME;
if (apu_led->platform == APU1_LED_PLATFORM)
led_cdev->brightness_set = apu1_led_brightness_set;
- else if (apu_led->platform == APU2_LED_PLATFORM)
- led_cdev->brightness_set = apu2_led_brightness_set;

pled->param.addr = devm_ioremap(dev,
apu_led->profile[i].offset, apu_led->iosize);
@@ -254,27 +165,10 @@ static int __init apu_led_probe(struct platform_device *pdev)

apu_led->pdev = pdev;

- if (dmi_match(DMI_PRODUCT_NAME, "APU")) {
- apu_led->profile = apu1_led_profile;
- apu_led->platform = APU1_LED_PLATFORM;
- apu_led->num_led_instances = ARRAY_SIZE(apu1_led_profile);
- apu_led->iosize = APU1_IOSIZE;
- } else if (dmi_match(DMI_BOARD_NAME, "APU2") ||
- dmi_match(DMI_BOARD_NAME, "apu2") ||
- dmi_match(DMI_BOARD_NAME, "PC Engines apu2")) {
- apu_led->profile = apu2_led_profile;
- apu_led->platform = APU2_LED_PLATFORM;
- apu_led->num_led_instances = ARRAY_SIZE(apu2_led_profile);
- apu_led->iosize = APU2_IOSIZE;
- } else if (dmi_match(DMI_BOARD_NAME, "APU3") ||
- dmi_match(DMI_BOARD_NAME, "apu3") ||
- dmi_match(DMI_BOARD_NAME, "PC Engines apu3")) {
- apu_led->profile = apu3_led_profile;
- /* Otherwise identical to APU2. */
- apu_led->platform = APU2_LED_PLATFORM;
- apu_led->num_led_instances = ARRAY_SIZE(apu3_led_profile);
- apu_led->iosize = APU2_IOSIZE;
- }
+ apu_led->profile = apu1_led_profile;
+ apu_led->platform = APU1_LED_PLATFORM;
+ apu_led->num_led_instances = ARRAY_SIZE(apu1_led_profile);
+ apu_led->iosize = APU1_IOSIZE;

spin_lock_init(&apu_led->lock);
return apu_led_config(&pdev->dev, apu_led);
@@ -295,13 +189,7 @@ static int __init apu_led_init(void)
pr_err("No PC Engines board detected\n");
return -ENODEV;
}
- if (!(dmi_match(DMI_PRODUCT_NAME, "APU") ||
- dmi_match(DMI_PRODUCT_NAME, "APU2") ||
- dmi_match(DMI_PRODUCT_NAME, "apu2") ||
- dmi_match(DMI_PRODUCT_NAME, "PC Engines apu2") ||
- dmi_match(DMI_PRODUCT_NAME, "APU3") ||
- dmi_match(DMI_PRODUCT_NAME, "apu3") ||
- dmi_match(DMI_PRODUCT_NAME, "PC Engines apu3"))) {
+ if (!(dmi_match(DMI_PRODUCT_NAME, "APU"))) {
pr_err("Unknown PC Engines board: %s\n",
dmi_get_system_info(DMI_PRODUCT_NAME));
return -ENODEV;
@@ -337,6 +225,6 @@ static void __exit apu_led_exit(void)
module_exit(apu_led_exit);

MODULE_AUTHOR("Alan Mizrahi");
-MODULE_DESCRIPTION("PC Engines APU family LED driver");
+MODULE_DESCRIPTION("PC Engines APU1 front LED driver");
MODULE_LICENSE("GPL v2");
MODULE_ALIAS("platform:leds_apu");
--
1.9.1


2019-07-16 19:31:18

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 1/6] leds: apu: drop superseeded apu2/3 led support

On Mon 2019-07-15 16:57:28, Enrico Weigelt, metux IT consult wrote:
> From: Enrico Weigelt <[email protected]>
>
> This driver only supports gpio-attached LEDs on apu1/2/3 boards,
> but neither gpio's themselves, nor other gpio-attached devices
> (eg. front button).
>
> For apu2+ a newer, more complete driver exists, based on a generic
> driver for the AMD SOCs gpio-controller, supporting LEDs as well
> other devices. Therefore these boards don't need legacy support
> from this driver anymore.
>
> Both drivers claim the same device, so only one driver may exist
> in a system, putting distros in the position to decide between
> either apu2+ keys or apu1 led support.
>
> Therefore drop the apu2+ led support from the old driver, solve
> the conflict and reduce unnecessary code.
>
> Signed-off-by: Enrico Weigelt <[email protected]>
> ---
> drivers/leds/Kconfig | 5 +-
> drivers/leds/leds-apu.c | 124 +++---------------------------------------------
> 2 files changed, 10 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index b0fdeef..f7a3dd7 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -74,9 +74,12 @@ config LEDS_APU
> depends on LEDS_CLASS
> depends on X86 && DMI
> help
> - This driver makes the PC Engines APU/APU2/APU3 front panel LEDs
> + This driver makes the PC Engines APU1 front panel LEDs
> accessible from userspace programs through the LED subsystem.
>
> + If you're looking for APU2/3, use the pcengines-apu2 driver.
> + (symbol CONFIG_PCENGINES_APU2)
> +
> To compile this driver as a module, choose M here: the
> module will be called leds-apu.
>

Ok, so I understand the reasons, but people updating from old kernels
(make oldconfig) will see nothing and their LEDs will stop working.

Can we do something to help them?

Rest is ok.

Acked-by: Pavel Machek <[email protected]>
Pavel

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


Attachments:
(No filename) (2.07 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments
Subject: Re: [PATCH 1/6] leds: apu: drop superseeded apu2/3 led support

On 16.07.19 21:30, Pavel Machek wrote:

Hi,

> Ok, so I understand the reasons, but people updating from old kernels
> (make oldconfig) will see nothing and their LEDs will stop working.
>
> Can we do something to help them?

I could announce that in pcengines forum. There've been several
discussions on this matter, replying there should give the participants
an automatic mail.

So far, I haven't seen any distro that enabled the old driver, so I
guess the actual users all compiled the kernel on their own and should
notice the change early enough.


--mtx

--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287