Patchwork [4/4] FEC: Add time stamping code and a PTP hardware clock

login
register
mail settings
Submitter Shawn Guo
Date Dec. 17, 2012, 3:14 p.m.
Message ID <20121217151445.GA17907@S2101-09.ap.freescale.net>
Download mbox | patch
Permalink /patch/206894/
State RFC
Delegated to: David Miller
Headers show

Comments

Shawn Guo - Dec. 17, 2012, 3:14 p.m.
Hi Sascha,

On Mon, Dec 17, 2012 at 10:48:31PM +0800, Frank Li wrote:
> > I don't know how to continue from here. Since the whole patch doesn't
> > seem to reviewed very much I tend to say we should revert it for now and
> > let Frank redo it for the next merge window.
> >
> > Other opinions?
> 
> Can we just disable CONFIG_FEC_PTP defaut instead of revert whole patch?
> 
To be clear, the following is what Frank meant.  Since Frank is out of
office for some time, I will send this immediate fix to David, if you
are fine with it.

Shawn



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sascha Hauer - Dec. 17, 2012, 8:02 p.m.
On Mon, Dec 17, 2012 at 11:14:46PM +0800, Shawn Guo wrote:
> Hi Sascha,
> 
> On Mon, Dec 17, 2012 at 10:48:31PM +0800, Frank Li wrote:
> > > I don't know how to continue from here. Since the whole patch doesn't
> > > seem to reviewed very much I tend to say we should revert it for now and
> > > let Frank redo it for the next merge window.
> > >
> > > Other opinions?
> > 
> > Can we just disable CONFIG_FEC_PTP defaut instead of revert whole patch?
> > 
> To be clear, the following is what Frank meant.  Since Frank is out of
> office for some time, I will send this immediate fix to David, if you
> are fine with it.

This leaves an option in the tree which can be used to break FEC on
i.MX3/5.

	depends on !SOC_IMX31 && !SOC_IMX35 && !SOC_IMX5

might be an option, but given that this patch seems to have bypassed any
review I feel more like reverting it.

Sascha
Richard Cochran - Dec. 18, 2012, 7:04 a.m.
On Mon, Dec 17, 2012 at 09:02:32PM +0100, Sascha Hauer wrote:
> This leaves an option in the tree which can be used to break FEC on
> i.MX3/5.
> 
> 	depends on !SOC_IMX31 && !SOC_IMX35 && !SOC_IMX5
> 
> might be an option, but given that this patch seems to have bypassed any
> review I feel more like reverting it.

Instead of reverting, I suggest finding a solution (Frank) to let the
code work when it can work and to prevent it when it cannot. This
could be kconfig, DT, or run time probing of silicon revisions, but I
don't have access to this hardware, and so I can't really say how to
fix it.

Just for the record, I did in fact review this patch, and I commented
on exactly this point. Frank said we would address this point, and he
did so. Not knowing the imx family very well, I took his word for it.
After all, Frank has a Freescale address, and I would expect a
Freescale employee to know how to provide the right fix.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Frank Li - Dec. 18, 2012, 8:51 a.m.
>>
>> might be an option, but given that this patch seems to have bypassed any
>> review I feel more like reverting it.
>
> Instead of reverting, I suggest finding a solution (Frank) to let the
> code work when it can work and to prevent it when it cannot. This
> could be kconfig, DT, or run time probing of silicon revisions, but I
> don't have access to this hardware, and so I can't really say how to
> fix it.

I am traveling in this week. I will try to find out the solution after
come back office
in next week. But quick solution is shawn's patch, which resolve mx3
and mx5 problem at least in short term.

>
> Just for the record, I did in fact review this patch, and I commented
> on exactly this point. Frank said we would address this point, and he
> did so. Not knowing the imx family very well, I took his word for it.
> After all, Frank has a Freescale address, and I would expect a
> Freescale employee to know how to provide the right fix.
>
> Thanks,
> Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - Dec. 19, 2012, 3:53 p.m.
On Tue, Dec 18, 2012 at 08:04:20AM +0100, Richard Cochran wrote:
> On Mon, Dec 17, 2012 at 09:02:32PM +0100, Sascha Hauer wrote:
> > This leaves an option in the tree which can be used to break FEC on
> > i.MX3/5.
> > 
> > 	depends on !SOC_IMX31 && !SOC_IMX35 && !SOC_IMX5
> > 
> > might be an option, but given that this patch seems to have bypassed any
> > review I feel more like reverting it.
> 
> Instead of reverting, I suggest finding a solution (Frank) to let the
> code work when it can work and to prevent it when it cannot. This
> could be kconfig, DT, or run time probing of silicon revisions, but I
> don't have access to this hardware, and so I can't really say how to
> fix it.
[...]

Please implement run-time probing.  A different configuration for
each SoC is just not sustainable for distributions.

Ben.

Patch

diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index 5ba6e1c..d1edb2e 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -96,7 +96,6 @@  config FEC_PTP
        bool "PTP Hardware Clock (PHC)"
        depends on FEC && ARCH_MXC
        select PTP_1588_CLOCK
-       default y if SOC_IMX6Q
        --help---
          Say Y here if you want to use PTP Hardware Clock (PHC) in the
          driver.  Only the basic clock operations have been implemented.