On Wednesday, February 14, 2024 1:39:19 AM PST Andy Shevchenko wrote:
> On Wed, Feb 14, 2024 at 12:39 AM Jonathan LoBue <jlobue10@xxxxxxxxx> wrote:
> >
> > This patch adds the ACPI match table for ASUS ROG ALLY to load the bmi323
> > driver with an ACPI match of "BOSC0200".
>
> With the remarks below,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> (carry the tag if you send a new version)
>
> ...
>
> The below comment...
>
> > +/*
> > + * The "BOSC0200" ACPI identifier used here in the bmi323 driver is not
>
> s/ACPI//
> s/in the bmi323 driver//
I will fix the first sentence in the comment.
> This seems different wording to the other one. Have you looked at the
> code if it's indeed the case? Because we may not rely on the module
> load order.
>
Yes it's slightly different wording intentionally. I am able to test that
when the bmc150 driver starts loading on the ASUS ROG ALLY with a bmi323
chip that the ACPI match happens, the driver attempts to initialize but
does correctly fail at the chip id check portion.
I do not own a device with a bmc150 chip in it, but the same should be
happening in the reverse situation where a device with a bmc150 chip
starts to load the bmi323 driver. There is a chip id check in the
bmi323_init function where a bmc150 device should fail at, and the driver
should release the device. Without a device, I am unable to test that
this works correctly or not. Logically the code looks similar between the
two drivers.
> > If and when a different driver (such as bmc150) starts to load
> > + * with the "BOSC0200" ACPI match, a short reset should ensure that the
> > + * device is not in a bad state during that driver initialization. This
> > + * device reset does occur in both the bmi323 and bmc150 init sequences.
> > + */
> > +
> > +static const struct acpi_device_id bmi323_acpi_match[] = {
>
> ...should be here (and indented accordingly).
>
> > + { "BOSC0200" },
> > + { }
> > +};
>
Depending on Jonathan Cameron's preference about where to put the comment,
and if he wants a v3 or not... If we want to make a v3, should I create a new thread for that?
Best Regards,
Jon LoBue
Attachment:
signature.asc
Description: This is a digitally signed message part.
