[PATCH v3 2/2] phy: freescale: add Samsung HDMI PHY

[Luca Ceresoli]

Hi Lucas,

[+Cc: Sandor]

On Wed,  6 Sep 2023 20:42:11 +0200
Lucas Stach <l.stach at pengutronix.de> wrote:

> This adds the driver for the Samsung HDMI PHY found on the
> i.MX8MP SoC. Based on downstream implementation from
> Sandor Yu <Sandor.yu at nxp.com>.
> 
> Tested-by: Luca Ceresoli <luca.ceresoli at bootlin.com> (v2)

Also for v3:
[On custom board based on MSC SM2S-IMX8PLUS SMARC module]
Tested-by: Luca Ceresoli <luca.ceresoli at bootlin.com>

I have a few notes however, see below.

> +#define PHY_REG_14		0x38
> +#define  REG14_TOL_MASK		GENMASK(7, 4)
> +#define  REG14_RP_CODE_MASK	GENMASK(2, 1)

According to the latest reference manual currently available on the NXP
website (Rev. 1, 06/2021), this should be GENMASK(3, 1). This is
somewhat nitpicking as the only possible value documented is 2. But
let's continue...

> +#define PHY_REG_33		0x84
> +#define  REG33_MODE_SET_DONE	BIT(7)
> +#define  REG33_FIX_DA		BIT(1)

Here the reference manual is very different:

MODE_SET_DONE BIT(4)
TX_INV2       BIT(3)
TX_INV1       BIT(2)
TX_INV0       BIT(1)
MON_RXD       BIT(0)
bits 7-5 are reserved

...which is strange: in the code you are always writing 0 in bit 4,
which according to the docs means MODE_SET_DONE is always "Assert
forced global reset". Thus I guess your definitions come from the
downstream driver which, as it sadly happens, is more authoritative
than the docs. :-/

Sandor, can you confirm this, or provide any clarifications?

Otherwise LGTM.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com