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;
Message-ID: <71492ab7-9d6b-e41c-e392-1bee04860f18@bootlin.com>
Date:   Fri, 16 Jun 2023 15:48:44 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.2
Subject: Re: [PATCH net-next 3/8] net: stmmac: move PTP interrupt handling to
 IP-specific DWMAC file
To:     Simon Horman <simon.horman@corigine.com>
Cc:     Giuseppe Cavallaro <peppe.cavallaro@st.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Richard Cochran <richardcochran@gmail.com>,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-stm32@st-md-mailman.stormreply.com,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        Nicolas Carrier <nicolas.carrier@nav-timing.safrangroup.com>
References: <20230616100409.164583-1-alexis.lothore@bootlin.com>
 <20230616100409.164583-4-alexis.lothore@bootlin.com>
 <ZIxkyfRIuVcmCzmD@corigine.com>
Content-Language: en-US
From:   =?UTF-8?Q?Alexis_Lothor=c3=a9?= <alexis.lothore@bootlin.com>
In-Reply-To: <ZIxkyfRIuVcmCzmD@corigine.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hello Simon,

On 6/16/23 15:34, Simon Horman wrote:
> On Fri, Jun 16, 2023 at 12:04:04PM +0200, alexis.lothore@bootlin.com wrote:
>> From: Alexis Lothoré <alexis.lothore@bootlin.com>
>>
>> As for auxiliary snapshot triggers configuration, reading snapshots depends
>> on specific registers addresses and layout. As a consequence, move
>> PTP-specific part of stmmac interrupt handling to specific DWMAC IP file
>>
>> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> 
> Hi Alexis,
> 
> thanks for your patch.
> 
> ...
> 
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>> index 01c0822d37e6..b36fbb0fa5da 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> 
> ...
> 
>> +static void dwmac4_ptp_isr(struct stmmac_priv *priv)
>> +{
>> +	u32 num_snapshot, ts_status;
>> +	struct ptp_clock_event event;
>> +	unsigned long flags;
>> +	u64 ptp_time;
>> +	int i;
> 
> Please use reverse xmas tree - longest line to shortest - for new
> Networking code.
> 
> 	struct ptp_clock_event event;
> 	u32 num_snapshot, ts_status;
> 	unsigned long flags;
> 	u64 ptp_time;
> 	int i;
> 
ACK
> ...
> 
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
>> index 9e0ff2cec352..92ed421702b9 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
>> @@ -23,9 +23,6 @@
>>  #define	PTP_STSUR	0x10	/* System Time – Seconds Update Reg */
>>  #define	PTP_STNSUR	0x14	/* System Time – Nanoseconds Update Reg */
>>  #define	PTP_TAR		0x18	/* Timestamp Addend Reg */
>> -#define	PTP_ACR		0x40	/* Auxiliary Control Reg */
> 
> Unfortunately this seems to break the build of
> drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> on an x86_64 alllodconfig, as PTP_ACR is used in that file.
> 
>> -#define	PTP_ATNR	0x48	/* Auxiliary Timestamp - Nanoseconds Reg */
>> -#define	PTP_ATSR	0x4c	/* Auxiliary Timestamp - Seconds Reg */
>>  
>>  #define	PTP_STNSUR_ADDSUB_SHIFT	31
>>  #define	PTP_DIGITAL_ROLLOVER_MODE	0x3B9ACA00	/* 10e9-1 ns */

Ouch. thanks for spotting the issue and the providing build details, I'll fix
this and wait a bit for more comments before sending v2
> 
> --
> pw-bot: changes-requested
> 

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com