Re: [PATCH v4 3/5] soc: fsl: guts: add the RCW override of any RCWSR

[Vladimir Oltean]

Hi Vincent,

On Fri, Nov 13, 2026 at 05:39:05PM +0100, Vincent Jardin wrote:
> Hi Christophe, or Vladimir,
> 
> Le 08/11/26 11:06, Vincent Jardin via B4 Relay a écrit :
> > From: Vincent Jardin <vjardin@xxxxxxx>
> > 
> > fsl_guts_rcw_rmw() only serves the SerDes lane modes. Export it for
> > other fields of the RCW:
> >  - fsl_guts_rcw_read() reads a status register
> >  - fsl_guts_rcw_override() changes fields of one under rcwcr_lock and
> >    waits for the status register to follow
> >  - fsl_guts_rcw_override_supported() tells whether the DCSR view of DCFG,
> >    which holds the override registers, is mapped
> 
> May I ask for your advise on this topic ?
> 
> I am looking for the review of this serie, then I'll adapt it
> for u-boot along with the integration of Linux.
> 
> thank you,
>   Vincent

Sorry for the late review. Responding here to your question on v3:
https://lore.kernel.org/lkml/avc9KbxQ4dxlibGQ@L30177.local/

I cannot agree with your API proposal. It exports to the rest of the
kernel a blanket API for RCW override of any RCWSR1-RCWSR31 register,
*including* the SerDes fields for which the fsl_guts module already
exposes an API. So with your proposal, what can be done with
fsl_guts_lane_set_mode() can also be (un)done with fsl_guts_rcw_override().
The possibilities of abusing this API design are endless, and I don't
want to have to deal with them when I can avoid them.

At a minimum, I would recommend renaming the API as fsl_guts_pmux_override(),
and make guts enforce which register fields it permits overriding via
this API. Example:

static const struct fsl_soc_pmux_mask lx2160a_pmux[] = {
	{ 12, GENMASK(29, 0) }, /* IIC2..SDHC1_DIR */
	{ 13, GENMASK(29, 0) }, /* SDHC2_DAT74..EVT43 */
	{ 14, GENMASK(10, 0) }, /* EVT20..IIC1 */
	{ 27, GENMASK(7, 0) },  /* EC1, EC2, USB_EXT, GTX_CLK, SDHC1_DS <- your series is missing these */
	{}
};