Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 16 Jun 2023 21:35:40 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     "Radu Pirea (NXP OSS)" <radu-nicolae.pirea@oss.nxp.com>
Cc:     andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, richardcochran@gmail.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        sebastian.tobuschat@nxp.com
Subject: Re: [PATCH net-next v1 05/14] net: phy: nxp-c45-tja11xx: prepare the
 ground for TJA1120
Message-ID: <ZIy5jBAS66rqXjAd@corigine.com>
References: <20230616135323.98215-1-radu-nicolae.pirea@oss.nxp.com>
 <20230616135323.98215-6-radu-nicolae.pirea@oss.nxp.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230616135323.98215-6-radu-nicolae.pirea@oss.nxp.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?AwO6JHs7wdqKQ1NM2NLIwv/NJ+sMI0v7tmi8avqIwMvi9e8ztxh3iv3I2eIS?=
 =?us-ascii?Q?D60EJijFVpeo8JHmMsmbD/M2pRyAPVXsaJFqgM+wf3AW395IQ9Pd/Pq9JBCs?=
 =?us-ascii?Q?Ptd38TXYsflcLK6CJ7dXF//MKEJ3rjRN0OUEiPm6E5nt6nXNfSCiodwui90U?=
 =?us-ascii?Q?N43KOkMUuT+NeXa48AGbKvhraAr1xB+/Elkk3kSCsxGowHcuby4iTftFRrgA?=
 =?us-ascii?Q?hlR4baJMs/hxnNTLhaaiHTG8fUSMUf0Oo9E1Do1oT7d18q3cOAXJfA0BGvsd?=
 =?us-ascii?Q?SVbdV0hIKvSb6bRmfF0wRcMEXU0I07T78JWUelX0YOSnwQeltnT5geim4e0X?=
 =?us-ascii?Q?Iv4Uy4D7tBwxHt5orrw/FofUcnCH+3TyRZNDF6u8Gg2b5ayLCGduE2TMSfOV?=
 =?us-ascii?Q?bbuOfPOd+XnEStk/WYPAWPWNxmsGukH0V53RvGIGDyqW8z1Jsudsh9UQFrk6?=
 =?us-ascii?Q?4WHAHOq3+SY86OHK3rYj8U4dG1DEHor44Dw3l+ZKlA5UX2iYiZ5FIXvXRIlZ?=
 =?us-ascii?Q?S92uu/YkwT+DdvTcO+b2V2qJGyrPRy585XrEZzJpG2NwOUJV4izDTQstxL6e?=
 =?us-ascii?Q?R0c6KrPajydDSW7x1aYRD5rl/xIloDBLytm6hd2Nbasfh8bvvinxBf2+v6C5?=
 =?us-ascii?Q?7IwhCbyO5SYN/uY4XTEp1qEXOAC6xO8T/rltoJDBLAg5yJWlirBU7ydSYad7?=
 =?us-ascii?Q?1HIW1j/ul6lBeBOB/LNHTmR0xRrPuJawFMGHaRPY9imjCdi0Pgw8sctGc5vF?=
 =?us-ascii?Q?iJKEE7Tu2drmrKZwjCStFiBuCiceCyKlWh3X04wJ3PoHRtEp3gOe+7aA0So5?=
 =?us-ascii?Q?Msme2eGRIn6puH6kONiJ1qJG5yiD5wf1zu5EE1WzXkKo51JN7o9ieb4L7iz/?=
 =?us-ascii?Q?UoY4fnR8p5CMD8uZsxl/w8V2umpe1Kb3qgwc7Bj6PKwnzpoiKpam87g+TplB?=
 =?us-ascii?Q?GvDI3+wdm+t2oswX0UEe9u0U3/jYGmX0vRXpxqnfPhoAbOPCB+R+zuEmwyDa?=
 =?us-ascii?Q?9v28WnNarryTtkrCyNNw3YCHhs/Xa+bIRBiAPY3joqkIWEqVLbGk3q/FnG/S?=
 =?us-ascii?Q?pbFglkfvVnYTOp09KmKkOjYXoxMDByVo+guEjtvzN82HCwBbv0E/+889ghBl?=
 =?us-ascii?Q?c5Y8in5AgXL9BKur++7assB/wIL6SPKuBBqj4wZKoK4yfxNpKpiMlAvCawVa?=
 =?us-ascii?Q?QVqXQBNQzQNBNnRFtEGCo8WCTmcXZD1bI9ERqAtw9lRPghi6cdu7KP65ks1R?=
 =?us-ascii?Q?VokX7cfaG00U0j0BrKaibsd4ILpHL6FY6ZUfDmOo8fd1+yClnoG65cjpUPve?=
 =?us-ascii?Q?pd52nOp73hI7AIX/jvHUgMGcnlMHNz9OcMU8ZZ8Ev9ZFzvfBkvvzyxfdIAj3?=
 =?us-ascii?Q?2u9KuAhMC4sCcFP820f4X9lrqzjo2jgikQ1kh9McGE2VHJNNqeKK1pqO2HfM?=
 =?us-ascii?Q?SbxqUlmLICEzIklysArwlAsdtb/GUXRiSMW8geCby7LOicNwaQ3p6rQUuRl8?=
 =?us-ascii?Q?DM5P6KiFn7hWqoFvYYyFhw69oJYgS5rcbohFVNv7g92whlXwRyRq51pDNf0A?=
 =?us-ascii?Q?uuKNS5cY1rDmXlaZJPmoyy3z7NJGlf/CE1Ya+nepI08t5CwHPf1N/b9RNsSJ?=
 =?us-ascii?Q?TKfI2sDSC6Ftw+U7xCOusbkfVMAuimDVoEBr3+SythUp+wrhHKNjVpwkDPM5?=
 =?us-ascii?Q?9V/I7w=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e3247974-ce36-4ccf-c7fc-08db6ea0e248
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jun 2023 19:35:48.2692
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Vz1XCWAgwFRAbBbyJuR0QUJvHaKftVcce6fXDHdr31Q6caocNxFhP+7jLa5ft38hAr9qyE6hALMT1N3YlBBV69CZ1+4fQhlw386xRvrH+VA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR13MB3896
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Jun 16, 2023 at 04:53:14PM +0300, Radu Pirea (NXP OSS) wrote:
> Remove the defined bits and register addresses that are not common
> between TJA1103 and TJA1120 and replace them with reg_fields and
> register addresses from phydev->drv->driver_data.
> 
> Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com>

Hi Radu,

thanks for your patch-set.

Some minor points from my side.

> @@ -831,27 +862,26 @@ static int nxp_c45_hwtstamp(struct mii_timestamper *mii_ts,
>  	}
>  
>  	if (priv->hwts_rx || priv->hwts_tx) {
> -		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_EVENT_MSG_FILT,
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +			      data->regmap->vend1_event_msg_filt,
>  			      EVENT_MSG_FILT_ALL);
> -		phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1,
> -				   VEND1_PORT_PTP_CONTROL,
> -				   PORT_PTP_CONTROL_BYPASS);
> +		if (data && data->ptp_enable)
> +			data->ptp_enable(phydev, true);

A check for data being null is made here before dereferencing.
But a few lines above data is dereferenced without such a guard.

This is flagged by Smatch as:

 .../nxp-c45-tja11xx.c:868 nxp_c45_hwtstamp() warn: variable dereferenced before check 'data' (see line 866)`

>  	} else {
> -		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_EVENT_MSG_FILT,
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +			      data->regmap->vend1_event_msg_filt,
>  			      EVENT_MSG_FILT_NONE);
> -		phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, VEND1_PORT_PTP_CONTROL,
> -				 PORT_PTP_CONTROL_BYPASS);
> +		if (data && data->ptp_enable)
> +			data->ptp_enable(phydev, false);

Likewise here.

>  	}
>  
>  	if (nxp_c45_poll_txts(priv->phydev))
>  		goto nxp_c45_no_ptp_irq;
>  
>  	if (priv->hwts_tx)
> -		phy_set_bits_mmd(phydev, MDIO_MMD_VEND1,
> -				 VEND1_PTP_IRQ_EN, PTP_IRQ_EGR_TS);
> +		nxp_c45_set_reg_field(phydev, &data->regmap->irq_egr_ts_en);
>  	else
> -		phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1,
> -				   VEND1_PTP_IRQ_EN, PTP_IRQ_EGR_TS);
> +		nxp_c45_clear_reg_field(phydev, &data->regmap->irq_egr_ts_en);
>  
>  nxp_c45_no_ptp_irq:
>  	return copy_to_user(ifreq->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;

...

> +static void nxp_c45_ptp_init(struct phy_device *phydev)
> +{
> +	const struct nxp_c45_phy_data *data = nxp_c45_get_data(phydev);
> +
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +		      data->regmap->vend1_ptp_clk_period,
> +		      data->ptp_clk_period);
> +	nxp_c45_clear_reg_field(phydev, &data->regmap->ltc_lock_ctrl);
> +
> +	if (data && data->ptp_init)
> +		data->ptp_init(phydev);

And here.

> +}
> +

...