Hi Hans,
On Wed, Jul 05, 2023 at 12:51:50PM +0200, Hans de Goede wrote:
> Hi,
>
> On 7/5/23 12:38, Sakari Ailus wrote:
> > Hi Hans,
> >
> > On Tue, Jul 04, 2023 at 09:21:47PM +0200, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 6/30/23 16:45, Andy Shevchenko wrote:
> >>>> + sensor->lanes = gmin_cfg_get_int(adev, "CsiLanes", lanes);
> >>>> + if (sensor->lanes > IPU_MAX_LANES) {
> >>>> + dev_err(&adev->dev, "Invalid lane-count: %d\n", sensor->lanes);
> >>>
> >>> Yeah, I think we would be consistent in using the ACPI handle to print
> >>> the messages from ACPI sensor devices.
> >>
> >> I do agree that we need to be consistent, but I regret having switched
> >> to using the handle for this in the csi2-bridge code. The dmesg logs
> >> this results in are much harder to read. Most devices typically have
> >> 2 different sensors and normally it is quite easy to see in the logs
> >> which GPIOs, etc. are being used for the sensor.
> >>
> >> But after the move to using the ACPI handle for logging the logs
> >> show up prefixed with \_SB_.I2C2.CAM8 resp CAM2 rather then with
> >> OVTI2680 and INT0310 making it much harder to figure on what
> >> is going on (first need to do
> >> "cat /sys/bus/i2c/devices/i2c-OVTI2680:00/firmware_node/path"
> >> to find out which path belongs to which sensor).
> >
> > In cases such as the above, the developer probably needs to address issues
> > not in the sensor driver but in the ACPI tables (or in IPU bridge code). So
> > for this reason I'd prefer printing the device path instead of the HID
> > (which is also somewhat opaque).
> >
> >>
> >> So I would rather get rid of the handle based logging, because it
> >> is very cumbersome to use.
> >
> > The V4L2 async and fwnode frameworks use handles, too, for the same reason.
> >
> > That said, I don't mind printing device names either. AFAIR Laurent
> > actually proposed that recently for the V4L2 fwnode and even promised to
> > send a patch. :-)
>
> Hmm, ok. I'll keep the acpi_handle logging then and add a " %s:", dev_name() to
> the logs so that we log both the ACPI handle path and the dev-name / HID.
>
> >> I'll add an extra patch to the next version of the set to switch all
> >> the logging to using the acpi_device for logging.
>
> So this extra patch is going to add logging of the dev_name() instead then.
Sounds good to me.
--
Sakari Ailus
