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. :-)
>
> 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.
--
Kind regards,
Sakari Ailus
