Hi Andy,
On Tue, Jan 17, 2023 at 05:27:13PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 17, 2023 at 02:22:42PM +0200, Sakari Ailus wrote:
> > MIPI DisCo for Imaging defines properties for sensor-adjacent devices such
> > as EEPROM, LED flash or lens VCM as either device or sub-node references.
> > This is compliant with existing DT definitions apart from property names.
> >
> > Rename parsed MIPI-defined properties so drivers will have a unified view
> > of them as defined in DT and already parsed by drivers. This can be done
> > in-place as the MIPI-defined property strings are always longer than the
> > DT one. This also results in loss of constness in parser function
> > arguments.
> >
> > Individual bindings to devices could define the references differently
> > between MIPI DisCo for Imaging and DT, in terms of device or sub-node
> > references. This will still need to be handled in the drivers themselves.
>
> ...
>
> > +static const struct mipi_disco_prop {
> > + const char *mipi_prop;
> > + const char *dt_prop;
> > +} mipi_disco_props[] = {
> > + { "mipi-img-lens-focus", "lens-focus" },
> > + { "mipi-img-flash-leds", "flash-leds" },
> > + { "mipi-img-clock-frequency", "clock-frequency" },
> > + { "mipi-img-led-max-current", "led-max-microamp" },
> > + { "mipi-img-flash-max-current", "flash-max-microamp" },
> > + { "mipi-img-flash-max-timeout", "flash-max-timeout-us" },
> > +};
>
> If we split this to 2 arrays (with static_assert() against their sizes)...
>
> ...
>
> > +void acpi_properties_prepare_mipi(union acpi_object *elements)
> > +{
> > + unsigned int i;
> > +
> > + /* Replace MIPI DisCo for Imaging property names with DT equivalents. */
> > + for (i = 0; i < ARRAY_SIZE(mipi_disco_props); i++) {
> > + if (!strcmp(mipi_disco_props[i].mipi_prop,
> > + elements[0].string.pointer)) {
>
> ...we can utilise match_string() here.
We could, yes.
I'd prefer to keep them separate still, the table is easier to maintain
that way. And I'd say it's the table above that matters more than this
function.
The current spec does not use more than these properties but I'd expect to
be added more of these in the future.
>
> > + WARN_ON(strscpy(elements[0].string.pointer,
> > + mipi_disco_props[i].dt_prop,
> > + elements[0].string.length) < 0);
> > + break;
> > + }
> > + }
> > +}
>
> ...
>
> > for (i = 0; i < properties->package.count; i++) {
> > - const union acpi_object *property;
> > + union acpi_object *property = &properties->package.elements[i];
> > + union acpi_object *elements = property->package.elements;
> >
> > - property = &properties->package.elements[i];
> > /*
> > * Only two elements allowed, the first one must be a string and
> > * the second one has to satisfy certain conditions.
> > */
> > if (property->package.count != 2
> > - || property->package.elements[0].type != ACPI_TYPE_STRING
> > - || !acpi_property_value_ok(&property->package.elements[1]))
> > + || elements[0].type != ACPI_TYPE_STRING
> > + || !acpi_property_value_ok(&elements[1]))
>
> While at it you can mode ||:s on the respective previous lines.
Yes, I'll fix that.
>
> > return false;
> > +
> > + acpi_properties_prepare_mipi(elements);
> > }
--
Kind regards,
Sakari Ailus
