Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

[Dan Scally]

On 01/12/2020 06:44, Sakari Ailus wrote:
> Hi Dan,
>
> On Mon, Nov 30, 2020 at 11:06:03PM +0000, Dan Scally wrote:
>> Hi Sakari
>>
>> On 30/11/2020 20:52, Sakari Ailus wrote:
>>>> +static const struct acpi_device_id int3472_device_id[] = {
>>>> +	{ "INT3472", 0 },
>>> The INT3472 _HID is really allocated for the tps68470 PMIC chip. It may not
>>> be used by other drivers; people will want to build kernels where both of
>>> these ACPI table layouts are functional.
>>>
>>> Instead, I propose, that you add this as an option to the tps68470 driver
>>> that figures out whether the ACPI device for the tps68470 device actually
>>> describes something else, in a similar fashion you do with the cio2-bridge
>>> driver. I think it may need a separate Kconfig option albeit this and
>>> cio2-bridge cannot be used separately.
>> It actually occurs to me that that may not work (I know I called that
>> out as an option we considered, but that was a while ago actually). The
>> reason I wasn't worried about the existing tps68470 driver binding to
>> these devices is that it's an i2c driver, and these dummy devices don't
>> have an I2cSerialBusV2, so no I2C device is created by them the kernel.
>>
>>
>> Won't that mean the tps68470 driver won't ever be probed for these devices?
> Oops. I missed this indeed was not an I²C driver. So please ignore the
> comment.
>
> So I guess this wouldn't be an actual problem. I'd still like to test this
> on a system with tps68470, as the rest of the set.
On my Go2, it .probes() for the actual tps68740 (that machine has both
types of INT3472 device) but fails with EINVAL when it can't find the
CLDB buffer that these discrete type devices have. My understanding is
that means it's free for the actual tps68470 driver to grab the device;
although that's not happening because I had to blacklist that driver or
it stops the machine from booting at the moment - haven't gotten round
to investigating yet.