Re: [PATCH v5 1/5] usb: Add support for Intel LJCA device

[Andy Shevchenko]

On Fri, Jun 30, 2023 at 07:40:48AM +0000, Wu, Wentong wrote:
> > From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> > Sent: Wednesday, March 15, 2023 5:10 PM
> > On Tue, Mar 14, 2023 at 05:52:52PM +0200, Andy Shevchenko wrote:
> > > On Tue, Mar 14, 2023 at 11:38:14PM +0800, Ye, Xiang wrote:
> > > > On Tue, Mar 14, 2023 at 10:36:57AM +0200, Heikki Krogerus wrote:
> > > > > On Tue, Mar 14, 2023 at 04:03:26PM +0800, Ye, Xiang wrote:

...

> > > > > You don't really seem to get any benefit from MFD. Perhaps it
> > > > > would be more appropriate and clear if you just registered
> > > > > auxiliary devices in this driver. Check drivers/base/auxiliary.c.
> > > > Yes, it should be a work. I have a question.
> > > > MFD provides the ACPI binding for sub-devices through struct
> > > > mfd_cell_acpi_match. But I didn't see this in drivers/base/auxiliary.c.
> > > > If using auxiliary bus to implement the LJCA sub-devices, we need to
> > > > do the sub-devices acpi binding manually in ljca.c.
> > > >
> > > > Something Like:
> > > > adr = LJCA_ACPI_MATCH_GPIO
> > > > adev = acpi_find_child_device(parent, adr, false);
> > > > ACPI_COMPANION_SET(&pdev->dev, adev ?: parent);
> > > >
> > > > Is that acceptable?
> 
> This actually doesn't work, look at the acpi_find_child_device(), it compares
> the bus address specified by _ADR object, but there is no _ADR object in DSDT
> for these three devices because the relationship between the parent and
> children isn't bus type listed in ACPI spec, so it always return NULL.

If you want to have this on ACPI enabled platform, ACPI table has to have
the necessary bits. What you are describing is a BIOS bug _or_ somebody has
to provide the SSDT overlay depending on the real connection of the device..

> > Looks ok to me.
> > 
> > > Maybe you can implement this on the level of auxiliary bus.
> > 
> > I would actually prefer that the auxiliary bus itself does not make any
> > assumptions regarding the whereabouts of the fwnodes at this stage. Maybe
> > later, when(if) there are more users.

-- 
With Best Regards,
Andy Shevchenko