This series removes backlight-related sysfs attributes from the platform
device registered by fujitsu-laptop and does some other cleanups to
platform device code which hopefully make it easier to understand.
drivers/platform/x86/fujitsu-laptop.c | 197 ++++++++++------------------------
1 file changed, 57 insertions(+), 140 deletions(-)
--
2.12.0
Setting backlight level using a vendor-specific interface should only be
possible when using the latter is either explicitly requested by the
user or automatically selected by the kernel. fujitsu-laptop violates
that premise by unconditionally attaching three backlight-related
attributes to the platform device it registers. Remove the offending
attributes from the platform device. Update module comments to reflect
this change.
Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 90 ++---------------------------------
1 file changed, 3 insertions(+), 87 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 3c795d591de0..9b9348af7626 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -32,18 +32,9 @@
* features made available on a range of Fujitsu laptops including the
* P2xxx/P5xxx/S6xxx/S7xxx series.
*
- * This driver exports a few files in /sys/devices/platform/fujitsu-laptop/;
- * others may be added at a later date.
- *
- * lcd_level - Screen brightness: contains a single integer in the
- * range 0..7. (rw)
- *
- * In addition to these platform device attributes the driver
- * registers itself in the Linux backlight control subsystem and is
- * available to userspace under /sys/class/backlight/fujitsu-laptop/.
- *
- * Hotkeys present on certain Fujitsu laptops (eg: the S6xxx series) are
- * also supported by this driver.
+ * This driver implements a vendor-specific backlight control interface for
+ * Fujitsu laptops and provides support for hotkeys present on certain Fujitsu
+ * laptops.
*
* This driver has been tested on a Fujitsu Lifebook S6410, S7020 and
* P8010. It should work on most P-series and S-series Lifebooks, but
@@ -489,74 +480,6 @@ static const struct backlight_ops fujitsu_bl_ops = {
.update_status = bl_update_status,
};
-/* Platform LCD brightness device */
-
-static ssize_t
-show_max_brightness(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
-
- int ret;
-
- ret = get_max_brightness();
- if (ret < 0)
- return ret;
-
- return sprintf(buf, "%i\n", ret);
-}
-
-static ssize_t
-show_brightness_changed(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
-
- int ret;
-
- ret = fujitsu_bl->brightness_changed;
- if (ret < 0)
- return ret;
-
- return sprintf(buf, "%i\n", ret);
-}
-
-static ssize_t show_lcd_level(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
-
- int ret;
-
- ret = get_lcd_level();
- if (ret < 0)
- return ret;
-
- return sprintf(buf, "%i\n", ret);
-}
-
-static ssize_t store_lcd_level(struct device *dev,
- struct device_attribute *attr, const char *buf,
- size_t count)
-{
-
- int level, ret;
-
- if (sscanf(buf, "%i", &level) != 1
- || (level < 0 || level >= fujitsu_bl->max_brightness))
- return -EINVAL;
-
- if (use_alt_lcd_levels)
- ret = set_lcd_level_alt(level);
- else
- ret = set_lcd_level(level);
- if (ret < 0)
- return ret;
-
- ret = get_lcd_level();
- if (ret < 0)
- return ret;
-
- return count;
-}
-
static ssize_t
ignore_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
@@ -600,18 +523,11 @@ show_radios_state(struct device *dev,
return sprintf(buf, "killed\n");
}
-static DEVICE_ATTR(max_brightness, 0444, show_max_brightness, ignore_store);
-static DEVICE_ATTR(brightness_changed, 0444, show_brightness_changed,
- ignore_store);
-static DEVICE_ATTR(lcd_level, 0644, show_lcd_level, store_lcd_level);
static DEVICE_ATTR(lid, 0444, show_lid_state, ignore_store);
static DEVICE_ATTR(dock, 0444, show_dock_state, ignore_store);
static DEVICE_ATTR(radios, 0444, show_radios_state, ignore_store);
static struct attribute *fujitsu_pf_attributes[] = {
- &dev_attr_brightness_changed.attr,
- &dev_attr_max_brightness.attr,
- &dev_attr_lcd_level.attr,
&dev_attr_lid.attr,
&dev_attr_dock.attr,
&dev_attr_radios.attr,
--
2.12.0
Platform device handling adds a lot of complexity to fujitsu_init(),
which hinders its readability. Extract code responsible for adding and
removing the platform device to separate functions. Adjust whitespace
to make checkpatch happy.
Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 66 +++++++++++++++++++++--------------
1 file changed, 40 insertions(+), 26 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 9719c821d64f..421402a19bb9 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -765,6 +765,40 @@ static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
/* ACPI device for hotkey handling */
+static int fujitsu_laptop_platform_add(void)
+{
+ int ret;
+
+ fujitsu_bl->pf_device = platform_device_alloc("fujitsu-laptop", -1);
+ if (!fujitsu_bl->pf_device)
+ return -ENOMEM;
+
+ ret = platform_device_add(fujitsu_bl->pf_device);
+ if (ret)
+ goto err_put_platform_device;
+
+ ret = sysfs_create_group(&fujitsu_bl->pf_device->dev.kobj,
+ &fujitsu_pf_attribute_group);
+ if (ret)
+ goto err_del_platform_device;
+
+ return 0;
+
+err_del_platform_device:
+ platform_device_del(fujitsu_bl->pf_device);
+err_put_platform_device:
+ platform_device_put(fujitsu_bl->pf_device);
+
+ return ret;
+}
+
+static void fujitsu_laptop_platform_remove(void)
+{
+ sysfs_remove_group(&fujitsu_bl->pf_device->dev.kobj,
+ &fujitsu_pf_attribute_group);
+ platform_device_unregister(fujitsu_bl->pf_device);
+}
+
static int acpi_fujitsu_laptop_add(struct acpi_device *device)
{
int result = 0;
@@ -1146,25 +1180,13 @@ static int __init fujitsu_init(void)
/* Register platform stuff */
- fujitsu_bl->pf_device = platform_device_alloc("fujitsu-laptop", -1);
- if (!fujitsu_bl->pf_device) {
- ret = -ENOMEM;
- goto err_unregister_acpi;
- }
-
- ret = platform_device_add(fujitsu_bl->pf_device);
- if (ret)
- goto err_put_platform_device;
-
- ret =
- sysfs_create_group(&fujitsu_bl->pf_device->dev.kobj,
- &fujitsu_pf_attribute_group);
+ ret = fujitsu_laptop_platform_add();
if (ret)
- goto err_del_platform_device;
+ goto err_unregister_acpi;
ret = platform_driver_register(&fujitsu_pf_driver);
if (ret)
- goto err_remove_sysfs_group;
+ goto err_remove_platform_device;
/* Register laptop driver */
@@ -1186,13 +1208,8 @@ static int __init fujitsu_init(void)
kfree(fujitsu_laptop);
err_unregister_platform_driver:
platform_driver_unregister(&fujitsu_pf_driver);
-err_remove_sysfs_group:
- sysfs_remove_group(&fujitsu_bl->pf_device->dev.kobj,
- &fujitsu_pf_attribute_group);
-err_del_platform_device:
- platform_device_del(fujitsu_bl->pf_device);
-err_put_platform_device:
- platform_device_put(fujitsu_bl->pf_device);
+err_remove_platform_device:
+ fujitsu_laptop_platform_remove();
err_unregister_acpi:
acpi_bus_unregister_driver(&acpi_fujitsu_bl_driver);
err_free_fujitsu_bl:
@@ -1209,10 +1226,7 @@ static void __exit fujitsu_cleanup(void)
platform_driver_unregister(&fujitsu_pf_driver);
- sysfs_remove_group(&fujitsu_bl->pf_device->dev.kobj,
- &fujitsu_pf_attribute_group);
-
- platform_device_unregister(fujitsu_bl->pf_device);
+ fujitsu_laptop_platform_remove();
acpi_bus_unregister_driver(&acpi_fujitsu_bl_driver);
--
2.12.0
Both struct fujitsu_bl and struct fujitsu_laptop have a pf_device field.
Up until now the latter has been redundant, which is logically incorrect
because the primary function of the platform device created by
fujitsu-laptop is to provide laptop-related (not brightness-related)
attributes to userspace.
Remove the pf_device field from struct fujitsu_bl and make all module
code use the pf_device field of struct fujitsu_laptop instead.
Suggested-by: Alan Jenkins <[email protected]>
Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index a2641cb79df9..f3ccef3d5a1e 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -138,7 +138,6 @@ struct fujitsu_bl {
struct input_dev *input;
char phys[32];
struct backlight_device *bl_device;
- struct platform_device *pf_device;
int keycode1, keycode2, keycode3, keycode4, keycode5;
unsigned int max_brightness;
@@ -769,15 +768,15 @@ static int fujitsu_laptop_platform_add(void)
{
int ret;
- fujitsu_bl->pf_device = platform_device_alloc("fujitsu-laptop", -1);
- if (!fujitsu_bl->pf_device)
+ fujitsu_laptop->pf_device = platform_device_alloc("fujitsu-laptop", -1);
+ if (!fujitsu_laptop->pf_device)
return -ENOMEM;
- ret = platform_device_add(fujitsu_bl->pf_device);
+ ret = platform_device_add(fujitsu_laptop->pf_device);
if (ret)
goto err_put_platform_device;
- ret = sysfs_create_group(&fujitsu_bl->pf_device->dev.kobj,
+ ret = sysfs_create_group(&fujitsu_laptop->pf_device->dev.kobj,
&fujitsu_pf_attribute_group);
if (ret)
goto err_del_platform_device;
@@ -785,18 +784,18 @@ static int fujitsu_laptop_platform_add(void)
return 0;
err_del_platform_device:
- platform_device_del(fujitsu_bl->pf_device);
+ platform_device_del(fujitsu_laptop->pf_device);
err_put_platform_device:
- platform_device_put(fujitsu_bl->pf_device);
+ platform_device_put(fujitsu_laptop->pf_device);
return ret;
}
static void fujitsu_laptop_platform_remove(void)
{
- sysfs_remove_group(&fujitsu_bl->pf_device->dev.kobj,
+ sysfs_remove_group(&fujitsu_laptop->pf_device->dev.kobj,
&fujitsu_pf_attribute_group);
- platform_device_unregister(fujitsu_bl->pf_device);
+ platform_device_unregister(fujitsu_laptop->pf_device);
}
static int acpi_fujitsu_laptop_add(struct acpi_device *device)
@@ -909,7 +908,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
#if IS_ENABLED(CONFIG_LEDS_CLASS)
if (call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) {
- result = led_classdev_register(&fujitsu_bl->pf_device->dev,
+ result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
&logolamp_led);
if (result == 0) {
fujitsu_laptop->logolamp_registered = 1;
@@ -921,7 +920,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
if ((call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & KEYBOARD_LAMPS) &&
(call_fext_func(FUNC_BUTTONS, 0x0, 0x0, 0x0) == 0x0)) {
- result = led_classdev_register(&fujitsu_bl->pf_device->dev,
+ result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
&kblamps_led);
if (result == 0) {
fujitsu_laptop->kblamps_registered = 1;
@@ -938,7 +937,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
* that an RF LED is present.
*/
if (call_fext_func(FUNC_BUTTONS, 0x0, 0x0, 0x0) & BIT(24)) {
- result = led_classdev_register(&fujitsu_bl->pf_device->dev,
+ result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
&radio_led);
if (result == 0) {
fujitsu_laptop->radio_led_registered = 1;
@@ -955,7 +954,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
*/
if ((call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & BIT(14)) &&
(call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0) != UNSUPPORTED_CMD)) {
- result = led_classdev_register(&fujitsu_bl->pf_device->dev,
+ result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
&eco_led);
if (result == 0) {
fujitsu_laptop->eco_led_registered = 1;
--
2.12.0
Use the DEVICE_ATTR_RO() macro to get rid of ignore_store() and shorten
attribute definitions. Adjust whitespace to make checkpatch happy.
Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 9b9348af7626..9719c821d64f 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -480,16 +480,8 @@ static const struct backlight_ops fujitsu_bl_ops = {
.update_status = bl_update_status,
};
-static ssize_t
-ignore_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t count)
-{
- return count;
-}
-
-static ssize_t
-show_lid_state(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t lid_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
{
if (!(fujitsu_laptop->flags_supported & FLAG_LID))
return sprintf(buf, "unknown\n");
@@ -499,9 +491,8 @@ show_lid_state(struct device *dev,
return sprintf(buf, "closed\n");
}
-static ssize_t
-show_dock_state(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t dock_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
{
if (!(fujitsu_laptop->flags_supported & FLAG_DOCK))
return sprintf(buf, "unknown\n");
@@ -511,9 +502,8 @@ show_dock_state(struct device *dev,
return sprintf(buf, "undocked\n");
}
-static ssize_t
-show_radios_state(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t radios_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
{
if (!(fujitsu_laptop->flags_supported & FLAG_RFKILL))
return sprintf(buf, "unknown\n");
@@ -523,9 +513,9 @@ show_radios_state(struct device *dev,
return sprintf(buf, "killed\n");
}
-static DEVICE_ATTR(lid, 0444, show_lid_state, ignore_store);
-static DEVICE_ATTR(dock, 0444, show_dock_state, ignore_store);
-static DEVICE_ATTR(radios, 0444, show_radios_state, ignore_store);
+static DEVICE_ATTR_RO(lid);
+static DEVICE_ATTR_RO(dock);
+static DEVICE_ATTR_RO(radios);
static struct attribute *fujitsu_pf_attributes[] = {
&dev_attr_lid.attr,
--
2.12.0
The platform device registered by fujitsu-laptop is registered
unconditionally while sysfs attributes attached to it depend on the
FUJ02E3 ACPI device being present. Fix this by moving platform device
creation and removal to acpi_fujitsu_laptop_add() and
acpi_fujitsu_laptop_remove(), respectively.
Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 421402a19bb9..a2641cb79df9 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -903,6 +903,10 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
fujitsu_bl->bl_device->props.power = FB_BLANK_UNBLANK;
}
+ error = fujitsu_laptop_platform_add();
+ if (error)
+ goto err_unregister_input_dev;
+
#if IS_ENABLED(CONFIG_LEDS_CLASS)
if (call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) {
result = led_classdev_register(&fujitsu_bl->pf_device->dev,
@@ -994,6 +998,8 @@ static int acpi_fujitsu_laptop_remove(struct acpi_device *device)
led_classdev_unregister(&eco_led);
#endif
+ fujitsu_laptop_platform_remove();
+
input_unregister_device(input);
kfifo_free(&fujitsu_laptop->fifo);
@@ -1180,13 +1186,9 @@ static int __init fujitsu_init(void)
/* Register platform stuff */
- ret = fujitsu_laptop_platform_add();
- if (ret)
- goto err_unregister_acpi;
-
ret = platform_driver_register(&fujitsu_pf_driver);
if (ret)
- goto err_remove_platform_device;
+ goto err_unregister_acpi;
/* Register laptop driver */
@@ -1208,8 +1210,6 @@ static int __init fujitsu_init(void)
kfree(fujitsu_laptop);
err_unregister_platform_driver:
platform_driver_unregister(&fujitsu_pf_driver);
-err_remove_platform_device:
- fujitsu_laptop_platform_remove();
err_unregister_acpi:
acpi_bus_unregister_driver(&acpi_fujitsu_bl_driver);
err_free_fujitsu_bl:
@@ -1226,8 +1226,6 @@ static void __exit fujitsu_cleanup(void)
platform_driver_unregister(&fujitsu_pf_driver);
- fujitsu_laptop_platform_remove();
-
acpi_bus_unregister_driver(&acpi_fujitsu_bl_driver);
kfree(fujitsu_bl);
--
2.12.0
Hi Michael
On Tue, Mar 14, 2017 at 11:26:26AM +0100, Micha?? K??pie?? wrote:
> This series removes backlight-related sysfs attributes from the platform
> device registered by fujitsu-laptop and does some other cleanups to
> platform device code which hopefully make it easier to understand.
Thanks for your continued clean up efforts. I will review and test this in
the next few days. I will be particularly busy between now and the weekend,
so in the worst case it could be Sunday before I get a chance to do this
thoroughly. However, given the relative simplicity of the patches I will
try to get it done before then.
Regards
jonathan
On Wed, Mar 15, 2017 at 10:49:08AM +1030, Jonathan Woithe wrote:
> Hi Michael
>
> On Tue, Mar 14, 2017 at 11:26:26AM +0100, Micha?? K??pie?? wrote:
> > This series removes backlight-related sysfs attributes from the platform
> > device registered by fujitsu-laptop and does some other cleanups to
> > platform device code which hopefully make it easier to understand.
>
> Thanks for your continued clean up efforts. I will review and test this in
> the next few days. I will be particularly busy between now and the weekend,
> so in the worst case it could be Sunday before I get a chance to do this
> thoroughly. However, given the relative simplicity of the patches I will
> try to get it done before then.
I'll await Jonathan's Reviewed-by, but I've reviewed these as well and they are
consistent with the discussion on the platform device sysfs attributes we had
previously and all look good to me.
--
Darren Hart
VMware Open Source Technology Center
Apologies for the delay in reviewing this series: I've had a busy few days.
On Tue, Mar 14, 2017 at 11:26:26AM +0100, Micha?? K??pie?? wrote:
> This series removes backlight-related sysfs attributes from the platform
> device registered by fujitsu-laptop and does some other cleanups to
> platform device code which hopefully make it easier to understand.
This patch series codifies the outcome of discussions held over the last few
weeks and improves the consistency of the fujitsu-laptop driver with respect
to its treatment of sysfs attributes. The additional minor cleanups
facilitated by these changes are also worthwhile. The patch series has been
tested on the S7020 hardware and no regressions have been observed with the
hardware devices of that platform. Please apply.
Tested-by: Jonathan Woithe <[email protected]>
Reviewed-by: Jonathan Woithe <[email protected]>
Regards
jonathan
On Sat, Mar 18, 2017 at 10:10:35PM +1030, Jonathan Woithe wrote:
> Apologies for the delay in reviewing this series: I've had a busy few days.
>
> On Tue, Mar 14, 2017 at 11:26:26AM +0100, Micha?? K??pie?? wrote:
> > This series removes backlight-related sysfs attributes from the platform
> > device registered by fujitsu-laptop and does some other cleanups to
> > platform device code which hopefully make it easier to understand.
>
> This patch series codifies the outcome of discussions held over the last few
> weeks and improves the consistency of the fujitsu-laptop driver with respect
> to its treatment of sysfs attributes. The additional minor cleanups
> facilitated by these changes are also worthwhile. The patch series has been
> tested on the S7020 hardware and no regressions have been observed with the
> hardware devices of that platform. Please apply.
>
> Tested-by: Jonathan Woithe <[email protected]>
> Reviewed-by: Jonathan Woithe <[email protected]>
Applied, thanks Jonathan.
--
Darren Hart
VMware Open Source Technology Center