Messages in this thread | | | Subject | Re: [PATCH 0/2] extcon: Intel Cherry Trail Whiskey Cove PMIC and external charger tweaks | From | Hans de Goede <> | Date | Fri, 15 Feb 2019 10:33:41 +0100 |
| |
Hi,
On 15-02-19 10:29, Andy Shevchenko wrote: > On Fri, Feb 15, 2019 at 10:31 AM Hans de Goede <hdegoede@redhat.com> wrote: >> On 14-02-19 15:15, Yauhen Kharuzhy wrote: > >> I would do something similar with the fuel-gauge in >> drivers/platform/x86/intel_cht_int33fe.c, one option would >> be to simply count the number of resources in the ACPI >> resource table for the INT33FE device, versions with >> the Type-C port have 7 resources, where as your INT33FE >> device has only 3. >> >> I'm even thinking that it might be best to rename >> intel_cht_int33fe.c to intel_cht_int33fe_typec.c and add >> a check for the resource table having 7 entries there, then >> you can make a intel_cht_int33fe_micro_usb.c copy and strip >> that mostly empty. Both would bind to the same "INT33FE" >> id and they would both silently bail with -ENODEV if the >> resource-count (or the PTYP value) don't match. >> >> The reason I'm thinking of having 2 drivers is because >> the current intel_cht_int33fe.c is quite special / ugly and >> already has enough ifs. >> >> If you do a stand-alone intel_cht_int33fe_micro_usb.c that can >> hopefully be much simpler. >> >> Andy what is your take on having separate intel_cht_int33fe_typec.c >> and intel_cht_int33fe_micro_usb.c drivers, both binding to >> the "INT33FE" ACPI-ID (with its totally !@#%$#-ed up "API") ? > > Depends on how code would look better,
Well the existing drivers/platform/x86/intel_cht_int33fe.c file, which already is full of kludges would not get even more code-paths added; and the new file which Yauhen will wrote should be nice and clean with only 1 straight code-path pretty much.
> though I care about users that > they will not get additional Kconfig option and broken their > configurations when new piece of code landed up. So, from mine, as > user, prospective, we may split driver as we wish, but we should get > it working as previously for the existing cases.
That is a valid point, I'm not a fan of having even more Kconfig options either, so we can simply enable/disable both modules through the same Kconfig option.
Regards,
Hans
| |