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

[Wu, Wentong]

> -----Original Message-----
> 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.

BR,
Wentong 

> 
> 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.
> 
> thanks,
> 
> --
> heikki