linux-kernel - Re: [PATCH v2 1/1] Platform: x86: Add Chrome OS Laptop driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANLzEks3-TMow2miPCGgPyG5z3S8c8jUHjLYS_-Aw38hON1Bxw@mail.gmail.com>
Date:	Sun, 4 Nov 2012 12:27:12 -0800
From:	Benson Leung <bleung@...omium.org>
To:	Corentin Chary <corentin.chary@...il.com>
Cc:	Olof Johansson <olof@...om.net>, Matthew Garrett <mjg@...hat.com>,
	"platform-driver-x86@...r.kernel.org" 
	<platform-driver-x86@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Olof Johansson <olofj@...omium.org>
Subject: Re: [PATCH v2 1/1] Platform: x86: Add Chrome OS Laptop driver

Hi Corentin,

Thanks for your feedback on this. Indeed, what Olof is saying is
correct. In its current form, this driver exists to register i2c
devices on our laptops. We have one piece of information (the irq,
specifically) that cannot be added to the i2c_board_info when using
user space probing.

To answer your question about where this driver is going in the
future, you can take a look at the current version of the driver in
the chromiumos kernel:

https://gerrit.chromium.org/gitweb/?p=chromiumos/third_party/kernel.git;a=blob;f=drivers/platform/x86/chromeos_laptop.c;h=f948f61d864a5265f6e144833358512f68b7e467;hb=chromeos-3.4

I added more i2c devices that are registered the same way for a number
of other chromeos systems, but there is one platform device (a
keyboard backlight) that is added using
platform_device_register_simple.

Thanks again for reviewing,
Benson

On Fri, Nov 2, 2012 at 6:32 AM, Corentin Chary <corentin.chary@...il.com> wrote:
> On Fri, Nov 2, 2012 at 1:09 PM, Olof Johansson <olof@...om.net> wrote:
>> On Fri, Nov 2, 2012 at 2:03 PM, Corentin Chary <corentin.chary@...il.com> wrote:
>>> On Fri, Nov 2, 2012 at 12:45 PM, Olof Johansson <olof@...om.net> wrote:
>>>> On Fri, Oct 26, 2012 at 10:30 AM, Corentin Chary
>>>> <corentin.chary@...il.com> wrote:
>>>>
>>>>> Looks better, but I'm curious, what is the final purpose of this driver ?
>>>>> What ABI will be exposed, who will use it ?
>>>>>
>>>>> If it is going to be bigger, it may be a good idea to convert it to a
>>>>> real platform driver (platform_drivers/platform_device stuff).
>>>>
>>>> It's not a driver per se. It's platform glue that, based on the DMI
>>>> table, registers platform and i2c devices (at this time only i2c
>>>> devices).
>>>>
>>>> Unfortunately there's no way to do this nicely from userspace after
>>>> boot, since there's limits to how much data you can provide with the
>>>> simpler userspace-driven i2c probing protocol.
>>>>
>>>> So, there's no user-facing ABI on this, and no one is expected to use
>>>> it from userspace. It's just there to make sure that the un-probably
>>>> devices on this kind of hardware gets bound to drivers properly.
>>>>
>>>> If it's converted to a platform_driver, how do you expect that to
>>>> probe, where would the platform_device be registered?
>>>
>>> I guess I would check dmi in the module init method, and then use the
>>> probe callback of platform_create_bundle to do more probing if
>>> necessary.
>>
>> Maybe I'm dense but I don't see how that could possibly be better than
>> what the code does today. It would just add more overhead and clutter
>> by creating a unnecessary dummy device/driver setup just to, in the
>> end, register the same i2c devices.
>>
>
> Well that was the point of "If it is going to be bigger".
> Of course, as long as it only register those i2c devices, it doesn't
> really matter.
>
> --
> Corentin Chary
> http://xf.iksaif.net



-- 
Benson Leung
Software Engineer, Chrom* OS
bleung@...omium.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/