> -----Original Message----- > From: Pali Rohár [mailto:pali.rohar@xxxxxxxxx] > Sent: Thursday, May 12, 2016 3:41 AM > To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx> > Cc: kernel@xxxxxxxxxx; mjg59@xxxxxxxxxxxxx; dvhart@xxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v3 2/2] dell-laptop: Expose auxiliary MAC address if > available > > On Wednesday 11 May 2016 22:41:16 Mario_Limonciello@xxxxxxxx wrote: > > > > > +static DEVICE_ATTR_RO(auxiliary_mac); static struct attribute > > > > > +*dell_attributes[] = { > > > > > + &dev_attr_auxiliary_mac.attr, > > > > > + NULL > > > > > +}; > > > > > + > > > > > +static const struct attribute_group dell_attr_group = { > > > > > + .attrs = dell_attributes, > > > > > +}; > > > > > + > > > > > > In my opinion this is not correct way to export "random" sysfs > > > attributes to userspace. If it is possible we should use existing > > > API/ABI for kernel and not to invent new ABI for userspace. > > > > > > Dell-laptop driver has already documented ABI for non standard > > > things (like extended settings for keyboard backlight), see: > > > https://www.kernel.org/doc/Documentation/ABI/testing/sysfs- > platform- > > > dell-laptop > > > > > > And exporting MAC address sounds for me like bad idea. I think it > > > should handle kernel itself (maybe send it to driver which use it?). > > > > > > And most important question: Who is going to use that sysfs file? Is > > > there any application? It is not possible to query for that address from > userspace, e.g. > > > from libsmbios tools? > > > > > > We have libsmbios functionality in kernel just for things which have > > > exiting API/ABI/interface in kernel. Not those which do not have... > > > > > > So why is needed to have such sysfs attribute exported by kernel? > > > > > > > I skipped your other comments because of this one. My original plan for > this was to do it in udev or Network Manager but you raise a good point. > > Maybe this patch should be scrapped all together. > > > > Mical - if you think patch 1/2 could still be useful to have as a general > interface I'll update it for your other comments and get it resubmitted. > > What is first patch doing? Can you send me link to it? > > > We do mirror the information in ACPI under the system bus: > > > > Scope (_SB) > > { > > Name (AMAC, Buffer (0x17) > > { > > "_AUXMAC_#847BEB5992D2#" > > }) > > } > > > > I don't know how to properly access this from the kernel side. I noticed > that most drivers that reference ACPI nodes refer to devices, not something > hanging off the system bus. > > If you could advise the right way to go about that, I would appreciate it. > > So there are two ways how to read that MAC address. One is via SMM and > one via ACPI. Yes, this isn't a general statement for read only static information, but in this case it is true. > You can also read ACPI buffer (name is probably \_SB.AMAC) with ACPI > functions in kernel. Ask ACPI people, for correct API. I'm sure this is possible > also without creating new ACPI driver... Thanks will do. > > > If I can access that, maybe it's better to do this directly as a patch to the > Ethernet driver in question (r8152). > > That's actually how it's handled on the OS side for Windows too from what I > understand. > > We have some FW bit set in them to indicate they're Dell Realtek products > (don't have this detail yet). > > When they see that bit they look for that ACPI buffer and use it to set the > MAC address the OS sees. > > Maybe it should be better to chose same way as Windows drivers? Better > ask on netdev mailing list and ping maintainers of that ethernet driver what > they think about it. > > For me it sounds like a better solution (patching that ethernet driver) as > exporting some non-standard sysfs node from kernel with MAC address and > then using another tool which send that MAC address back to kernel. > Great, thank you for your feedback. I'll wander down that rabbit hole. ��.n��������+%������w��{.n������_���v��z����n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
