Message-ID: <20230509105302.3703216c@kernel.org>
Date: Tue, 9 May 2023 10:53:02 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jiri Pirko <jiri@...nulli.us>
Cc: Paolo Abeni <pabeni@...hat.com>, "Kubalewski, Arkadiusz"
 <arkadiusz.kubalewski@...el.com>, Vadim Fedorenko
 <vadim.fedorenko@...ux.dev>, Vadim Fedorenko <vadfed@...a.com>, Jonathan
 Lemon <jonathan.lemon@...il.com>, poros <poros@...hat.com>, mschmidt
 <mschmidt@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 linux-arm-kernel@...ts.infradead.org, "linux-clk@...r.kernel.org"
 <linux-clk@...r.kernel.org>, "Olech, Milena" <milena.olech@...el.com>,
 "Michalik, Michal" <michal.michalik@...el.com>
Subject: Re: [PATCH RFC v6 2/6] dpll: Add DPLL framework base functions

On Tue, 9 May 2023 17:21:42 +0200 Jiri Pirko wrote:
> Tue, May 09, 2023 at 04:52:47PM CEST, kuba@...nel.org wrote:
> >On Tue, 9 May 2023 09:53:07 +0200 Jiri Pirko wrote:  
> >> >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)
> };

Maybe we can keep the EXT here, just not in the label itself.
Don't think we care to add pin type for PANEL vs SMT vs jumper?

> >> 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

Not sure about always, if there's only one maybe there's no need 
to provide the label?

> 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?

yup (obviously we need to document the semantics)

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

It doesn't but it hopefully makes it much more likely that (1) reviewer
will notice that something is off if the driver printfs random crap;
and (2) that the user reading the documentation will complain that 
e.g.BOARD_LABEL is used but does not match the label on they see...

Powered by blists - more mailing lists