Hi, On 5/5/24 3:07 PM, Weifeng Liu wrote: > Emits messages upon errors during probing of SAM. Hopefully this could > provide useful context to user for the purpose of diagnosis when > something miserable happen. > > Reviewed-by: Maximilian Luz <luzmaximilian@xxxxxxxxx> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Acked-by: Hans de Goede <hdegoede@xxxxxxxxxx> > Signed-off-by: Weifeng Liu <weifeng.liu.z@xxxxxxxxx> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > drivers/platform/surface/aggregator/core.c | 42 ++++++++++++++-------- > 1 file changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/platform/surface/aggregator/core.c b/drivers/platform/surface/aggregator/core.c > index 7b1871eb7a6f..046fa63446bf 100644 > --- a/drivers/platform/surface/aggregator/core.c > +++ b/drivers/platform/surface/aggregator/core.c > @@ -618,15 +618,17 @@ static const struct acpi_gpio_mapping ssam_acpi_gpios[] = { > > static int ssam_serial_hub_probe(struct serdev_device *serdev) > { > - struct acpi_device *ssh = ACPI_COMPANION(&serdev->dev); > + struct device *dev = &serdev->dev; > + struct acpi_device *ssh = ACPI_COMPANION(dev); > struct ssam_controller *ctrl; > acpi_status astatus; > int status; > > - if (gpiod_count(&serdev->dev, NULL) < 0) > - return -ENODEV; > + status = gpiod_count(dev, NULL); > + if (status < 0) > + return dev_err_probe(dev, status, "no GPIO found\n"); > > - status = devm_acpi_dev_add_driver_gpios(&serdev->dev, ssam_acpi_gpios); > + status = devm_acpi_dev_add_driver_gpios(dev, ssam_acpi_gpios); > if (status) > return status; > > @@ -637,8 +639,11 @@ static int ssam_serial_hub_probe(struct serdev_device *serdev) > > /* Initialize controller. */ > status = ssam_controller_init(ctrl, serdev); > - if (status) > + if (status) { > + dev_err_probe(dev, status, > + "failed to initialize ssam controller\n"); > goto err_ctrl_init; > + } > > ssam_controller_lock(ctrl); > > @@ -657,14 +662,13 @@ static int ssam_serial_hub_probe(struct serdev_device *serdev) > if (status == -ENXIO) > status = -EPROBE_DEFER; > if (status) { > - dev_err_probe(&serdev->dev, status, > - "failed to open serdev device\n"); > + dev_err_probe(dev, status, "failed to open serdev device\n"); > goto err_devopen; > } > > astatus = ssam_serdev_setup_via_acpi(ssh->handle, serdev); > if (ACPI_FAILURE(astatus)) { > - status = -ENXIO; > + status = dev_err_probe(dev, -ENXIO, "failed to setup serdev\n"); > goto err_devinit; > } > > @@ -680,25 +684,33 @@ static int ssam_serial_hub_probe(struct serdev_device *serdev) > * states. > */ > status = ssam_log_firmware_version(ctrl); > - if (status) > + if (status) { > + dev_err_probe(dev, status, "failed to get firmware version\n"); > goto err_initrq; > + } > > status = ssam_ctrl_notif_d0_entry(ctrl); > - if (status) > + if (status) { > + dev_err_probe(dev, status, "D0-entry notification failed\n"); > goto err_initrq; > + } > > status = ssam_ctrl_notif_display_on(ctrl); > - if (status) > + if (status) { > + dev_err_probe(dev, status, "display-on notification failed\n"); > goto err_initrq; > + } > > - status = sysfs_create_group(&serdev->dev.kobj, &ssam_sam_group); > + status = sysfs_create_group(&dev->kobj, &ssam_sam_group); > if (status) > goto err_initrq; > > /* Set up IRQ. */ > status = ssam_irq_setup(ctrl); > - if (status) > + if (status) { > + dev_err_probe(dev, status, "failed to setup IRQ\n"); > goto err_irq; > + } > > /* Finally, set main controller reference. */ > status = ssam_try_set_controller(ctrl); > @@ -715,7 +727,7 @@ static int ssam_serial_hub_probe(struct serdev_device *serdev) > * resumed. In short, this causes some spurious unwanted wake-ups. > * For now let's thus default power/wakeup to false. > */ > - device_set_wakeup_capable(&serdev->dev, true); > + device_set_wakeup_capable(dev, true); > acpi_dev_clear_dependencies(ssh); > > return 0; > @@ -723,7 +735,7 @@ static int ssam_serial_hub_probe(struct serdev_device *serdev) > err_mainref: > ssam_irq_free(ctrl); > err_irq: > - sysfs_remove_group(&serdev->dev.kobj, &ssam_sam_group); > + sysfs_remove_group(&dev->kobj, &ssam_sam_group); > err_initrq: > ssam_controller_lock(ctrl); > ssam_controller_shutdown(ctrl);
