[PATCH] phy: xilinx: zynqmp: Fix bus width setting for SGMII

[Laurent Pinchart]

Hi Robert,

Thank you for the patch.

On Tue, Jan 25, 2022 at 06:16:00PM -0600, Robert Hancock wrote:
> TX_PROT_BUS_WIDTH and RX_PROT_BUS_WIDTH are single registers with
> separate bit fields for each lane. The code in xpsgtr_phy_init_sgmii was
> not preserving the existing register value for other lanes, so enabling
> the PHY in SGMII mode on one lane zeroed out the settings for all other
> lanes, causing other PS-GTR peripherals such as USB3 to malfunction.
> 
> Use xpsgtr_clr_set to only manipulate the desired bits in the register.
> 
> Fixes: 4a33bea00314 ("phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver")
> Signed-off-by: Robert Hancock <robert.hancock at calian.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  drivers/phy/xilinx/phy-zynqmp.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index f478d8a17115..9be9535ad7ab 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -134,7 +134,8 @@
>  #define PROT_BUS_WIDTH_10		0x0
>  #define PROT_BUS_WIDTH_20		0x1
>  #define PROT_BUS_WIDTH_40		0x2
> -#define PROT_BUS_WIDTH_SHIFT		2
> +#define PROT_BUS_WIDTH_SHIFT(n)		((n) * 2)
> +#define PROT_BUS_WIDTH_MASK(n)		GENMASK((n) * 2 + 1, (n) * 2)
>  
>  /* Number of GT lanes */
>  #define NUM_LANES			4
> @@ -445,12 +446,12 @@ static void xpsgtr_phy_init_sata(struct xpsgtr_phy *gtr_phy)
>  static void xpsgtr_phy_init_sgmii(struct xpsgtr_phy *gtr_phy)
>  {
>  	struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
> +	u32 mask = PROT_BUS_WIDTH_MASK(gtr_phy->lane);
> +	u32 val = PROT_BUS_WIDTH_10 << PROT_BUS_WIDTH_SHIFT(gtr_phy->lane);
>  
>  	/* Set SGMII protocol TX and RX bus width to 10 bits. */
> -	xpsgtr_write(gtr_dev, TX_PROT_BUS_WIDTH,
> -		     PROT_BUS_WIDTH_10 << (gtr_phy->lane * PROT_BUS_WIDTH_SHIFT));
> -	xpsgtr_write(gtr_dev, RX_PROT_BUS_WIDTH,
> -		     PROT_BUS_WIDTH_10 << (gtr_phy->lane * PROT_BUS_WIDTH_SHIFT));
> +	xpsgtr_clr_set(gtr_dev, TX_PROT_BUS_WIDTH, mask, val);
> +	xpsgtr_clr_set(gtr_dev, RX_PROT_BUS_WIDTH, mask, val);
>  
>  	xpsgtr_bypass_scrambler_8b10b(gtr_phy);
>  }

-- 
Regards,

Laurent Pinchart