fujitsu_init() cleanup

These patches should make fujitsu_init() a bit more palatable. I intentionally stopped after four patches, because they should do no harm and the next steps require some discussion.

The problem I have with this driver is that it is generally oblivious to the user's chosen backlight provider. It was written before acpi_video support for backlight control was automatically detected by the kernel and as such it required a fix which was allegedly provided by 7d5c89a615c5 ("fujitsu-laptop: fingers off backlight if video.ko is serving this functionality"). However, that fix was superficial as the module still registered its vendor-specific ACPI driver for backlight control. That in turn caused SBLL/SBL2 to be called when brightness hotkeys were pressed even when acpi_video was supposed to handle brightness changes, which is exactly what that patch was hoping to prevent.

Moreover, the module unconditionally exports backlight-related sysfs attributes which enable userspace to change the brightness level, which should also only be possible when vendor backlight control is used. Fast forward several years and on modern Fujitsu machines (e.g. Lifebook E744), the kernel automatically detects that native backlight control [1] should be used and SBLL becomes a noop [2]. This creates confusion, because the driver claims that it can get/set LCD brightness level via the platform device's sysfs attributes, but it actually cannot. It gets even more confusing on Skylake machines on which the FUJ02B1 ACPI device is not present at all.

The proposal I put forward in regard to the above is to remove the three backlight-related attributes (brightness_changed, lcd_level, max_brightness) from the platform driver's sysfs tree. I believe the functions they implement should only be exposed through the backlight device, because the latter is only created when the user explicitly selects vendor backlight control or it is automatically selected by the kernel (this should be the case for older machines).

That would leave us with the remaining three sysfs attributes of the platform device, namely dock, lid and radios. These all depend on the FUJ02E3 ACPI device. Which begs the question: shall we reassign them to that ACPI device and drop the platform device altogether? This would logically be the correct thing to do (panasonic-laptop and toshiba_acpi already assign extra sysfs attributes to ACPI nodes). But I understand that this would break an 8-year-old userspace interface as functions previously exposed through /sys/devices/platform/fujitsu-laptop would be moved to /sys/bus/acpi/devices/FUJ02E3:00. If that is unacceptable, the least we can (and should) do is to move platform device registration to

acpi_fujitsu_hotkey_add(). What the driver currently does may create confusion in the future, because the platform device is registered unconditionally while it clearly depends on FUJ02E3 being present. I do not know whether FUJ02E3 is present on all Fujitsu devices today without exception, but I do know that if Fujitsu ever decides to drop that device from its firmware, we would again (see above) expose a userspace interface (dock, lid, radios) which simply will not be able to function properly.

I am happy to provide patches for whatever solution gets chosen, but first I would really appreciate if you (and anyone interested) could comment on the issues I described above.

Quoted 3 lines

These patches should make fujitsu_init() a bit more palatable. I intentionally stopped after four patches, because they should do no harm and the next steps require some discussion.

I will review these as soon as I can, which might have to be after LCA2017. That is, there could be a delay of just over a week before I can get to this.

Quoted 11 lines

The problem I have with this driver is that it is generally oblivious to the user's chosen backlight provider. It was written before acpi_video support for backlight control was automatically detected by the kernel and as such it required a fix which was allegedly provided by 7d5c89a615c5 ("fujitsu-laptop: fingers off backlight if video.ko is serving this functionality"). However, that fix was superficial as the module still registered its vendor-specific ACPI driver for backlight control. That in turn caused SBLL/SBL2 to be called when brightness hotkeys were pressed even when acpi_video was supposed to handle brightness changes, which is exactly what that patch was hoping to prevent.

I suspect this is going to get messy fairly quickly. The problem that this module was orginally written to solve was that on certain Fujitsu laptops (the S7020 for example) the standard ACPI video functionality was not able to control the backlight (even after acpi_video gained its automatic capabilities). The non-standard manipulations done by fujitsu-laptop were required for these machines or else it was completely impossible for the backlight to be controlled by software.

I would have to go digging through
some ancient notes to verifiy this, but I think there was also an issue whereby the brightness buttons on these laptops didn't work without the fujitsu-laptop brightness functions being called in response to the button presses. Or something like that - it was a long time ago.

With this in mind, I think what we're starting to see now is a conflict between the requirements of newer machines (which do sane things with regard to ACPI video and backlight, and therefore only really need the standard approach offered by acpi_video) and the older hardware which simply doesn't work with acpi_video or the standard backlight approach.

Quoted 10 lines

Moreover, the module unconditionally exports backlight-related sysfs attributes which enable userspace to change the brightness level, which should also only be possible when vendor backlight control is used. Fast forward several years and on modern Fujitsu machines (e.g. Lifebook E744), the kernel automatically detects that native backlight control [1] should be used and SBLL becomes a noop [2]. This creates confusion, because the driver claims that it can get/set LCD brightness level via the platform device's sysfs attributes, but it actually cannot. It gets even more confusing on Skylake machines on which the FUJ02B1 ACPI device is not present at all.

Again, what I think this highlights is that the older hardware requires code which turns out to break (or at least significantly complicate) things on modern hardware.

There were some older fujitsu systems where vendor backlight control was the only option.

That said, there is clearly merit in suppressing the backlight sysfs attributes on systems where they are not functional.

Quoted 7 lines

The proposal I put forward in regard to the above is to remove the three backlight-related attributes (brightness_changed, lcd_level, max_brightness) from the platform driver's sysfs tree. I believe the functions they implement should only be exposed through the backlight device, because the latter is only created when the user explicitly selects vendor backlight control or it is automatically selected by the kernel (this should be the case for older machines).

I will need to do some more digging into the historical developments. At face value I'm not sure how machines like the S7020 would end up controlling the backlight if these sysfs attributes were removed because on this machine (at least last time I checked) the standard backlight/video drivers could not effect brightness control on this hardware. Or is there a side effect that I have forgotten?

Quoted 10 lines

That would leave us with the remaining three sysfs attributes of the platform device, namely dock, lid and radios. These all depend on the FUJ02E3 ACPI device. Which begs the question: shall we reassign them to that ACPI device and drop the platform device altogether? This would logically be the correct thing to do (panasonic-laptop and toshiba_acpi already assign extra sysfs attributes to ACPI nodes). But I understand that this would break an 8-year-old userspace interface as functions previously exposed through /sys/devices/platform/fujitsu-laptop would be moved to /sys/bus/acpi/devices/FUJ02E3:00. If that is unacceptable, the least we can (and should) do is to move platform device registration to

Quoted 8 lines

acpi_fujitsu_hotkey_add(). What the driver currently does may create confusion in the future, because the platform device is registered unconditionally while it clearly depends on FUJ02E3 being present. I do not know whether FUJ02E3 is present on all Fujitsu devices today without exception, but I do know that if Fujitsu ever decides to drop that device from its firmware, we would again (see above) expose a userspace interface (dock, lid, radios) which simply will not be able to function properly.

There is a significant misalignment of names in the fujitsu-laptop driver which is probably adding to the overall confusion. Back in 2009 Alan Jenkins proposed a series of patches to clean this up. Unfortunately these came at a bad time for me and I wasn't able to evaluate them on my hardware, so they langished. I have been meaning to resurrect those patches ever since but have never quite had the time to do so. Most recently I was planning to do it over the New Year break, but something else came up which prevented that. :-(

It might be worth glancing through these because the resulting renames in particular definitely improved the clarity of the driver:

Date: Thu, 17 Sep 2009
From: Alan Jenkins
Subject: [PATCH 1/4] fujitsu-laptop: renames and cleanup
[PATCH 2/4] fujitsu-laptop: restructure initialisation [PATCH 3/4] fujitsu-laptop: autodetect lcd interface on unknown models [PATCH 4/4] fujitsu-laptop: simplify modalias (autoload)

Unfortunately I do not have access to recent Fujitsu hardware so I cannot comment as to whether FUJ02E3 is ubiquitous.

Regarding the movement of sysfs attributes, it is my understanding that breaking the userspace API in this way is frowned upon.

Personally I could
argue that given the relatively specialised nature of these attributes and their consequential low rate of use in the wild, such a move might be justifiable. However, the kernel tends to hold userspace interfaces to be perpetual so I can't see how such a change would get up in this case. Darren may like to comment on this.

Quoted 3 lines

I am happy to provide patches for whatever solution gets chosen, but first I would really appreciate if you (and anyone interested) could comment on the issues I described above.

As mentioned I will need to dig up information from 2007/8 to identify the precise issues encountered with hardware from that time. I am fairly certain that the old hardware required the custom backlight control implementation or else it was simply not functional. There was also an issue that the hardware did not turn the backlight on coming out of
hibernation: it was necessary to arrange for this to be fixed by software. When the unified ACPI video and backlight drivers eventuated they did not seem capable of controlling the backlight on this hardware, which is why the fujitsu-laptop backlight control remained in place.

Perhaps we might have to consider splitting fujitsu-laptop such that it provides two distinct and indepentent functions: the old backlight interface which is loaded only for the older hardware which requires it, and support for the other devices where needed. The need for this obviously depends on the requirements of the hardware.

I am in no way opposed to a clean up of the driver. However, it must be done in a way which preserves functionality on the older hardware.

Disclaimer: I have an S7020 which is still in active service, and I don't want to loose software control over the backlight.

Quoted 9 lines

It might be worth glancing through these because the resulting renames in particular definitely improved the clarity of the driver:

Date: Thu, 17 Sep 2009
From: Alan Jenkins
Subject: [PATCH 1/4] fujitsu-laptop: renames and cleanup
[PATCH 2/4] fujitsu-laptop: restructure initialisation [PATCH 3/4] fujitsu-laptop: autodetect lcd interface on unknown models [PATCH 4/4] fujitsu-laptop: simplify modalias (autoload)

These subjects suggest that it makes little sense for me to duplicate Alan's work. I have a lot more cleanups on my todo list, but the more I change, the less sense Alan's patches will make. I guess the most reasonable course of action would be for you to go through Alan's series after all so that I will only need to send fixes for whatever he has missed. Feel free to suggest otherwise, though.

Quoted 12 lines

Quoted 12 lines

The problem I have with this driver is that it is generally oblivious to the user's chosen backlight provider. It was written before acpi_video support for backlight control was automatically detected by the kernel and as such it required a fix which was allegedly provided by 7d5c89a615c5 ("fujitsu-laptop: fingers off backlight if video.ko is serving this functionality"). However, that fix was superficial as the module still registered its vendor-specific ACPI driver for backlight control. That in turn caused SBLL/SBL2 to be called when brightness hotkeys were pressed even when acpi_video was supposed to handle brightness changes, which is exactly what that patch was hoping to prevent. Moreover, the module unconditionally exports backlight-related sysfs attributes which enable userspace to change the brightness level,

Quoted 8 lines

Quoted 8 lines

which should also only be possible when vendor backlight control is used. Fast forward several years and on modern Fujitsu machines (e.g. Lifebook E744), the kernel automatically detects that native backlight control [1] should be used and SBLL becomes a noop [2]. This creates confusion, because the driver claims that it can get/set LCD brightness level via the platform device's sysfs attributes, but it actually cannot. It gets even more confusing on Skylake machines on which the FUJ02B1 ACPI device is not present at all.

Quoted 8 lines

Quoted 7 lines

The proposal I put forward in regard to the above is to remove the three backlight-related attributes (brightness_changed, lcd_level, max_brightness) from the platform driver's sysfs tree. I believe the functions they implement should only be exposed through the backlight device, because the latter is only created when the user explicitly selects vendor backlight control or it is automatically selected by the kernel (this should be the case for older machines).

Quoted 6 lines

I will need to do some more digging into the historical developments. At face value I'm not sure how machines like the S7020 would end up controlling the backlight if these sysfs attributes were removed because on this machine (at least last time I checked) the standard backlight/video drivers could not effect brightness control on this hardware. Or is there a side effect that I have forgotten?

Let us not confuse three separate things that fujitsu-laptop enables:

* changing LCD brightness using the keyboard,
* changing LCD brightness from userspace, using the backlight device,
* changing LCD brightness from userspace, using sysfs attributes.

I have nothing against the first two, apart from the notion that they should be more tightly coupled (i.e. one should not be enabled without the other one and vice versa).

I am all against the last one. IMHO it is a duplicate, misplaced feature which only makes clean separation of components inside the driver hard.

Quoted 18 lines

Quoted 18 lines

That would leave us with the remaining three sysfs attributes of the platform device, namely dock, lid and radios. These all depend on the FUJ02E3 ACPI device. Which begs the question: shall we reassign them to that ACPI device and drop the platform device altogether? This would logically be the correct thing to do (panasonic-laptop and toshiba_acpi already assign extra sysfs attributes to ACPI nodes). But I understand that this would break an 8-year-old userspace interface as functions previously exposed through /sys/devices/platform/fujitsu-laptop would be moved to /sys/bus/acpi/devices/FUJ02E3:00. If that is unacceptable, the least we can (and should) do is to move platform device registration to acpi_fujitsu_hotkey_add(). What the driver currently does may create confusion in the future, because the platform device is registered unconditionally while it clearly depends on FUJ02E3 being present. I do not know whether FUJ02E3 is present on all Fujitsu devices today without exception, but I do know that if Fujitsu ever decides to drop that device from its firmware, we would again (see above) expose a userspace interface (dock, lid, radios) which simply will not be able to function properly.

Quoted 2 lines

Regarding the movement of sysfs attributes, it is my understanding that breaking the userspace API in this way is frowned upon.

This is my understanding as well, which is why this is only a proposal and not a patch.

Quoted 6 lines

Personally I could
argue that given the relatively specialised nature of these attributes and their consequential low rate of use in the wild, such a move might be justifiable. However, the kernel tends to hold userspace interfaces to be perpetual so I can't see how such a change would get up in this case. Darren may like to comment on this.

Yes, this definitely needs input from subsystem maintainers.

Let me just emphasize once again that I believe backlight-related sysfs attributes belonging to the platform driver are a misplaced feature. Backlight-related features belong in backlight devices. The only situation I can think of that someone will get hurt by these getting removed is that they have some custom script which uses the platform device instead of the backlight device to control LCD brightness. Removing these attributes has no effect on whether brightness-related keys on the keyboard work or not.

Nevertheless, if the consensus is that these should stay where they are, they need to be added conditionally, only when acpi_backlight=vendor. Otherwise they simply do not work on modern hardware and cause confusion.

Darren, Andy,

Please drop this patch series for now. I will send a rebased v2 after a long overdue patch series from Alan Jenkins gets applied in a reworked form.

However, your input is still essential for determining the future shape of fujitsu-laptop. I quoted the essential parts of my discussion with Jonathan below.

Quoted 5 lines · Michał Kępień

Darren, Andy,

Please drop this patch series for now. I will send a rebased v2 after a long overdue patch series from Alan Jenkins gets applied in a reworked form.

Noted.

Just for you information. Our repo now consists three branches: fixes, for-next, and testing. testing is kinda for internal use, it might be broken or be in a wrong state.
Better to use for-next as a base. For this cycle (which is my first as

Quoted 9 lines · Michał Kępień

Quoted 5 lines

Darren, Andy,

Please drop this patch series for now. I will send a rebased v2 after a long overdue patch series from Alan Jenkins gets applied in a reworked form.

Noted.

Quoted 4 lines

Just for you information. Our repo now consists three branches: fixes, for-next, and testing. testing is kinda for internal use, it might be broken or be in a wrong state.
Better to use for-next as a base. For this cycle (which is my first as

In general, you should be able to use Linus' master as the base. Only if you require patches already in for-next, should you use for-next.

Quoted 15 lines · Andy Shevchenko

Quoted 13 lines · Michał Kępień

Quoted 5 lines

Darren, Andy,

Please drop this patch series for now. I will send a rebased v2 after a long overdue patch series from Alan Jenkins gets applied in a reworked form.

Noted.

Just for you information. Our repo now consists three branches: fixes, for-next, and testing. testing is kinda for internal use, it might be broken or be in a wrong state.
Better to use for-next as a base. For this cycle (which is my first as

Quoted 2 lines

In general, you should be able to use Linus' master as the base. Only if you require patches already in for-next, should you use for-next.

Thanks, guys, this is all helpful information. I have always assumed testing was the correct base branch because my patches got pushed there once they got accepted.

Darren, Andy,

Please drop this patch series for now. I will send a rebased v2 after a long overdue patch series from Alan Jenkins gets applied in a reworked form.

However, your input is still essential for determining the future shape of fujitsu-laptop. I quoted the essential parts of my discussion with Jonathan below.

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

The problem I have with this driver is that it is generally oblivious to the user's chosen backlight provider. It was written before acpi_video support for backlight control was automatically detected by the kernel and as such it required a fix which was allegedly provided by 7d5c89a615c5 ("fujitsu-laptop: fingers off backlight if video.ko is serving this functionality"). However, that fix was superficial as the module still registered its vendor-specific ACPI driver for backlight control. That in turn caused SBLL/SBL2 to be called when brightness hotkeys were pressed even when acpi_video was supposed to handle brightness changes, which is exactly what that patch was hoping to prevent. Moreover, the module unconditionally exports backlight-related sysfs attributes which enable userspace to change the brightness level,

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

which should also only be possible when vendor backlight control is used. Fast forward several years and on modern Fujitsu machines (e.g. Lifebook E744), the kernel automatically detects that native backlight control [1] should be used and SBLL becomes a noop [2]. This creates confusion, because the driver claims that it can get/set LCD brightness level via the platform device's sysfs attributes, but it actually cannot. It gets even more confusing on Skylake machines on which the FUJ02B1 ACPI device is not present at all.

[]

Quoted 21 lines

Quoted 8 lines

Quoted 7 lines

The proposal I put forward in regard to the above is to remove the three backlight-related attributes (brightness_changed, lcd_level, max_brightness) from the platform driver's sysfs tree. I believe the functions they implement should only be exposed through the backlight device, because the latter is only created when the user explicitly selects vendor backlight control or it is automatically selected by the kernel (this should be the case for older machines).

Quoted 6 lines

I will need to do some more digging into the historical developments. At face value I'm not sure how machines like the S7020 would end up controlling the backlight if these sysfs attributes were removed because on this machine (at least last time I checked) the standard backlight/video drivers could not effect brightness control on this hardware. Or is there a side effect that I have forgotten?

Let us not confuse three separate things that fujitsu-laptop enables:

* changing LCD brightness using the keyboard,
* changing LCD brightness from userspace, using the backlight device,
* changing LCD brightness from userspace, using sysfs attributes.

Quoted 26 lines

I have nothing against the first two, apart from the notion that they should be more tightly coupled (i.e. one should not be enabled without the other one and vice versa).

I am all against the last one. IMHO it is a duplicate, misplaced feature which only makes clean separation of components inside the driver hard.

Quoted 18 lines

Quoted 18 lines

That would leave us with the remaining three sysfs attributes of the platform device, namely dock, lid and radios. These all depend on the FUJ02E3 ACPI device. Which begs the question: shall we reassign them to that ACPI device and drop the platform device altogether? This would logically be the correct thing to do (panasonic-laptop and toshiba_acpi already assign extra sysfs attributes to ACPI nodes). But I understand that this would break an 8-year-old userspace interface as functions previously exposed through /sys/devices/platform/fujitsu-laptop would be moved to /sys/bus/acpi/devices/FUJ02E3:00. If that is unacceptable, the least we can (and should) do is to move platform device registration to acpi_fujitsu_hotkey_add(). What the driver currently does may create confusion in the future, because the platform device is registered unconditionally while it clearly depends on FUJ02E3 being present. I do not know whether FUJ02E3 is present on all Fujitsu devices today without exception, but I do know that if Fujitsu ever decides to drop that device from its firmware, we would again (see above) expose a userspace interface (dock, lid, radios) which simply will not be able to function properly.

[]

Quoted 24 lines

Quoted 2 lines

Regarding the movement of sysfs attributes, it is my understanding that breaking the userspace API in this way is frowned upon.

This is my understanding as well, which is why this is only a proposal and not a patch.

Quoted 6 lines

Personally I could
argue that given the relatively specialised nature of these attributes and their consequential low rate of use in the wild, such a move might be justifiable. However, the kernel tends to hold userspace interfaces to be perpetual so I can't see how such a change would get up in this case. Darren may like to comment on this.

Yes, this definitely needs input from subsystem maintainers.

Let me just emphasize once again that I believe backlight-related sysfs attributes belonging to the platform driver are a misplaced feature. Backlight-related features belong in backlight devices. The only situation I can think of that someone will get hurt by these getting removed is that they have some custom script which uses the platform device instead of the backlight device to control LCD brightness. Removing these attributes has no effect on whether brightness-related keys on the keyboard work or not.

Quoted 4 lines

Nevertheless, if the consensus is that these should stay where they are, they need to be added conditionally, only when acpi_backlight=vendor. Otherwise they simply do not work on modern hardware and cause confusion.

In regard to the above, please let me know what you think about:

- removing backlight-related attributes from the platform device,

- removing the platform device altogether and attaching laptop-specific attributes to the ACPI device instead.

Quoted 9 lines

Darren, Andy,

Please drop this patch series for now. I will send a rebased v2 after a long overdue patch series from Alan Jenkins gets applied in a reworked form.

However, your input is still essential for determining the future shape of fujitsu-laptop. I quoted the essential parts of my discussion with Jonathan below.

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

The problem I have with this driver is that it is generally oblivious to the user's chosen backlight provider. It was written before acpi_video support for backlight control was automatically detected by the kernel and as such it required a fix which was allegedly provided by 7d5c89a615c5 ("fujitsu-laptop: fingers off backlight if video.ko is serving this functionality"). However, that fix was superficial as the module still registered its vendor-specific ACPI driver for backlight control. That in turn caused SBLL/SBL2 to be called when brightness hotkeys were pressed even when acpi_video was supposed to handle brightness changes, which is exactly what that patch was hoping to prevent. Moreover, the module unconditionally exports backlight-related sysfs attributes which enable userspace to change the brightness level,

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

which should also only be possible when vendor backlight control is used. Fast forward several years and on modern Fujitsu machines (e.g. Lifebook E744), the kernel automatically detects that native backlight control [1] should be used and SBLL becomes a noop [2]. This creates confusion, because the driver claims that it can get/set LCD brightness level via the platform device's sysfs attributes, but it actually cannot. It gets even more confusing on Skylake machines on which the FUJ02B1 ACPI device is not present at all.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

The proposal I put forward in regard to the above is to remove the three backlight-related attributes (brightness_changed, lcd_level, max_brightness) from the platform driver's sysfs tree. I believe the functions they implement should only be exposed through the backlight device, because the latter is only created when the user explicitly selects vendor backlight control or it is automatically selected by the kernel (this should be the case for older machines).

Quoted 21 lines

Quoted 21 lines

Quoted 6 lines

I will need to do some more digging into the historical developments. At face value I'm not sure how machines like the S7020 would end up controlling the backlight if these sysfs attributes were removed because on this machine (at least last time I checked) the standard backlight/video drivers could not effect brightness control on this hardware. Or is there a side effect that I have forgotten?

Let us not confuse three separate things that fujitsu-laptop enables:

* changing LCD brightness using the keyboard,
* changing LCD brightness from userspace, using the backlight device,
* changing LCD brightness from userspace, using sysfs attributes.

I have nothing against the first two, apart from the notion that they should be more tightly coupled (i.e. one should not be enabled without the other one and vice versa).

I am all against the last one. IMHO it is a duplicate, misplaced feature which only makes clean separation of components inside the driver hard.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

That would leave us with the remaining three sysfs attributes of the platform device, namely dock, lid and radios. These all depend on the FUJ02E3 ACPI device. Which begs the question: shall we reassign them to that ACPI device and drop the platform device altogether? This would logically be the correct thing to do (panasonic-laptop and toshiba_acpi already assign extra sysfs attributes to ACPI nodes). But I understand that this would break an 8-year-old userspace interface as functions previously exposed through /sys/devices/platform/fujitsu-laptop would be moved to /sys/bus/acpi/devices/FUJ02E3:00. If that is unacceptable, the

Quoted 10 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

least we can (and should) do is to move platform device registration to acpi_fujitsu_hotkey_add(). What the driver currently does may create confusion in the future, because the platform device is registered unconditionally while it clearly depends on FUJ02E3 being present. I do not know whether FUJ02E3 is present on all Fujitsu devices today without exception, but I do know that if Fujitsu ever decides to drop that device from its firmware, we would again (see above) expose a userspace interface (dock, lid, radios) which simply will not be able to function properly.

Quoted 4 lines

[]

Quoted 2 lines

Quoted 2 lines

Regarding the movement of sysfs attributes, it is my understanding that breaking the userspace API in this way is frowned upon.

Quoted 27 lines

Quoted 26 lines

This is my understanding as well, which is why this is only a proposal and not a patch.

Quoted 6 lines

Personally I could
argue that given the relatively specialised nature of these attributes and their consequential low rate of use in the wild, such a move might be justifiable. However, the kernel tends to hold userspace interfaces to be perpetual so I can't see how such a change would get up in this case. Darren may like to comment on this.

Yes, this definitely needs input from subsystem maintainers.

Let me just emphasize once again that I believe backlight-related sysfs attributes belonging to the platform driver are a misplaced feature. Backlight-related features belong in backlight devices. The only situation I can think of that someone will get hurt by these getting removed is that they have some custom script which uses the platform device instead of the backlight device to control LCD brightness. Removing these attributes has no effect on whether brightness-related keys on the keyboard work or not.

Nevertheless, if the consensus is that these should stay where they are, they need to be added conditionally, only when acpi_backlight=vendor. Otherwise they simply do not work on modern hardware and cause confusion.

Quoted 6 lines

In regard to the above, please let me know what you think about:

- removing backlight-related attributes from the platform device,

- removing the platform device altogether and attaching laptop-specific attributes to the ACPI device instead.

Darren, Andy,

As six weeks have passed since I originally asked for your comments, I hope another gentle reminder is okay. Your advice is needed before I can post further cleanups for fujitsu-laptop.

Greg, question for you below, see "GregKH"...

Quoted 9 lines · Michał Kępień

Quoted 9 lines

Darren, Andy,

Please drop this patch series for now. I will send a rebased v2 after a long overdue patch series from Alan Jenkins gets applied in a reworked form.

However, your input is still essential for determining the future shape of fujitsu-laptop. I quoted the essential parts of my discussion with Jonathan below.

Very sorry for the delay, I've lost context on this, let me see if I can pick it up with your summary below...

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

The problem I have with this driver is that it is generally oblivious to the user's chosen backlight provider. It was written before acpi_video support for backlight control was automatically detected by the kernel and as such it required a fix which was allegedly provided by 7d5c89a615c5 ("fujitsu-laptop: fingers off backlight if video.ko is serving this functionality"). However, that fix was superficial as the module still registered its vendor-specific ACPI driver for backlight control. That in turn caused SBLL/SBL2 to be called when brightness hotkeys were pressed even when acpi_video was supposed to handle brightness changes, which is exactly what that patch was hoping to prevent. Moreover, the module unconditionally exports backlight-related sysfs attributes which enable userspace to change the brightness level,

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

which should also only be possible when vendor backlight control is used. Fast forward several years and on modern Fujitsu machines (e.g. Lifebook E744), the kernel automatically detects that native backlight control [1] should be used and SBLL becomes a noop [2]. This creates confusion, because the driver claims that it can get/set LCD brightness level via the platform device's sysfs attributes, but it actually cannot. It gets even more confusing on Skylake machines on which the FUJ02B1 ACPI device is not present at all.

Right, evolved.... time to take a step back and clean it up.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

The proposal I put forward in regard to the above is to remove the three backlight-related attributes (brightness_changed, lcd_level, max_brightness) from the platform driver's sysfs tree. I believe the functions they implement should only be exposed through the backlight device, because the latter is only created when the user explicitly selects vendor backlight control or it is automatically selected by the kernel (this should be the case for older machines).

This seems to be a good approach as in combination with the discussion below re the ACPI device, it eliminates the sysfs attributes from the platform device completely and makes the driver more consistent with others in the subsystem.

Quoted 21 lines

Quoted 21 lines

Quoted 21 lines

Quoted 6 lines

I will need to do some more digging into the historical developments. At face value I'm not sure how machines like the S7020 would end up controlling the backlight if these sysfs attributes were removed because on this machine (at least last time I checked) the standard backlight/video drivers could not effect brightness control on this hardware. Or is there a side effect that I have forgotten?

Let us not confuse three separate things that fujitsu-laptop enables:

* changing LCD brightness using the keyboard,
* changing LCD brightness from userspace, using the backlight device,
* changing LCD brightness from userspace, using sysfs attributes.

I have nothing against the first two, apart from the notion that they should be more tightly coupled (i.e. one should not be enabled without the other one and vice versa).

I am all against the last one. IMHO it is a duplicate, misplaced feature which only makes clean separation of components inside the driver hard.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

That would leave us with the remaining three sysfs attributes of the platform device, namely dock, lid and radios. These all depend on the FUJ02E3 ACPI device. Which begs the question: shall we reassign them to that ACPI device and drop the platform device altogether? This would logically be the correct thing to do (panasonic-laptop and toshiba_acpi already assign extra sysfs attributes to ACPI nodes). But I understand that this would break an 8-year-old userspace interface as functions previously exposed through /sys/devices/platform/fujitsu-laptop would be moved to /sys/bus/acpi/devices/FUJ02E3:00. If that is unacceptable, the

We can change the device sysfs attributes using versioning. We should of course do our due diligence to discover if there are any users of this sysfs interface, and if so, if they have strong objections to changing. But, if someone screams, we can always revert.

GregKH - Can you please confirm the above? Moving an attribute is different than the format and contents, which is what I explicitly documented in Documentation/admin-guide/sysfs-rules.rst (last section).

Quoted 88 lines

Greg, question for you below, see "GregKH"...

Quoted 9 lines · Michał Kępień

Quoted 9 lines

Darren, Andy,

Please drop this patch series for now. I will send a rebased v2 after a long overdue patch series from Alan Jenkins gets applied in a reworked form.

However, your input is still essential for determining the future shape of fujitsu-laptop. I quoted the essential parts of my discussion with Jonathan below.

Very sorry for the delay, I've lost context on this, let me see if I can pick it up with your summary below...

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

The problem I have with this driver is that it is generally oblivious to the user's chosen backlight provider. It was written before acpi_video support for backlight control was automatically detected by the kernel and as such it required a fix which was allegedly provided by 7d5c89a615c5 ("fujitsu-laptop: fingers off backlight if video.ko is serving this functionality"). However, that fix was superficial as the module still registered its vendor-specific ACPI driver for backlight control. That in turn caused SBLL/SBL2 to be called when brightness hotkeys were pressed even when acpi_video was supposed to handle brightness changes, which is exactly what that patch was hoping to prevent. Moreover, the module unconditionally exports backlight-related sysfs attributes which enable userspace to change the brightness level,

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

which should also only be possible when vendor backlight control is used. Fast forward several years and on modern Fujitsu machines (e.g. Lifebook E744), the kernel automatically detects that native backlight control [1] should be used and SBLL becomes a noop [2]. This creates confusion, because the driver claims that it can get/set LCD brightness level via the platform device's sysfs attributes, but it actually cannot. It gets even more confusing on Skylake machines on which the FUJ02B1 ACPI device is not present at all.

Right, evolved.... time to take a step back and clean it up.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

The proposal I put forward in regard to the above is to remove the three backlight-related attributes (brightness_changed, lcd_level, max_brightness) from the platform driver's sysfs tree. I believe the functions they implement should only be exposed through the backlight device, because the latter is only created when the user explicitly selects vendor backlight control or it is automatically selected by the kernel (this should be the case for older machines).

This seems to be a good approach as in combination with the discussion below re the ACPI device, it eliminates the sysfs attributes from the platform device completely and makes the driver more consistent with others in the subsystem.

Quoted 21 lines

Quoted 21 lines

Quoted 21 lines

Quoted 6 lines

I will need to do some more digging into the historical developments. At face value I'm not sure how machines like the S7020 would end up controlling the backlight if these sysfs attributes were removed because on this machine (at least last time I checked) the standard backlight/video drivers could not effect brightness control on this hardware. Or is there a side effect that I have forgotten?

Let us not confuse three separate things that fujitsu-laptop enables:

* changing LCD brightness using the keyboard,
* changing LCD brightness from userspace, using the backlight device,
* changing LCD brightness from userspace, using sysfs attributes.

I have nothing against the first two, apart from the notion that they should be more tightly coupled (i.e. one should not be enabled without the other one and vice versa).

I am all against the last one. IMHO it is a duplicate, misplaced feature which only makes clean separation of components inside the driver hard.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

That would leave us with the remaining three sysfs attributes of the platform device, namely dock, lid and radios. These all depend on the FUJ02E3 ACPI device. Which begs the question: shall we reassign them to that ACPI device and drop the platform device altogether? This would logically be the correct thing to do (panasonic-laptop and toshiba_acpi already assign extra sysfs attributes to ACPI nodes). But I understand that this would break an 8-year-old userspace interface as functions previously exposed through /sys/devices/platform/fujitsu-laptop would be moved to /sys/bus/acpi/devices/FUJ02E3:00. If that is unacceptable, the

We can change the device sysfs attributes using versioning. We should of course do our due diligence to discover if there are any users of this sysfs interface, and if so, if they have strong objections to changing. But, if someone screams, we can always revert.

Quoted 3 lines

GregKH - Can you please confirm the above? Moving an attribute is different than the format and contents, which is what I explicitly documented in Documentation/admin-guide/sysfs-rules.rst (last section).

Moving an attribute to a different device structure is usually a bad idea, if the userspace tool counting on it to be present in a specific place breaks.

And I really don't like putting random device attributes on "parent" devices. I know Dmitry Torokhov disagrees about this though, his subsystem does like doing that. But whatever you do, you should at least try to be consistent.

But, as you can't be consistent here, don't break userspace please, I'd recommend just leaving it alone for now.

Quoted 5 lines · GregKH

Quoted 3 lines · Darren Hart

GregKH - Can you please confirm the above? Moving an attribute is different than the format and contents, which is what I explicitly documented in Documentation/admin-guide/sysfs-rules.rst (last section).

Quoted 3 lines

Moving an attribute to a different device structure is usually a bad idea, if the userspace tool counting on it to be present in a specific place breaks.

Yes, I am familiar with that premise. Here is the thing though: I am unaware of any userspace tool which uses these attributes. Though, obviously, that does not mean such tools do not exist.

Quoted 2 lines

But, as you can't be consistent here, don't break userspace please, I'd recommend just leaving it alone for now.

Darren, in the light of the above I will be awaiting your final call on this before posting any further patches touching this area. My number one priority was to drop the broken backlight-related attributes, because leaving the other attributes where they currently are does not prevent achieving a clean split between the two drivers registered by fujitsu-laptop, which is the ultimate objective of all these cleanups.

Quoted 6 lines

Quoted 5 lines · GregKH

Quoted 3 lines · Darren Hart

GregKH - Can you please confirm the above? Moving an attribute is different than the format and contents, which is what I explicitly documented in Documentation/admin-guide/sysfs-rules.rst (last section).

Quoted 7 lines

Quoted 3 lines

Moving an attribute to a different device structure is usually a bad idea, if the userspace tool counting on it to be present in a specific place breaks.

Yes, I am familiar with that premise. Here is the thing though: I am unaware of any userspace tool which uses these attributes. Though, obviously, that does not mean such tools do not exist.

For what it's worth I too am unaware of any utilities which use the /sys/devices/platform/fujitsu-laptop/ attributes associated with the backlight - this after using the S7020 since 2005. I would be surprised if any existed since they would have to be specifically for the Fujitsu hardware. If writing any utility to control the backlight the logical thing to do would be to use the standard backlight attributes in /sys/devices/virtual/backlight/fujitsu-laptop/.

Quoted 9 lines

Quoted 2 lines

But, as you can't be consistent here, don't break userspace please, I'd recommend just leaving it alone for now.

Darren, in the light of the above I will be awaiting your final call on this before posting any further patches touching this area. My number one priority was to drop the broken backlight-related attributes, because leaving the other attributes where they currently are does not prevent achieving a clean split between the two drivers registered by fujitsu-laptop, which is the ultimate objective of all these cleanups.

As I explained in my response to GregKH earlier and having investigated this in more detail, I have no objection to the removal of the non-standard backlight-related sysfs attributes (brightness_changed, lcd_level and max_brightness). They are almost certainly unused and their removal will allow a significant cleanup of fujitsu-laptop. Obviously however there are competing viewpoints which take a bigger picture view so I will defer to Darren's judgement.

Quoted 33 lines · Micha?? K??pie??

Quoted 6 lines

Quoted 5 lines · GregKH

Quoted 3 lines · Darren Hart

GregKH - Can you please confirm the above? Moving an attribute is different than the format and contents, which is what I explicitly documented in Documentation/admin-guide/sysfs-rules.rst (last section).

Quoted 7 lines

Quoted 3 lines

Moving an attribute to a different device structure is usually a bad idea, if the userspace tool counting on it to be present in a specific place breaks.

Yes, I am familiar with that premise. Here is the thing though: I am unaware of any userspace tool which uses these attributes. Though, obviously, that does not mean such tools do not exist.

For what it's worth I too am unaware of any utilities which use the /sys/devices/platform/fujitsu-laptop/ attributes associated with the backlight - this after using the S7020 since 2005. I would be surprised if any existed since they would have to be specifically for the Fujitsu hardware. If writing any utility to control the backlight the logical thing to do would be to use the standard backlight attributes in /sys/devices/virtual/backlight/fujitsu-laptop/.

Quoted 9 lines

Quoted 2 lines

But, as you can't be consistent here, don't break userspace please, I'd recommend just leaving it alone for now.

Darren, in the light of the above I will be awaiting your final call on this before posting any further patches touching this area. My number one priority was to drop the broken backlight-related attributes, because leaving the other attributes where they currently are does not prevent achieving a clean split between the two drivers registered by fujitsu-laptop, which is the ultimate objective of all these cleanups.

Quoted 7 lines

As I explained in my response to GregKH earlier and having investigated this in more detail, I have no objection to the removal of the non-standard backlight-related sysfs attributes (brightness_changed, lcd_level and max_brightness). They are almost certainly unused and their removal will allow a significant cleanup of fujitsu-laptop. Obviously however there are competing viewpoints which take a bigger picture view so I will defer to Darren's judgement.

Okay, so it looks like I did the best I could to explain my intentions and some confusion crept in anyway, sorry about that. Let me try again, I will be as concise as I can.

Current custom attributes attached to the platform device are:

    /sys/bus/platform/devices/fujitsu-laptop
    `- brightness_changed [*]
    `- dock
    `- lcd_level [*]
    `- lid
    `- max_brightness [*]
    `- radios

AFAICT, all four of us agree that attributes marked with an asterisk [*] should be removed from the platform device because they are exposed by the backlight device anyway and do not work correctly under certain circumstances.

However, Darren's question to Greg did not apply to these attributes. It was about the other three attributes: dock, lid and radios.

In other words, my proposal was to change this:

    /sys/bus/platform/devices/fujitsu-laptop
    `- dock
    `- lid
    `- radios

into this:

    /sys/bus/acpi/devices/FUJ02E3:00
    `- dock
    `- lid
    `- radios

That would enable us to completely rip the platform driver and platform device out of fujitsu-laptop, cutting the driver down by ca. 40 lines.

Greg objected to that, arguing that there might be userspace applications using these attributes under their current path. I do not know of such an application. My question to Darren was thus: do we move dock, lid and radios and rip the platform driver and platform device out of fujitsu-laptop or should these attributes rather stay where they are?

Quoted 8 lines · Michał Kępień

Quoted 2 lines · Micha?? K??pie??

Quoted 1 line

GregKH wrote:

Greg objected to that, arguing that there might be userspace applications using these attributes under their current path. I do not know of such an application. My question to Darren was thus: do we move dock, lid and radios and rip the platform driver and platform device out of fujitsu-laptop or should these attributes rather stay where they are?

I would say just to leave them as-is, there's no harm in those 40 lines, and you really don't want to break someone's existing setup for no good reason.

Quoted 55 lines · Michał Kępień

Quoted 33 lines · Micha?? K??pie??

Quoted 6 lines

Quoted 5 lines · GregKH

Quoted 3 lines · Darren Hart

GregKH - Can you please confirm the above? Moving an attribute is different than the format and contents, which is what I explicitly documented in Documentation/admin-guide/sysfs-rules.rst (last section).

Quoted 7 lines

Quoted 3 lines

Moving an attribute to a different device structure is usually a bad idea, if the userspace tool counting on it to be present in a specific place breaks.

Yes, I am familiar with that premise. Here is the thing though: I am unaware of any userspace tool which uses these attributes. Though, obviously, that does not mean such tools do not exist.

For what it's worth I too am unaware of any utilities which use the /sys/devices/platform/fujitsu-laptop/ attributes associated with the backlight - this after using the S7020 since 2005. I would be surprised if any existed since they would have to be specifically for the Fujitsu hardware. If writing any utility to control the backlight the logical thing to do would be to use the standard backlight attributes in /sys/devices/virtual/backlight/fujitsu-laptop/.

Quoted 9 lines

Quoted 2 lines

But, as you can't be consistent here, don't break userspace please, I'd recommend just leaving it alone for now.

Darren, in the light of the above I will be awaiting your final call on this before posting any further patches touching this area. My number one priority was to drop the broken backlight-related attributes, because leaving the other attributes where they currently are does not prevent achieving a clean split between the two drivers registered by fujitsu-laptop, which is the ultimate objective of all these cleanups.

Quoted 7 lines

As I explained in my response to GregKH earlier and having investigated this in more detail, I have no objection to the removal of the non-standard backlight-related sysfs attributes (brightness_changed, lcd_level and max_brightness). They are almost certainly unused and their removal will allow a significant cleanup of fujitsu-laptop. Obviously however there are competing viewpoints which take a bigger picture view so I will defer to Darren's judgement.

Okay, so it looks like I did the best I could to explain my intentions and some confusion crept in anyway, sorry about that. Let me try again, I will be as concise as I can.

Current custom attributes attached to the platform device are:

    /sys/bus/platform/devices/fujitsu-laptop
    `- brightness_changed [*]
    `- dock
    `- lcd_level [*]
    `- lid
    `- max_brightness [*]
    `- radios

Quoted 4 lines

AFAICT, all four of us agree that attributes marked with an asterisk [*] should be removed from the platform device because they are exposed by the backlight device anyway and do not work correctly under certain circumstances.

Quoted 18 lines

However, Darren's question to Greg did not apply to these attributes. It was about the other three attributes: dock, lid and radios.

In other words, my proposal was to change this:

    /sys/bus/platform/devices/fujitsu-laptop
    `- dock
    `- lid
    `- radios

into this:

    /sys/bus/acpi/devices/FUJ02E3:00
    `- dock
    `- lid
    `- radios

Quoted 8 lines

That would enable us to completely rip the platform driver and platform device out of fujitsu-laptop, cutting the driver down by ca. 40 lines.

Greg objected to that, arguing that there might be userspace applications using these attributes under their current path. I do not know of such an application. My question to Darren was thus: do we move dock, lid and radios and rip the platform driver and platform device out of fujitsu-laptop or should these attributes rather stay where they are?

I would say just to leave them as-is, there's no harm in those 40 lines, and you really don't want to break someone's existing setup for no good reason.

Quoted 31 lines · Darren Hart

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

The problem I have with this driver is that it is generally oblivious to the user's chosen backlight provider. It was written before acpi_video support for backlight control was automatically detected by the kernel and as such it required a fix which was allegedly provided by 7d5c89a615c5 ("fujitsu-laptop: fingers off backlight if video.ko is serving this functionality"). However, that fix was superficial as the module still registered its vendor-specific ACPI driver for backlight control. That in turn caused SBLL/SBL2 to be called when brightness hotkeys were pressed even when acpi_video was supposed to handle brightness changes, which is exactly what that patch was hoping to prevent. Moreover, the module unconditionally exports backlight-related sysfs attributes which enable userspace to change the brightness level,

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

which should also only be possible when vendor backlight control is used. Fast forward several years and on modern Fujitsu machines (e.g. Lifebook E744), the kernel automatically detects that native backlight control [1] should be used and SBLL becomes a noop [2]. This creates confusion, because the driver claims that it can get/set LCD brightness level via the platform device's sysfs attributes, but it actually cannot. It gets even more confusing on Skylake machines on which the FUJ02B1 ACPI device is not present at all.

Right, evolved.... time to take a step back and clean it up.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

The proposal I put forward in regard to the above is to remove the three backlight-related attributes (brightness_changed, lcd_level, max_brightness) from the platform driver's sysfs tree. I believe the functions they implement should only be exposed through the backlight device, because the latter is only created when the user explicitly selects vendor backlight control or it is automatically selected by the kernel (this should be the case for older machines).

Quoted 3 lines

This seems to be a good approach as in combination with the discussion below re the ACPI device, it eliminates the sysfs attributes from the platform device completely and makes the driver more consistent with others in the subsystem.

Quoted 15 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

That would leave us with the remaining three sysfs attributes of the platform device, namely dock, lid and radios. These all depend on the FUJ02E3 ACPI device. Which begs the question: shall we reassign them to that ACPI device and drop the platform device altogether? This would logically be the correct thing to do (panasonic-laptop and toshiba_acpi already assign extra sysfs attributes to ACPI nodes). But I understand that this would break an 8-year-old userspace interface as functions previously exposed through /sys/devices/platform/fujitsu-laptop would be moved to /sys/bus/acpi/devices/FUJ02E3:00. If that is unacceptable, the

We can change the device sysfs attributes using versioning. We should of course do our due diligence to discover if there are any users of this sysfs interface, and if so, if they have strong objections to changing. But, if someone screams, we can always revert.

Quoted 3 lines

GregKH - Can you please confirm the above? Moving an attribute is different than the format and contents, which is what I explicitly documented in Documentation/admin-guide/sysfs-rules.rst (last section).

Moving an attribute to a different device structure is usually a bad idea, if the userspace tool counting on it to be present in a specific place breaks.

And I really don't like putting random device attributes on "parent" devices. I know Dmitry Torokhov disagrees about this though, his subsystem does like doing that. But whatever you do, you should at least try to be consistent.

But, as you can't be consistent here, don't break userspace please, I'd recommend just leaving it alone for now.

Quoted 32 lines · Greg Kroah-Hartman

Quoted 31 lines · Darren Hart

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

The problem I have with this driver is that it is generally oblivious to the user's chosen backlight provider. It was written before acpi_video support for backlight control was automatically detected by the kernel and as such it required a fix which was allegedly provided by 7d5c89a615c5 ("fujitsu-laptop: fingers off backlight if video.ko is serving this functionality"). However, that fix was superficial as the module still registered its vendor-specific ACPI driver for backlight control. That in turn caused SBLL/SBL2 to be called when brightness hotkeys were pressed even when acpi_video was supposed to handle brightness changes, which is exactly what that patch was hoping to prevent. Moreover, the module unconditionally exports backlight-related sysfs attributes which enable userspace to change the brightness level,

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

which should also only be possible when vendor backlight control is used. Fast forward several years and on modern Fujitsu machines (e.g. Lifebook E744), the kernel automatically detects that native backlight control [1] should be used and SBLL becomes a noop [2]. This creates confusion, because the driver claims that it can get/set LCD brightness level via the platform device's sysfs attributes, but it actually cannot. It gets even more confusing on Skylake machines on which the FUJ02B1 ACPI device is not present at all.

Right, evolved.... time to take a step back and clean it up.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

The proposal I put forward in regard to the above is to remove the three backlight-related attributes (brightness_changed, lcd_level, max_brightness) from the platform driver's sysfs tree. I believe the functions they implement should only be exposed through the backlight device, because the latter is only created when the user explicitly selects vendor backlight control or it is automatically selected by the kernel (this should be the case for older machines).

Quoted 3 lines

Quoted 3 lines

This seems to be a good approach as in combination with the discussion below re the ACPI device, it eliminates the sysfs attributes from the platform device completely and makes the driver more consistent with others in the subsystem.

I have now had a chance to revisit the historical background, especially as it relates to the three sysfs attributes mentioned (brightness_changed, lcd_level, max_brightness). It seems to me that these are most likely left overs from a very early version of fujitsu-laptop, originally out-of-tree, merged to mainline by myself and others. Like Michael I doubt there is any userspace utility which makes use of these. Checking the scripts which I personally use on my S7020 I see that I only ever use the attributes provided under/sys/devices/virtual/backlight/fujitsu-laptop when controlling the backlight. Like Michael said, it's impossible to completely rule out the possiblity of an obscure Fujitsu-only utility somewhere in the universe which makes use of the /sys/devices/platform/fujitsu-laptop/ backlight attributes but I think the probability of such a thing existing is vanishingly small. Even if there was, there is a mostly trivial mapping from old to new (lcd_level -> brightness, max_brightness -> max_brightness). Only brightness_changed isn't replicated but I'm sure something could be hooked if necessary.

What this means is that from my perspective it is highly unlikely that removing the three sysfs attributes as proposed by Michael (brightness_changed, lcd_level, max_brightness) will break userspace in a practical sense. Instead, it will improve the structure of the fujitsu-laptop driver and make it more consistent with other platform drivers. Putting all this together, I have no objections to the removal as proposed by Michael: there appears to be far more gains than losses.