Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support

On Mon, Feb 5, 2024 at 10:11 AM Peng Fan [off-list ref] wrote:

[Me]

quoted

This is very much to the core of the problem isn't it?

Yes. Code size should be as small as possible.

This is a valid argument, I don't know how big your SCMI FW is, or
if it has to fit into some on-chip memory or so.

And using SCMI generic pinconf, there will be multiple
SCMI calls(not MMIO access), such as setting mux(ops->set_mux)
needs an SCMI call, pad settings(ops->pin_config_set) needs an SCMI call.
And maybe ops->get_function_name  needs an extra SCMI call.

With current i.MX design, only one SCMI call is used, which
use less time.

I think this could be fixed in the spec, let's see what the spec author says.

In any case: pin control is pretty much never on a hot path, a few
microseconds more or less doesn't matter. It's usually just used
and probe, suspend/resume and maybe shutdown. All usecases on
the slowpath, so I think it's a premature optimization.

quoted

Over the years I have come to regret it a bit, I think insisting on groups and
functions as strings is better for abstraction. And the point of firmware is to
abstract things so they work the same on all systems.

With current:
        pinctrl_uart1: uart1grp {
                fsl,pins = <
                        IMX95_PAD_UART1_RXD__AONMIX_TOP_LPUART1_RX      0x31e
                        IMX95_PAD_UART1_TXD__AONMIX_TOP_LPUART1_TX      0x31e
                >;
        };

It is very easy for people to know the meaning from reading reference manual.

Yes If the defines are provided it's quite readable. And I think Freescale
people really get used to it.

But from a maintenance and standardization point of view, it's nice if
everything works the same way and looks the same.

If using generic pinconf, the dt node will be like
Uartgrp {
        pins = "uart1txd", "uart1rxd";
        functions = "uart1";
        bias-xx
        drive-strength =
};

Well that looks good to me :)

It looks the same as e.g. Qualcomm or nVidia pins. So it's easy for me
to read and understand, and that's my perspective as a maintainer.

The firmware needs add more code to export functions, pack the conf settings,
each pins needs a function name per current i.MX HW logic.

Indeed, but I think any standard requires a bit of code to implement.
How much is "too much" code is a matter of taste and physical contraints.
(A PC UEFI BIOS isn't exactly small either.)

I understand your point of view, and I think it is pretty much
the same stance that the Freescale maintainers put forward for the DT
bindings for Freescale.

Yours,
Linus Walleij