Re: [PATCH net-next] net: phy: micrel: Add support for non PTP SKUs for lan8814

[Gerhard Engleder <gerhard@engleder-embedded.com>]

On 17.10.25 09:47, Horatiu Vultur wrote:
> The lan8814 has 4 different SKUs and for 2 of these SKUs the PTP is
> disabled. All these SKUs have the same value in the register 2 and 3
> meaning we can't differentiate them based on device id therefore check

Did you miss to start a new sentence?

> the SKU register and based on this allow or not to create a PTP device.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>   drivers/net/phy/micrel.c | 38 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 79ce3eb6752b6..16855bf8c3916 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -101,6 +101,8 @@
>   #define LAN8814_CABLE_DIAG_VCT_DATA_MASK	GENMASK(7, 0)
>   #define LAN8814_PAIR_BIT_SHIFT			12
>   
> +#define LAN8814_SKUS				0xB
> +
>   #define LAN8814_WIRE_PAIR_MASK			0xF
>   
>   /* Lan8814 general Interrupt control/status reg in GPHY specific block. */
> @@ -367,6 +369,9 @@
>   
>   #define LAN8842_REV_8832			0x8832
>   
> +#define LAN8814_REV_LAN8814			0x8814
> +#define LAN8814_REV_LAN8818			0x8818
> +
>   struct kszphy_hw_stat {
>   	const char *string;
>   	u8 reg;
> @@ -449,6 +454,7 @@ struct kszphy_priv {
>   	bool rmii_ref_clk_sel;
>   	bool rmii_ref_clk_sel_val;
>   	bool clk_enable;
> +	bool is_ptp_available;
>   	u64 stats[ARRAY_SIZE(kszphy_hw_stats)];
>   	struct kszphy_phy_stats phy_stats;
>   };
> @@ -4130,6 +4136,17 @@ static int lan8804_config_intr(struct phy_device *phydev)
>   	return 0;
>   }
>   
> +/* Check if the PHY has 1588 support. There are multiple skus of the PHY and
> + * some of them support PTP while others don't support it. This function will
> + * return true is the sku supports it, otherwise will return false.
> + */

Hasn't net also switched to the common kernel multiline comment style
starting with an empty line?

> +static bool lan8814_has_ptp(struct phy_device *phydev)
> +{
> +	struct kszphy_priv *priv = phydev->priv;
> +
> +	return priv->is_ptp_available;
> +}
> +
>   static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
>   {
>   	int ret = IRQ_NONE;
> @@ -4146,6 +4163,9 @@ static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
>   		ret = IRQ_HANDLED;
>   	}
>   
> +	if (!lan8814_has_ptp(phydev))
> +		return ret;
> +
>   	while (true) {
>   		irq_status = lanphy_read_page_reg(phydev, LAN8814_PAGE_PORT_REGS,
>   						  PTP_TSU_INT_STS);
> @@ -4207,6 +4227,9 @@ static void lan8814_ptp_init(struct phy_device *phydev)
>   	    !IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING))
>   		return;
>   
> +	if (!lan8814_has_ptp(phydev))
> +		return;
> +
>   	lanphy_write_page_reg(phydev, LAN8814_PAGE_PORT_REGS,
>   			      TSU_HARD_RESET, TSU_HARD_RESET_);
>   
> @@ -4336,6 +4359,9 @@ static int __lan8814_ptp_probe_once(struct phy_device *phydev, char *pin_name,
>   
>   static int lan8814_ptp_probe_once(struct phy_device *phydev)
>   {
> +	if (!lan8814_has_ptp(phydev))
> +		return 0;
> +
>   	return __lan8814_ptp_probe_once(phydev, "lan8814_ptp_pin",
>   					LAN8814_PTP_GPIO_NUM);
>   }
> @@ -4450,6 +4476,18 @@ static int lan8814_probe(struct phy_device *phydev)
>   	devm_phy_package_join(&phydev->mdio.dev, phydev,
>   			      addr, sizeof(struct lan8814_shared_priv));
>   
> +	/* There are lan8814 SKUs that don't support PTP. Make sure that for
> +	 * those skus no PTP device is created. Here we check if the SKU
> +	 * supports PTP.
> +	 */

Check comment style.

> +	err = lanphy_read_page_reg(phydev, LAN8814_PAGE_COMMON_REGS,
> +				   LAN8814_SKUS);
> +	if (err < 0)
> +		return err;
> +
> +	priv->is_ptp_available = err == LAN8814_REV_LAN8814 ||
> +				 err == LAN8814_REV_LAN8818;
> +
>   	if (phy_package_init_once(phydev)) {
>   		err = lan8814_release_coma_mode(phydev);
>   		if (err)