Re: [PATCH RFC v6 2/6] dpll: Add DPLL framework base functions

Tue, May 09, 2023 at 04:52:47PM CEST, kuba@kernel.org wrote:

On Tue, 9 May 2023 09:53:07 +0200 Jiri Pirko wrote:

quoted

quoted

Yup. Even renaming EXT to something that's less.. relative :(  

Suggestion?

Well, is an SMT socket on the board an EXT pin?
Which is why I prefer PANEL.

Makes sense.
To speak code, we'll have:

/**
 * enum dpll_pin_type - defines possible types of a pin, valid values for
 *   DPLL_A_PIN_TYPE attribute
 * @DPLL_PIN_TYPE_UNSPEC: unspecified value
 * @DPLL_PIN_TYPE_MUX: aggregates another layer of selectable pins
 * @DPLL_PIN_TYPE_PANEL: physically facing user, for example on a front panel
 * @DPLL_PIN_TYPE_SYNCE_ETH_PORT: ethernet port PHY's recovered clock
 * @DPLL_PIN_TYPE_INT_OSCILLATOR: device internal oscillator
 * @DPLL_PIN_TYPE_GNSS: GNSS recovered clock
 */
enum dpll_pin_type {
        DPLL_PIN_TYPE_UNSPEC,
        DPLL_PIN_TYPE_MUX,
        DPLL_PIN_TYPE_PANEL,
        DPLL_PIN_TYPE_SYNCE_ETH_PORT,
        DPLL_PIN_TYPE_INT_OSCILLATOR,
        DPLL_PIN_TYPE_GNSS,

        __DPLL_PIN_TYPE_MAX,
        DPLL_PIN_TYPE_MAX = (__DPLL_PIN_TYPE_MAX - 1)
};

quoted

quoted

quoted

Well sure, in case there is no "label" attr for the rest of the types.
Which I believe it is, for the ice implementation in this patchset.
Otherwise, there is no way to distinguish between the pins.
To have multiple attrs for label for multiple pin types does not make
any sense to me, that was my point.  

Come on, am I really this bad at explaining this?  

Or perhaps I'm just slow.

quoted

If we make a generic "label" attribute driver authors will pack
everything they want to expose to the user into it, and then some.  

What's difference in generic label string attr and type specific label
string attr. What is stopping driver developers to pack crap in either
of these 2. Perhaps I'm missing something. Could you draw examples?

quoted

So we need attributes which will feel *obviously* *wrong* to abuse.  

Sure, I get what you say and agree. I'm just trying to find out the
actual attributes :)

PANEL label must match the name on the panel. User can take the card
into their hand, look at the front, and there should be a label/sticker/
/engraving which matches exactly what the kernel reports.

If the label is printed on the board it's a BOARD_LABEL, if it's the
name of a trace in board docs it's a BOARD_TRACE, if it's a pin of 
the ASIC it's a PACKAGE_PIN.

If it's none of those, or user does not have access to the detailed
board / pinout - don't use the label.

To speak code, we'll have:
DPLL_A_PIN_PANEL_LABEL (string)
   available always when attr[DPLL_A_PIN_TYPE] == DPLL_PIN_TYPE_PANEL
DPLL_A_PIN_BOARD_LABEL (string)
   may be available for any type, optional
DPLL_A_PIN_BOARD_TRACE (string)
   may be available for any type, optional
DPLL_A_PIN_PACKAGE_PIN (string)
   may be available for any type, optional

Makes sense?

But this does not prevent driver developer to pack random crap in the
string anyway :/