On Tue, Feb 28, 2017 at 02:24:40PM +0100, Michał Kępień wrote: > > On Tue, Feb 28, 2017 at 09:33:28AM +0100, Micha?? K??pie?? wrote: > > > GregKH wrote: > > > > On Mon, Feb 27, 2017 at 10:17:55PM -0800, Darren Hart wrote: > > > > > 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. > > > > > > 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/. > > > > > > 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. > > 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? 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. thanks, greg k-h
