Re: [RFC PATCH v3 9/9] ipu3-cio2: Add functionality allowing software_node connections to sensors on platforms designed for Windows

Laurent Pinchart
2020-10-29T20:18:36Z

Hi Andy,

On Mon, Oct 26, 2020 at 06:05:49PM +0200, Andy Shevchenko wrote:
> On Mon, Oct 26, 2020 at 08:20:14AM +0000, Dan Scally wrote:
> > On 24/10/2020 23:36, Laurent Pinchart wrote:
> > > On Sat, Oct 24, 2020 at 11:28:06PM +0100, Daniel Scally wrote:
> > >> On 24/10/2020 10:37, Laurent Pinchart wrote:
> > >>>>> I wonder if we could avoid depending on the I2C device being created by
> > >>>>> getting the fwnode from adev, and setting ->secondary manually. adev
> > >>>>> would need to be passed to get_acpi_ssdb_sensor_data() instead of dev.
> > >>>>
> > >>>> Let me try that; I initially wanted to do
> > >>>> set_secondary_fwnode(&adev->dev, fwnode) to avoid depending on the I2C
> > >>>> dev being created but it turns out &adev->dev isn't the same pointer. I
> > >>>> shall try it and see.
> > >>
> > >> Actually, thinking on this further I think maybe we can't avoid that -
> > >> it's not actually in this patch series but during assigning GPIO
> > >> resources parsed from PMIC's ACPI node to the sensor, I'm using
> > >> dev_name() on the i2c dev to pass to .dev_id member of gpiod_lookup_table
> > >
> > > Any chance we can construct the I2C device name from the ACPI device,
> > > the same way that the ACPI/I2C core does ? It may be a dead end, but if
> > > we could avoid depending on the I2C device, I think it will make
> > > initialization easier. I have a feeling that will be difficult though,
> > > as we'll need the I2C bus number, which won't be readily available.
> >
> > OK yeah; the i2c core does indeed just prefix "i2c-" onto the acpi
> > device name, so I will make this change too.
>
> This is part of the I²C core and if you go this way, do not home grow the
> functionality that doesn't belong here.
>
> Instead, export a helper function that will do it for you and for I²C core with
> explanation why it's needed.

Agreed, I was actually considering suggesting that. Hardcoding the same
naming scheme in two places is asking for trouble, especially if we
don't let the I2C maintainers know. It should be easy to do, not
necessarily the highest priority task, but something that should be
handled to get this merged.

--
Regards,

Laurent Pinchart