[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4503eedb949c4d56b426f1d4c0f0eed1@BLUPR03MB373.namprd03.prod.outlook.com>
Date: Mon, 18 Aug 2014 05:26:18 +0000
From: "fugang.duan@...escale.com" <fugang.duan@...escale.com>
To: Richard Cochran <richardcochran@...il.com>
CC: "davem@...emloft.net" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"shawn.guo@...aro.org" <shawn.guo@...aro.org>
Subject: RE: [PATCH v3 1/1] net: fec: ptp: avoid register access when ipg
clock is disabled
From: Richard Cochran <richardcochran@...il.com> Data: Saturday, August 16, 2014 2:09 AM
>To: Duan Fugang-B38611
>Cc: davem@...emloft.net; netdev@...r.kernel.org; shawn.guo@...aro.org
>Subject: Re: [PATCH v3 1/1] net: fec: ptp: avoid register access when ipg
>clock is disabled
>
>Looks better, but ...
>
>On Fri, Aug 15, 2014 at 01:52:55PM +0800, Fugang Duan wrote:
>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>> index 66fe1f6..ba35994 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -1613,14 +1613,18 @@ static int fec_enet_clk_enable(struct net_device
>*ndev, bool enable)
>> ret = clk_prepare_enable(fep->clk_ptp);
>> if (ret)
>> goto failed_clk_ptp;
>> + else
>> + fep->ptp_clk_on = true;
>> }
>> } else {
>> clk_disable_unprepare(fep->clk_ahb);
>> clk_disable_unprepare(fep->clk_ipg);
>> if (fep->clk_enet_out)
>> clk_disable_unprepare(fep->clk_enet_out);
>> - if (fep->clk_ptp)
>> + if (fep->clk_ptp) {
>> clk_disable_unprepare(fep->clk_ptp);
>> + fep->ptp_clk_on = false;
>
>Set the flag to false first, because this races ...
I will change it. Thanks!
If there have race, so it is better to add mutex to protect the flag ?
>
>> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
>b/drivers/net/ethernet/freescale/fec_ptp.c
>> index 82386b2..8084aaf 100644
>> --- a/drivers/net/ethernet/freescale/fec_ptp.c
>> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
>> @@ -245,6 +245,10 @@ static int fec_ptp_settime(struct ptp_clock_info
>*ptp,
>> u64 ns;
>> unsigned long flags;
>>
>> + /* Check the ptp clock */
>> + if (!fep->ptp_clk_on)
>> + return -EINVAL;
>
>with this and ...
>
>> +
>> ns = ts->tv_sec * 1000000000ULL;
>> ns += ts->tv_nsec;
>>
>> @@ -338,17 +342,20 @@ int fec_ptp_get(struct net_device *ndev, struct
>ifreq *ifr)
>> * fec_time_keep - call timecounter_read every second to avoid timer
>overrun
>> * because ENET just support 32bit counter, will
>timeout in 4s
>> */
>> -static void fec_time_keep(unsigned long _data)
>> +static void fec_time_keep(struct work_struct *work)
>> {
>> - struct fec_enet_private *fep = (struct fec_enet_private *)_data;
>> + struct delayed_work *dwork = to_delayed_work(work);
>> + struct fec_enet_private *fep = container_of(dwork, struct
>fec_enet_private, time_keep);
>> u64 ns;
>> unsigned long flags;
>>
>> - spin_lock_irqsave(&fep->tmreg_lock, flags);
>> - ns = timecounter_read(&fep->tc);
>> - spin_unlock_irqrestore(&fep->tmreg_lock, flags);
>> + if (fep->ptp_clk_on) {
>
>with this.
>
>> + spin_lock_irqsave(&fep->tmreg_lock, flags);
>> + ns = timecounter_read(&fep->tc);
>> + spin_unlock_irqrestore(&fep->tmreg_lock, flags);
>> + }
>
>Thanks,
>Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists