Re: [PATCH 0/2] extcon: Intel Cherry Trail Whiskey Cove PMIC and external charger tweaks

[Hans de Goede]

Hi,

On 15-02-19 10:29, Andy Shevchenko wrote:
> On Fri, Feb 15, 2019 at 10:31 AM Hans de Goede <hdegoede@xxxxxxxxxx> 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