On Wed, Feb 21, 2024 at 8:56 AM Sakari Ailus
<sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> I think Rafael has already applied this but I can send a new patch...
Depends if it's final or can be dropped.
If the former is the case, follow ups, please.
> On Wed, Feb 21, 2024 at 12:21:13AM +0200, andy.shevchenko@xxxxxxxxx wrote:
> > Tue, Feb 13, 2024 at 03:46:06PM +0200, Sakari Ailus kirjoitti:
...
> > > void acpi_mipi_scan_crs_csi2(void);
> > > void acpi_mipi_init_crs_csi2_swnodes(void);
> > > void acpi_mipi_crs_csi2_cleanup(void);
> >
> > + blank line?
>
> The usa of blank lines elsewhere in the file is consistent with the above.
> These are all related.
Naming does not suggest that. I see either naming or semantic tights
issues here. Hence the proposal to have a blank line.
> > > +bool acpi_graph_ignore_port(acpi_handle handle);
...
> > > +static const struct dmi_system_id dmi_ignore_port_nodes[] = {
> > > + {
> > > + .matches = {
> > > + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > > + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "XPS 9315"),
> > > + },
> > > + },
> > > + { 0 }
> >
> > 0 is not needed. Moreover, it's a bit unusual.
>
> But also required by the C standard.
We have a lot of code that doesn't use that (yet this is valid C23,
and yes I know that we rely on C11).
> > > +};
...
> > > +static const char *strnext(const char *s1, const char *s2)
> > > +{
> > > + s1 = strstr(s1, s2);
> > > +
> > > + if (!s1)
> > > + return NULL;
> > > +
> > > + return s1 + strlen(s2);
> > > +}
> >
> > NIH str_has_prefix() ?
>
> It doesn't do the same thing. Yes, it could be used, but the code looks
> cleaner with this.
"Not Invented Here" was even at Nokia times, why do we repeat our mistakes?
...
> > > +/**
> > > + * acpi_graph_ignore_port - Tell whether a port node should be ignored
> > > + * @handle: The ACPI handle of the node (which may be a port node)
> > > + *
> > > + * Returns true if a port node should be ignored and the data to that should
> >
> > Please, run kernel-doc validation and fix the warnings.
>
> Running kernel-doc on the file doesn't produce any here.
You haven't run it correctly?
$ scripts/kernel-doc -v -none -Wall drivers/acpi/mipi-disco-img.c
drivers/acpi/mipi-disco-img.c:163: info: Scanning doc for function
acpi_mipi_check_crs_csi2
drivers/acpi/mipi-disco-img.c:384: info: Scanning doc for function
acpi_mipi_scan_crs_csi2
drivers/acpi/mipi-disco-img.c:703: info: Scanning doc for function
acpi_mipi_init_crs_csi2_swnodes
drivers/acpi/mipi-disco-img.c:718: info: Scanning doc for function
acpi_mipi_crs_csi2_cleanup
drivers/acpi/mipi-disco-img.c:749: info: Scanning doc for function
acpi_graph_ignore_port
drivers/acpi/mipi-disco-img.c:759: warning: No description found for
return value of 'acpi_graph_ignore_port'
1 warnings
> > > + * come from other sources instead (Windows ACPI definitions and
> > > + * ipu-bridge). This is currently used to ignore bad port nodes related to IPU6
> > > + * ("IPU?") and camera sensor devices ("LNK?") in certain Dell systems with
> > > + * Intel VSC.
> > > + */
--
With Best Regards,
Andy Shevchenko
