On Wed, Aug 1, 2018 at 10:37 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > Hi, > > > On 01-08-18 10:27, Rafael J. Wysocki wrote: >> >> On Wed, Aug 1, 2018 at 9:56 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >>> >>> Hi, >>> >>> On 31-07-18 21:30, Wolfram Sang wrote: >>>> >>>> >>>> >>>>> As mentioned in an earlier reply in this thread, the only alternative >>>>> to this patch-set which I see would be to duplicate the list of >>>>> ACPI HIDs found in the new drivers/i2c/i2c-multi-instantiate.c >>>>> inside code under drivers/acpi and make the acpi code not >>>>> skip enumerating devices with these HIDs as platform drivers. >>>>> >>>>> Then the i2c-multi-instantiate.c driver could become a >>>>> platform driver (*) and the I2C_CLIENT_IGNORE_BUSY >>>>> flag / hack can go away (at the price of duplicating >>>>> the HIDs in 2 places). >>>> >>>> >>>> >>>> What is the downside of duplicating the HID? >>> >>> >>> >>> The downside is that as new HIDs show up they need to be added in >>> 2 places. The drivers/hid code has something similar where devices >>> needing custom drivers needed to have their VID:PID added to a >>> blacklist for the generic driver and in my experience I always >>> forget about this and the first time I test a new custom hid >>> driver it fails because of this. >>> >>> IOW its not ideal but it is not really a problem either. >>> >>>> My gut feeling is that it can't be worse than having two devices mapped >>>> to the same I2C address and work around that by I2C_CLIENT_IGNORE_BUSY. >>>> But I may be biased :) >>>> >>>> I mean this is broken firmware, and having that fixed in ACPI and >>>> platform/x86 code instead of I2C core sounds way more proper to me. >>> >>> >>> >>> Andy, Heikki, what is your take on this approach: >>> >>> 1) Have a list of ACPI IDs for which to not set the >>> acpi_device_enumeration_by_parent flag in drivers/acpi/scan.c >>> so that they get enumerated as a platform device. >>> >>> 2) Turn the i2c-multi-instantiate.c driver from the second patch >>> of this series into a platform driver which will then >>> instantiate *all* the i2c-clients for the ACPI fwnode as >>> before. >> >> >> What would that bind to? > > To the platform device which will be instantiated for the > ACPI fwnode because of us not setting the acpi_device_enumeration_by_parent > flag (*) for fwnode's with a HID for which we know we need the > i2c-multi-instantiate.c driver, which is why we will then have > a list of the HIDs in 2 places. OK