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

[Dan Scally]

On 01/12/2020 12:32, Sakari Ailus wrote:
> Hi Dan,
>
> On Tue, Dec 01, 2020 at 08:08:26AM +0000, Dan Scally wrote:
>> 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.
> Oh, then the problem is actually there. If it probes the tps68470 driver on
> the systems with Windows ACPI tables, then it should be that driver which
> works with the Windows ACPI tables, too.
Sorry, clarification here: The INT3472 driver in patch #18 runs probe()
for the device representing a physical tps68470, but then -EINVAL's. The
existing tps68470 mfd driver doesn't probe() for the dummy INT3472 device.