* [PATCH v4] platform/x86: add meraki-mx100 platform driver
@ 2021-08-10 0:40 Chris Blake
2021-08-10 14:42 ` Hans de Goede
2021-08-15 2:34 ` kernel test robot
0 siblings, 2 replies; 4+ messages in thread
From: Chris Blake @ 2021-08-10 0:40 UTC (permalink / raw)
To: platform-driver-x86, linux-gpio, chunkeey, hdegoede, andy.shevchenko
Cc: Chris Blake
This adds platform support for the Cisco Meraki MX100 (Tinkerbell)
network appliance. This sets up the network LEDs and Reset
button.
Depends-on: ef0eea5b151ae ("mfd: lpc_ich: Enable GPIO driver for DH89xxCC")
Co-developed-by: Christian Lamparter <chunkeey@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Signed-off-by: Chris Blake <chrisrblake93@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
Changelog:
v4: Added missing LEDs, better error handling on pdev register
V3: Additional cleanups, formatting changes
V2: Move to using gpiod lookup tables, misc cleanups
V1: Initial Patch
drivers/platform/x86/Kconfig | 13 ++
drivers/platform/x86/Makefile | 3 +
drivers/platform/x86/meraki-mx100.c | 230 ++++++++++++++++++++++++++++
3 files changed, 246 insertions(+)
create mode 100644 drivers/platform/x86/meraki-mx100.c
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7d385c3b2239..8d70176e335f 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -302,6 +302,19 @@ config ASUS_NB_WMI
If you have an ACPI-WMI compatible Asus Notebook, say Y or M
here.
+config MERAKI_MX100
+ tristate "Cisco Meraki MX100 Platform Driver"
+ depends on GPIOLIB
+ depends on GPIO_ICH
+ depends on LEDS_CLASS
+ select LEDS_GPIO
+ help
+ This driver provides support for the front button and LEDs on
+ the Cisco Meraki MX100 (Tinkerbell) 1U appliance.
+
+ To compile this driver as a module, choose M here: the module
+ will be called meraki-mx100.
+
config EEEPC_LAPTOP
tristate "Eee PC Hotkey Driver"
depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 7ee369aab10d..25c5aee1cde7 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -39,6 +39,9 @@ obj-$(CONFIG_ASUS_NB_WMI) += asus-nb-wmi.o
obj-$(CONFIG_EEEPC_LAPTOP) += eeepc-laptop.o
obj-$(CONFIG_EEEPC_WMI) += eeepc-wmi.o
+# Cisco/Meraki
+obj-$(CONFIG_MERAKI_MX100) += meraki-mx100.o
+
# Dell
obj-$(CONFIG_X86_PLATFORM_DRIVERS_DELL) += dell/
diff --git a/drivers/platform/x86/meraki-mx100.c b/drivers/platform/x86/meraki-mx100.c
new file mode 100644
index 000000000000..1235529483cb
--- /dev/null
+++ b/drivers/platform/x86/meraki-mx100.c
@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Cisco Meraki MX100 (Tinkerbell) board platform driver
+ *
+ * Based off of arch/x86/platform/meraki/tink.c from the
+ * Meraki GPL release meraki-firmware-sources-r23-20150601
+ *
+ * Format inspired by platform/x86/pcengines-apuv2.c
+ *
+ * Copyright (C) 2021 Chris Blake <chrisrblake93@gmail.com>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/dmi.h>
+#include <linux/err.h>
+#include <linux/gpio_keys.h>
+#include <linux/gpio/machine.h>
+#include <linux/input.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define TINK_GPIO_DRIVER_NAME "gpio_ich"
+
+/* LEDs */
+static const struct gpio_led tink_leds[] = {
+ {
+ .name = "mx100:green:internet",
+ .default_trigger = "default-on",
+ },
+ {
+ .name = "mx100:green:lan2",
+ },
+ {
+ .name = "mx100:green:lan3",
+ },
+ {
+ .name = "mx100:green:lan4",
+ },
+ {
+ .name = "mx100:green:lan5",
+ },
+ {
+ .name = "mx100:green:lan6",
+ },
+ {
+ .name = "mx100:green:lan7",
+ },
+ {
+ .name = "mx100:green:lan8",
+ },
+ {
+ .name = "mx100:green:lan9",
+ },
+ {
+ .name = "mx100:green:lan10",
+ },
+ {
+ .name = "mx100:green:lan11",
+ },
+ {
+ .name = "mx100:green:ha",
+ },
+ {
+ .name = "mx100:orange:ha",
+ },
+ {
+ .name = "mx100:green:usb",
+ },
+ {
+ .name = "mx100:orange:usb",
+ },
+};
+
+static const struct gpio_led_platform_data tink_leds_pdata = {
+ .num_leds = ARRAY_SIZE(tink_leds),
+ .leds = tink_leds,
+};
+
+static struct gpiod_lookup_table tink_leds_table = {
+ .dev_id = "leds-gpio",
+ .table = {
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 11,
+ NULL, 0, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 18,
+ NULL, 1, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 20,
+ NULL, 2, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 22,
+ NULL, 3, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 23,
+ NULL, 4, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 32,
+ NULL, 5, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 34,
+ NULL, 6, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 35,
+ NULL, 7, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 36,
+ NULL, 8, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 37,
+ NULL, 9, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 48,
+ NULL, 10, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 16,
+ NULL, 11, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 7,
+ NULL, 12, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 21,
+ NULL, 13, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 19,
+ NULL, 14, GPIO_ACTIVE_LOW),
+ {} /* Terminating entry */
+ }
+};
+
+/* Reset Button */
+static struct gpio_keys_button tink_buttons[] = {
+ {
+ .desc = "Reset",
+ .type = EV_KEY,
+ .code = KEY_RESTART,
+ .active_low = 1,
+ .debounce_interval = 100,
+ },
+};
+
+static const struct gpio_keys_platform_data tink_buttons_pdata = {
+ .buttons = tink_buttons,
+ .nbuttons = ARRAY_SIZE(tink_buttons),
+ .poll_interval = 20,
+ .rep = 0,
+ .name = "mx100-keys",
+};
+
+static struct gpiod_lookup_table tink_keys_table = {
+ .dev_id = "gpio-keys-polled",
+ .table = {
+ GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 60,
+ NULL, 0, GPIO_ACTIVE_LOW),
+ {} /* Terminating entry */
+ }
+};
+
+/* Board setup */
+static const struct dmi_system_id tink_systems[] __initconst = {
+ {
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Cisco"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "MX100-HW"),
+ },
+ },
+ {} /* Terminating entry */
+};
+MODULE_DEVICE_TABLE(dmi, tink_systems);
+
+static struct platform_device *tink_leds_pdev;
+static struct platform_device *tink_keys_pdev;
+
+static struct platform_device * __init tink_create_dev(
+ const char *name, const void *pdata, size_t sz)
+{
+ struct platform_device *pdev;
+
+ pdev = platform_device_register_data(NULL,
+ name, PLATFORM_DEVID_NONE, pdata, sz);
+ if (IS_ERR(pdev))
+ pr_err("failed registering %s: %ld\n", name, PTR_ERR(pdev));
+
+ return pdev;
+}
+
+static int __init tink_board_init(void)
+{
+ int ret = 0;
+
+ if (!dmi_first_match(tink_systems))
+ return -ENODEV;
+
+ /*
+ * We need to make sure that GPIO60 isn't set to native mode as is default since it's our
+ * Reset Button. To do this, write to GPIO_USE_SEL2 to have GPIO60 set to GPIO mode.
+ * This is documented on page 1609 of the PCH datasheet, order number 327879-005US
+ */
+ outl(inl(0x530) | BIT(28), 0x530);
+
+ gpiod_add_lookup_table(&tink_leds_table);
+ gpiod_add_lookup_table(&tink_keys_table);
+
+ tink_leds_pdev = tink_create_dev("leds-gpio",
+ &tink_leds_pdata, sizeof(tink_leds_pdata));
+ if (IS_ERR(tink_leds_pdev)) {
+ ret = ERR_PTR(tink_leds_pdev);
+ goto err;
+ }
+
+ tink_keys_pdev = tink_create_dev("gpio-keys-polled",
+ &tink_buttons_pdata, sizeof(tink_buttons_pdata));
+ if (IS_ERR(tink_keys_pdev)) {
+ ret = ERR_PTR(tink_keys_pdev);
+ platform_device_unregister(tink_leds_pdev);
+ goto err;
+ }
+
+ return ret;
+
+err:
+ gpiod_remove_lookup_table(&tink_keys_table);
+ gpiod_remove_lookup_table(&tink_leds_table);
+ return ret;
+}
+module_init(tink_board_init);
+
+static void __exit tink_board_exit(void)
+{
+ platform_device_unregister(tink_keys_pdev);
+ platform_device_unregister(tink_leds_pdev);
+ gpiod_remove_lookup_table(&tink_keys_table);
+ gpiod_remove_lookup_table(&tink_leds_table);
+}
+module_exit(tink_board_exit);
+
+MODULE_AUTHOR("Chris Blake <chrisrblake93@gmail.com>");
+MODULE_DESCRIPTION("Cisco Meraki MX100 Platform Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:meraki-mx100");
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4] platform/x86: add meraki-mx100 platform driver
2021-08-10 0:40 [PATCH v4] platform/x86: add meraki-mx100 platform driver Chris Blake
@ 2021-08-10 14:42 ` Hans de Goede
2021-08-10 14:58 ` Chris
2021-08-15 2:34 ` kernel test robot
1 sibling, 1 reply; 4+ messages in thread
From: Hans de Goede @ 2021-08-10 14:42 UTC (permalink / raw)
To: Chris Blake, platform-driver-x86, linux-gpio, chunkeey, andy.shevchenko
Hi,
On 8/10/21 2:40 AM, Chris Blake wrote:
> This adds platform support for the Cisco Meraki MX100 (Tinkerbell)
> network appliance. This sets up the network LEDs and Reset
> button.
>
> Depends-on: ef0eea5b151ae ("mfd: lpc_ich: Enable GPIO driver for DH89xxCC")
> Co-developed-by: Christian Lamparter <chunkeey@gmail.com>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> Signed-off-by: Chris Blake <chrisrblake93@gmail.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>
> Changelog:
> v4: Added missing LEDs, better error handling on pdev register
Unfortunately there is an error in the better error handling,
see comments inline. Note for me this gives a compiler warning,
next time please check for compiler warnings.
This is trivial to fix though and everything else looks good,
so I've fixed this locally and added the patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
> V3: Additional cleanups, formatting changes
> V2: Move to using gpiod lookup tables, misc cleanups
> V1: Initial Patch
>
> drivers/platform/x86/Kconfig | 13 ++
> drivers/platform/x86/Makefile | 3 +
> drivers/platform/x86/meraki-mx100.c | 230 ++++++++++++++++++++++++++++
> 3 files changed, 246 insertions(+)
> create mode 100644 drivers/platform/x86/meraki-mx100.c
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 7d385c3b2239..8d70176e335f 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -302,6 +302,19 @@ config ASUS_NB_WMI
> If you have an ACPI-WMI compatible Asus Notebook, say Y or M
> here.
>
> +config MERAKI_MX100
> + tristate "Cisco Meraki MX100 Platform Driver"
> + depends on GPIOLIB
> + depends on GPIO_ICH
> + depends on LEDS_CLASS
> + select LEDS_GPIO
> + help
> + This driver provides support for the front button and LEDs on
> + the Cisco Meraki MX100 (Tinkerbell) 1U appliance.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called meraki-mx100.
> +
> config EEEPC_LAPTOP
> tristate "Eee PC Hotkey Driver"
> depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 7ee369aab10d..25c5aee1cde7 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -39,6 +39,9 @@ obj-$(CONFIG_ASUS_NB_WMI) += asus-nb-wmi.o
> obj-$(CONFIG_EEEPC_LAPTOP) += eeepc-laptop.o
> obj-$(CONFIG_EEEPC_WMI) += eeepc-wmi.o
>
> +# Cisco/Meraki
> +obj-$(CONFIG_MERAKI_MX100) += meraki-mx100.o
> +
> # Dell
> obj-$(CONFIG_X86_PLATFORM_DRIVERS_DELL) += dell/
>
> diff --git a/drivers/platform/x86/meraki-mx100.c b/drivers/platform/x86/meraki-mx100.c
> new file mode 100644
> index 000000000000..1235529483cb
> --- /dev/null
> +++ b/drivers/platform/x86/meraki-mx100.c
> @@ -0,0 +1,230 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Cisco Meraki MX100 (Tinkerbell) board platform driver
> + *
> + * Based off of arch/x86/platform/meraki/tink.c from the
> + * Meraki GPL release meraki-firmware-sources-r23-20150601
> + *
> + * Format inspired by platform/x86/pcengines-apuv2.c
> + *
> + * Copyright (C) 2021 Chris Blake <chrisrblake93@gmail.com>
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/dmi.h>
> +#include <linux/err.h>
> +#include <linux/gpio_keys.h>
> +#include <linux/gpio/machine.h>
> +#include <linux/input.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#define TINK_GPIO_DRIVER_NAME "gpio_ich"
> +
> +/* LEDs */
> +static const struct gpio_led tink_leds[] = {
> + {
> + .name = "mx100:green:internet",
> + .default_trigger = "default-on",
> + },
> + {
> + .name = "mx100:green:lan2",
> + },
> + {
> + .name = "mx100:green:lan3",
> + },
> + {
> + .name = "mx100:green:lan4",
> + },
> + {
> + .name = "mx100:green:lan5",
> + },
> + {
> + .name = "mx100:green:lan6",
> + },
> + {
> + .name = "mx100:green:lan7",
> + },
> + {
> + .name = "mx100:green:lan8",
> + },
> + {
> + .name = "mx100:green:lan9",
> + },
> + {
> + .name = "mx100:green:lan10",
> + },
> + {
> + .name = "mx100:green:lan11",
> + },
> + {
> + .name = "mx100:green:ha",
> + },
> + {
> + .name = "mx100:orange:ha",
> + },
> + {
> + .name = "mx100:green:usb",
> + },
> + {
> + .name = "mx100:orange:usb",
> + },
> +};
> +
> +static const struct gpio_led_platform_data tink_leds_pdata = {
> + .num_leds = ARRAY_SIZE(tink_leds),
> + .leds = tink_leds,
> +};
> +
> +static struct gpiod_lookup_table tink_leds_table = {
> + .dev_id = "leds-gpio",
> + .table = {
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 11,
> + NULL, 0, GPIO_ACTIVE_LOW),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 18,
> + NULL, 1, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 20,
> + NULL, 2, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 22,
> + NULL, 3, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 23,
> + NULL, 4, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 32,
> + NULL, 5, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 34,
> + NULL, 6, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 35,
> + NULL, 7, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 36,
> + NULL, 8, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 37,
> + NULL, 9, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 48,
> + NULL, 10, GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 16,
> + NULL, 11, GPIO_ACTIVE_LOW),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 7,
> + NULL, 12, GPIO_ACTIVE_LOW),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 21,
> + NULL, 13, GPIO_ACTIVE_LOW),
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 19,
> + NULL, 14, GPIO_ACTIVE_LOW),
> + {} /* Terminating entry */
> + }
> +};
> +
> +/* Reset Button */
> +static struct gpio_keys_button tink_buttons[] = {
> + {
> + .desc = "Reset",
> + .type = EV_KEY,
> + .code = KEY_RESTART,
> + .active_low = 1,
> + .debounce_interval = 100,
> + },
> +};
> +
> +static const struct gpio_keys_platform_data tink_buttons_pdata = {
> + .buttons = tink_buttons,
> + .nbuttons = ARRAY_SIZE(tink_buttons),
> + .poll_interval = 20,
> + .rep = 0,
> + .name = "mx100-keys",
> +};
> +
> +static struct gpiod_lookup_table tink_keys_table = {
> + .dev_id = "gpio-keys-polled",
> + .table = {
> + GPIO_LOOKUP_IDX(TINK_GPIO_DRIVER_NAME, 60,
> + NULL, 0, GPIO_ACTIVE_LOW),
> + {} /* Terminating entry */
> + }
> +};
> +
> +/* Board setup */
> +static const struct dmi_system_id tink_systems[] __initconst = {
> + {
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Cisco"),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "MX100-HW"),
> + },
> + },
> + {} /* Terminating entry */
> +};
> +MODULE_DEVICE_TABLE(dmi, tink_systems);
> +
> +static struct platform_device *tink_leds_pdev;
> +static struct platform_device *tink_keys_pdev;
> +
> +static struct platform_device * __init tink_create_dev(
> + const char *name, const void *pdata, size_t sz)
> +{
> + struct platform_device *pdev;
> +
> + pdev = platform_device_register_data(NULL,
> + name, PLATFORM_DEVID_NONE, pdata, sz);
> + if (IS_ERR(pdev))
> + pr_err("failed registering %s: %ld\n", name, PTR_ERR(pdev));
> +
> + return pdev;
> +}
> +
> +static int __init tink_board_init(void)
> +{
> + int ret = 0;
> +
> + if (!dmi_first_match(tink_systems))
> + return -ENODEV;
> +
> + /*
> + * We need to make sure that GPIO60 isn't set to native mode as is default since it's our
> + * Reset Button. To do this, write to GPIO_USE_SEL2 to have GPIO60 set to GPIO mode.
> + * This is documented on page 1609 of the PCH datasheet, order number 327879-005US
> + */
> + outl(inl(0x530) | BIT(28), 0x530);
> +
> + gpiod_add_lookup_table(&tink_leds_table);
> + gpiod_add_lookup_table(&tink_keys_table);
> +
> + tink_leds_pdev = tink_create_dev("leds-gpio",
> + &tink_leds_pdata, sizeof(tink_leds_pdata));
> + if (IS_ERR(tink_leds_pdev)) {
> + ret = ERR_PTR(tink_leds_pdev);
This should be PTR_ERR (pointer-to-error).
> + goto err;
> + }
> +
> + tink_keys_pdev = tink_create_dev("gpio-keys-polled",
> + &tink_buttons_pdata, sizeof(tink_buttons_pdata));
> + if (IS_ERR(tink_keys_pdev)) {
> + ret = ERR_PTR(tink_keys_pdev);
idem.
> + platform_device_unregister(tink_leds_pdev);
> + goto err;
> + }
> +
> + return ret;
You can just use "return 0" here.
> +
> +err:
> + gpiod_remove_lookup_table(&tink_keys_table);
> + gpiod_remove_lookup_table(&tink_leds_table);
> + return ret;
> +}
> +module_init(tink_board_init);
> +
> +static void __exit tink_board_exit(void)
> +{
> + platform_device_unregister(tink_keys_pdev);
> + platform_device_unregister(tink_leds_pdev);
> + gpiod_remove_lookup_table(&tink_keys_table);
> + gpiod_remove_lookup_table(&tink_leds_table);
> +}
> +module_exit(tink_board_exit);
> +
> +MODULE_AUTHOR("Chris Blake <chrisrblake93@gmail.com>");
> +MODULE_DESCRIPTION("Cisco Meraki MX100 Platform Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:meraki-mx100");
>
Regards,
Hans
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] platform/x86: add meraki-mx100 platform driver
2021-08-10 14:42 ` Hans de Goede
@ 2021-08-10 14:58 ` Chris
0 siblings, 0 replies; 4+ messages in thread
From: Chris @ 2021-08-10 14:58 UTC (permalink / raw)
To: Hans de Goede
Cc: Platform Driver, open list:GPIO SUBSYSTEM, Christian Lamparter,
Andy Shevchenko
Hello Hans,
Thanks for the explanation and fix-up, it's appreciated! I'll be sure
to do more testing my next go around.
Regards,
Chris B
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] platform/x86: add meraki-mx100 platform driver
2021-08-10 0:40 [PATCH v4] platform/x86: add meraki-mx100 platform driver Chris Blake
2021-08-10 14:42 ` Hans de Goede
@ 2021-08-15 2:34 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-08-15 2:34 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5078 bytes --]
Hi Chris,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on platform-drivers-x86/for-next]
[also build test WARNING on linus/master v5.14-rc5]
[cannot apply to next-20210813]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Chris-Blake/platform-x86-add-meraki-mx100-platform-driver/20210810-084154
base: git://git.infradead.org/linux-platform-drivers-x86.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/06f3007cbc04cda6358255a16fa3dc49a9a8ce3e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Chris-Blake/platform-x86-add-meraki-mx100-platform-driver/20210810-084154
git checkout 06f3007cbc04cda6358255a16fa3dc49a9a8ce3e
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/platform/x86/meraki-mx100.c: In function 'tink_board_init':
>> drivers/platform/x86/meraki-mx100.c:197:17: warning: passing argument 1 of 'ERR_PTR' makes integer from pointer without a cast [-Wint-conversion]
197 | ret = ERR_PTR(tink_leds_pdev);
| ^~~~~~~~~~~~~~
| |
| struct platform_device *
In file included from include/linux/kernfs.h:10,
from include/linux/sysfs.h:16,
from include/linux/kobject.h:20,
from include/linux/dmi.h:6,
from drivers/platform/x86/meraki-mx100.c:16:
include/linux/err.h:24:48: note: expected 'long int' but argument is of type 'struct platform_device *'
24 | static inline void * __must_check ERR_PTR(long error)
| ~~~~~^~~~~
>> drivers/platform/x86/meraki-mx100.c:197:7: warning: assignment to 'int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
197 | ret = ERR_PTR(tink_leds_pdev);
| ^
drivers/platform/x86/meraki-mx100.c:204:17: warning: passing argument 1 of 'ERR_PTR' makes integer from pointer without a cast [-Wint-conversion]
204 | ret = ERR_PTR(tink_keys_pdev);
| ^~~~~~~~~~~~~~
| |
| struct platform_device *
In file included from include/linux/kernfs.h:10,
from include/linux/sysfs.h:16,
from include/linux/kobject.h:20,
from include/linux/dmi.h:6,
from drivers/platform/x86/meraki-mx100.c:16:
include/linux/err.h:24:48: note: expected 'long int' but argument is of type 'struct platform_device *'
24 | static inline void * __must_check ERR_PTR(long error)
| ~~~~~^~~~~
drivers/platform/x86/meraki-mx100.c:204:7: warning: assignment to 'int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
204 | ret = ERR_PTR(tink_keys_pdev);
| ^
vim +/ERR_PTR +197 drivers/platform/x86/meraki-mx100.c
176
177 static int __init tink_board_init(void)
178 {
179 int ret = 0;
180
181 if (!dmi_first_match(tink_systems))
182 return -ENODEV;
183
184 /*
185 * We need to make sure that GPIO60 isn't set to native mode as is default since it's our
186 * Reset Button. To do this, write to GPIO_USE_SEL2 to have GPIO60 set to GPIO mode.
187 * This is documented on page 1609 of the PCH datasheet, order number 327879-005US
188 */
189 outl(inl(0x530) | BIT(28), 0x530);
190
191 gpiod_add_lookup_table(&tink_leds_table);
192 gpiod_add_lookup_table(&tink_keys_table);
193
194 tink_leds_pdev = tink_create_dev("leds-gpio",
195 &tink_leds_pdata, sizeof(tink_leds_pdata));
196 if (IS_ERR(tink_leds_pdev)) {
> 197 ret = ERR_PTR(tink_leds_pdev);
198 goto err;
199 }
200
201 tink_keys_pdev = tink_create_dev("gpio-keys-polled",
202 &tink_buttons_pdata, sizeof(tink_buttons_pdata));
203 if (IS_ERR(tink_keys_pdev)) {
204 ret = ERR_PTR(tink_keys_pdev);
205 platform_device_unregister(tink_leds_pdev);
206 goto err;
207 }
208
209 return ret;
210
211 err:
212 gpiod_remove_lookup_table(&tink_keys_table);
213 gpiod_remove_lookup_table(&tink_leds_table);
214 return ret;
215 }
216 module_init(tink_board_init);
217
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 65643 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-15 2:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 0:40 [PATCH v4] platform/x86: add meraki-mx100 platform driver Chris Blake
2021-08-10 14:42 ` Hans de Goede
2021-08-10 14:58 ` Chris
2021-08-15 2:34 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.