Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Corentin Labbe]

On Sat, Jul 01, 2017 at 02:42:14PM -0700, Florian Fainelli wrote:
> On 30/06/2017 23:53, Corentin Labbe wrote:
> > On Tue, Jun 27, 2017 at 10:37:34AM -0700, Florian Fainelli wrote:
> >> On 06/27/2017 10:29 AM, Maxime Ripard wrote:
> >>> On Tue, Jun 27, 2017 at 02:37:48PM +0200, Corentin Labbe wrote:
>  On Tue, Jun 27, 2017 at 11:33:56AM +0100, Andre Przywara wrote:
> > Hi,
> >
> > On 27/06/17 11:23, Icenowy Zheng wrote:
> >>
> >>
> >> 于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara 
> >>  写到:
> >>> Hi,
> >>>
> >>> On 27/06/17 10:41, Maxime Ripard wrote:
>  On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
> > Hi,
> >
> > (CC:ing some people from that Rockchip dmwac series)
> >
> > On 27/06/17 09:21, Corentin Labbe wrote:
> >> On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> >>> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
> >>>  wrote:
>  On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
> > On 31/05/17 08:18, Corentin Labbe wrote:
> >> The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> >> allwinner.
> >> In fact the only common part is the descriptor management and
> >>> the first
> >> register function.
> >
> > Hi,
> >
> > I know I am a bit late with this, but while adapting the U-Boot
> >>> driver
> > to the new binding I was wondering about the internal PHY
> >>> detection:
> >
> >
> > So here you seem to deduce the usage of the internal PHY by the
> >>> PHY
> > interface specified in the DT (MII = internal, RGMII =
> >>> external).
> > I think I raised this question before, but isn't it perfectly
> >>> legal for
> > a board to use MII with an external PHY even on those SoCs that
> >>> feature
> > an internal PHY?
> > On the first glance that does not make too much sense, but apart
> >>> from
> > not being the correct binding to describe all of the SoCs
> >>> features I see
> > two scenarios:
> > 1) A board vendor might choose to not use the internal PHY
> >>> because it
> > has bugs, lacks features (configurability) or has other issues.
> >>> For
> > instance I have heard reports that the internal PHY makes the
> >>> SoC go
> > rather hot, possibly limiting the CPU frequency. By using an
> >>> external
> > MII PHY (which are still cheaper than RGMII PHYs) this can be
> >>> avoided.
> > 2) A PHY does not necessarily need to be directly connected to
> > magnetics. Indeed quite some boards use (RG)MII to connect to a
> >>> switch
> > IC or some other network circuitry, for instance fibre
> >>> connectors.
> >
> > So I was wondering if we would need an explicit:
> >   allwinner,use-internal-phy;
> > boolean DT property to signal the usage of the internal PHY?
> > Alternatively we could go with the negative version:
> >   allwinner,disable-internal-phy;
> >
> > Or what about introducing a new "allwinner,internal-mii-phy"
> >>> compatible
> > string for the *PHY* node and use that?
> >
> > I just want to avoid that we introduce a binding that causes us
> > headaches later. I think we can still fix this with a followup
> >>> patch
> > before the driver and its binding hit a release kernel.
> >
> > Cheers,
> > Andre.
> >
> 
>  I just see some patch, where "phy-mode = internal" is valid.
>  I will try to find a way to use it
> >>>
> >>> Can you provide a link?
> >>
> >> https://lkml.org/lkml/2017/6/23/479
> >>
> >>>
> >>> I'm not a fan of using phy-mode for this. There's no guarantee
> >>> what
> >>> mode the internal PHY uses. That's what phy-mode is for.
> >
> > I can understand Chen-Yu's concerns, but ...
> >
> >> For each soc the internal PHY mode is know and setted in
> >>> emac_variant/internal_phy
> >> So its not a problem.
> >
> > that is true as well, at least for now.
> >
> > So while I agree that having a separate property to indicate
> > the usage of the internal PHY would be nice, I am bit tempted
> > to use this easier approach and piggy back on the existing
> > phy-mode property.
> 
>  We're trying to fix an issue that works for now 

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Florian Fainelli]

On 30/06/2017 23:53, Corentin Labbe wrote:
> On Tue, Jun 27, 2017 at 10:37:34AM -0700, Florian Fainelli wrote:
>> On 06/27/2017 10:29 AM, Maxime Ripard wrote:
>>> On Tue, Jun 27, 2017 at 02:37:48PM +0200, Corentin Labbe wrote:
 On Tue, Jun 27, 2017 at 11:33:56AM +0100, Andre Przywara wrote:
> Hi,
>
> On 27/06/17 11:23, Icenowy Zheng wrote:
>>
>>
>> 于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara 
>>  写到:
>>> Hi,
>>>
>>> On 27/06/17 10:41, Maxime Ripard wrote:
 On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
> Hi,
>
> (CC:ing some people from that Rockchip dmwac series)
>
> On 27/06/17 09:21, Corentin Labbe wrote:
>> On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
>>> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>>>  wrote:
 On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
> On 31/05/17 08:18, Corentin Labbe wrote:
>> The dwmac-sun8i is a heavy hacked version of stmmac hardware by
>> allwinner.
>> In fact the only common part is the descriptor management and
>>> the first
>> register function.
>
> Hi,
>
> I know I am a bit late with this, but while adapting the U-Boot
>>> driver
> to the new binding I was wondering about the internal PHY
>>> detection:
>
>
> So here you seem to deduce the usage of the internal PHY by the
>>> PHY
> interface specified in the DT (MII = internal, RGMII =
>>> external).
> I think I raised this question before, but isn't it perfectly
>>> legal for
> a board to use MII with an external PHY even on those SoCs that
>>> feature
> an internal PHY?
> On the first glance that does not make too much sense, but apart
>>> from
> not being the correct binding to describe all of the SoCs
>>> features I see
> two scenarios:
> 1) A board vendor might choose to not use the internal PHY
>>> because it
> has bugs, lacks features (configurability) or has other issues.
>>> For
> instance I have heard reports that the internal PHY makes the
>>> SoC go
> rather hot, possibly limiting the CPU frequency. By using an
>>> external
> MII PHY (which are still cheaper than RGMII PHYs) this can be
>>> avoided.
> 2) A PHY does not necessarily need to be directly connected to
> magnetics. Indeed quite some boards use (RG)MII to connect to a
>>> switch
> IC or some other network circuitry, for instance fibre
>>> connectors.
>
> So I was wondering if we would need an explicit:
>   allwinner,use-internal-phy;
> boolean DT property to signal the usage of the internal PHY?
> Alternatively we could go with the negative version:
>   allwinner,disable-internal-phy;
>
> Or what about introducing a new "allwinner,internal-mii-phy"
>>> compatible
> string for the *PHY* node and use that?
>
> I just want to avoid that we introduce a binding that causes us
> headaches later. I think we can still fix this with a followup
>>> patch
> before the driver and its binding hit a release kernel.
>
> Cheers,
> Andre.
>

 I just see some patch, where "phy-mode = internal" is valid.
 I will try to find a way to use it
>>>
>>> Can you provide a link?
>>
>> https://lkml.org/lkml/2017/6/23/479
>>
>>>
>>> I'm not a fan of using phy-mode for this. There's no guarantee
>>> what
>>> mode the internal PHY uses. That's what phy-mode is for.
>
> I can understand Chen-Yu's concerns, but ...
>
>> For each soc the internal PHY mode is know and setted in
>>> emac_variant/internal_phy
>> So its not a problem.
>
> that is true as well, at least for now.
>
> So while I agree that having a separate property to indicate
> the usage of the internal PHY would be nice, I am bit tempted
> to use this easier approach and piggy back on the existing
> phy-mode property.

 We're trying to fix an issue that works for now too.

 If we want to consider future weird cases, then we must
 consider all of them. And the phy mode changing is definitely
 not really far fetched.

 I agree with Chen-Yu, and I really feel like the compatible
 solution you suggested 

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Corentin Labbe]

On Tue, Jun 27, 2017 at 10:37:34AM -0700, Florian Fainelli wrote:
> On 06/27/2017 10:29 AM, Maxime Ripard wrote:
> > On Tue, Jun 27, 2017 at 02:37:48PM +0200, Corentin Labbe wrote:
> >> On Tue, Jun 27, 2017 at 11:33:56AM +0100, Andre Przywara wrote:
> >>> Hi,
> >>>
> >>> On 27/06/17 11:23, Icenowy Zheng wrote:
> 
> 
>  于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara 
>   写到:
> > Hi,
> >
> > On 27/06/17 10:41, Maxime Ripard wrote:
> >> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
> >>> Hi,
> >>>
> >>> (CC:ing some people from that Rockchip dmwac series)
> >>>
> >>> On 27/06/17 09:21, Corentin Labbe wrote:
>  On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> > On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
> >  wrote:
> >> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
> >>> On 31/05/17 08:18, Corentin Labbe wrote:
>  The dwmac-sun8i is a heavy hacked version of stmmac hardware by
>  allwinner.
>  In fact the only common part is the descriptor management and
> > the first
>  register function.
> >>>
> >>> Hi,
> >>>
> >>> I know I am a bit late with this, but while adapting the U-Boot
> > driver
> >>> to the new binding I was wondering about the internal PHY
> > detection:
> >>>
> >>>
> >>> So here you seem to deduce the usage of the internal PHY by the
> > PHY
> >>> interface specified in the DT (MII = internal, RGMII =
> > external).
> >>> I think I raised this question before, but isn't it perfectly
> > legal for
> >>> a board to use MII with an external PHY even on those SoCs that
> > feature
> >>> an internal PHY?
> >>> On the first glance that does not make too much sense, but apart
> > from
> >>> not being the correct binding to describe all of the SoCs
> > features I see
> >>> two scenarios:
> >>> 1) A board vendor might choose to not use the internal PHY
> > because it
> >>> has bugs, lacks features (configurability) or has other issues.
> > For
> >>> instance I have heard reports that the internal PHY makes the
> > SoC go
> >>> rather hot, possibly limiting the CPU frequency. By using an
> > external
> >>> MII PHY (which are still cheaper than RGMII PHYs) this can be
> > avoided.
> >>> 2) A PHY does not necessarily need to be directly connected to
> >>> magnetics. Indeed quite some boards use (RG)MII to connect to a
> > switch
> >>> IC or some other network circuitry, for instance fibre
> > connectors.
> >>>
> >>> So I was wondering if we would need an explicit:
> >>>   allwinner,use-internal-phy;
> >>> boolean DT property to signal the usage of the internal PHY?
> >>> Alternatively we could go with the negative version:
> >>>   allwinner,disable-internal-phy;
> >>>
> >>> Or what about introducing a new "allwinner,internal-mii-phy"
> > compatible
> >>> string for the *PHY* node and use that?
> >>>
> >>> I just want to avoid that we introduce a binding that causes us
> >>> headaches later. I think we can still fix this with a followup
> > patch
> >>> before the driver and its binding hit a release kernel.
> >>>
> >>> Cheers,
> >>> Andre.
> >>>
> >>
> >> I just see some patch, where "phy-mode = internal" is valid.
> >> I will try to find a way to use it
> >
> > Can you provide a link?
> 
>  https://lkml.org/lkml/2017/6/23/479
> 
> >
> > I'm not a fan of using phy-mode for this. There's no guarantee
> > what
> > mode the internal PHY uses. That's what phy-mode is for.
> >>>
> >>> I can understand Chen-Yu's concerns, but ...
> >>>
>  For each soc the internal PHY mode is know and setted in
> > emac_variant/internal_phy
>  So its not a problem.
> >>>
> >>> that is true as well, at least for now.
> >>>
> >>> So while I agree that having a separate property to indicate
> >>> the usage of the internal PHY would be nice, I am bit tempted
> >>> to use this easier approach and piggy back on the existing
> >>> phy-mode property.
> >>
> >> We're trying to fix an issue that works for now too.
> >>
> >> If we want to consider future weird cases, then we must
> >> consider all of them. And the phy mode changing is definitely
> >> not really far fetched.
> >>
> >> I agree with Chen-Yu, and I really feel like the compatible
> >> solution you suggested would cover both your concerns, and
> >> ours.

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Corentin Labbe]

On Tue, Jun 27, 2017 at 07:29:37PM +0200, Maxime Ripard wrote:
> On Tue, Jun 27, 2017 at 02:37:48PM +0200, Corentin Labbe wrote:
> > On Tue, Jun 27, 2017 at 11:33:56AM +0100, Andre Przywara wrote:
> > > Hi,
> > > 
> > > On 27/06/17 11:23, Icenowy Zheng wrote:
> > > > 
> > > > 
> > > > 于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara 
> > > >  写到:
> > > >> Hi,
> > > >>
> > > >> On 27/06/17 10:41, Maxime Ripard wrote:
> > > >>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
> > >  Hi,
> > > 
> > >  (CC:ing some people from that Rockchip dmwac series)
> > > 
> > >  On 27/06/17 09:21, Corentin Labbe wrote:
> > > > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> > > >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
> > > >>  wrote:
> > > >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
> > >  On 31/05/17 08:18, Corentin Labbe wrote:
> > > > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> > > > allwinner.
> > > > In fact the only common part is the descriptor management and
> > > >> the first
> > > > register function.
> > > 
> > >  Hi,
> > > 
> > >  I know I am a bit late with this, but while adapting the U-Boot
> > > >> driver
> > >  to the new binding I was wondering about the internal PHY
> > > >> detection:
> > > 
> > > 
> > >  So here you seem to deduce the usage of the internal PHY by the
> > > >> PHY
> > >  interface specified in the DT (MII = internal, RGMII =
> > > >> external).
> > >  I think I raised this question before, but isn't it perfectly
> > > >> legal for
> > >  a board to use MII with an external PHY even on those SoCs that
> > > >> feature
> > >  an internal PHY?
> > >  On the first glance that does not make too much sense, but apart
> > > >> from
> > >  not being the correct binding to describe all of the SoCs
> > > >> features I see
> > >  two scenarios:
> > >  1) A board vendor might choose to not use the internal PHY
> > > >> because it
> > >  has bugs, lacks features (configurability) or has other issues.
> > > >> For
> > >  instance I have heard reports that the internal PHY makes the
> > > >> SoC go
> > >  rather hot, possibly limiting the CPU frequency. By using an
> > > >> external
> > >  MII PHY (which are still cheaper than RGMII PHYs) this can be
> > > >> avoided.
> > >  2) A PHY does not necessarily need to be directly connected to
> > >  magnetics. Indeed quite some boards use (RG)MII to connect to a
> > > >> switch
> > >  IC or some other network circuitry, for instance fibre
> > > >> connectors.
> > > 
> > >  So I was wondering if we would need an explicit:
> > >    allwinner,use-internal-phy;
> > >  boolean DT property to signal the usage of the internal PHY?
> > >  Alternatively we could go with the negative version:
> > >    allwinner,disable-internal-phy;
> > > 
> > >  Or what about introducing a new "allwinner,internal-mii-phy"
> > > >> compatible
> > >  string for the *PHY* node and use that?
> > > 
> > >  I just want to avoid that we introduce a binding that causes us
> > >  headaches later. I think we can still fix this with a followup
> > > >> patch
> > >  before the driver and its binding hit a release kernel.
> > > 
> > >  Cheers,
> > >  Andre.
> > > 
> > > >>>
> > > >>> I just see some patch, where "phy-mode = internal" is valid.
> > > >>> I will try to find a way to use it
> > > >>
> > > >> Can you provide a link?
> > > >
> > > > https://lkml.org/lkml/2017/6/23/479
> > > >
> > > >>
> > > >> I'm not a fan of using phy-mode for this. There's no guarantee
> > > >> what
> > > >> mode the internal PHY uses. That's what phy-mode is for.
> > > 
> > >  I can understand Chen-Yu's concerns, but ...
> > > 
> > > > For each soc the internal PHY mode is know and setted in
> > > >> emac_variant/internal_phy
> > > > So its not a problem.
> > > 
> > >  that is true as well, at least for now.
> > > 
> > >  So while I agree that having a separate property to indicate
> > >  the usage of the internal PHY would be nice, I am bit tempted
> > >  to use this easier approach and piggy back on the existing
> > >  phy-mode property.
> > > >>>
> > > >>> We're trying to fix an issue that works for now too.
> > > >>>
> > > >>> If we want to consider future weird cases, then we must
> > > >>> consider all of them. And the phy mode changing is definitely
> > > >>> not really far fetched.
> > > >>>
> > > >>> I agree with Chen-Yu, and I really feel like the compatible
> > > >>> solution 

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Florian Fainelli]

On 06/27/2017 10:29 AM, Maxime Ripard wrote:
> On Tue, Jun 27, 2017 at 02:37:48PM +0200, Corentin Labbe wrote:
>> On Tue, Jun 27, 2017 at 11:33:56AM +0100, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 27/06/17 11:23, Icenowy Zheng wrote:


 于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara  
 写到:
> Hi,
>
> On 27/06/17 10:41, Maxime Ripard wrote:
>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>>> Hi,
>>>
>>> (CC:ing some people from that Rockchip dmwac series)
>>>
>>> On 27/06/17 09:21, Corentin Labbe wrote:
 On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>  wrote:
>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>>> On 31/05/17 08:18, Corentin Labbe wrote:
 The dwmac-sun8i is a heavy hacked version of stmmac hardware by
 allwinner.
 In fact the only common part is the descriptor management and
> the first
 register function.
>>>
>>> Hi,
>>>
>>> I know I am a bit late with this, but while adapting the U-Boot
> driver
>>> to the new binding I was wondering about the internal PHY
> detection:
>>>
>>>
>>> So here you seem to deduce the usage of the internal PHY by the
> PHY
>>> interface specified in the DT (MII = internal, RGMII =
> external).
>>> I think I raised this question before, but isn't it perfectly
> legal for
>>> a board to use MII with an external PHY even on those SoCs that
> feature
>>> an internal PHY?
>>> On the first glance that does not make too much sense, but apart
> from
>>> not being the correct binding to describe all of the SoCs
> features I see
>>> two scenarios:
>>> 1) A board vendor might choose to not use the internal PHY
> because it
>>> has bugs, lacks features (configurability) or has other issues.
> For
>>> instance I have heard reports that the internal PHY makes the
> SoC go
>>> rather hot, possibly limiting the CPU frequency. By using an
> external
>>> MII PHY (which are still cheaper than RGMII PHYs) this can be
> avoided.
>>> 2) A PHY does not necessarily need to be directly connected to
>>> magnetics. Indeed quite some boards use (RG)MII to connect to a
> switch
>>> IC or some other network circuitry, for instance fibre
> connectors.
>>>
>>> So I was wondering if we would need an explicit:
>>>   allwinner,use-internal-phy;
>>> boolean DT property to signal the usage of the internal PHY?
>>> Alternatively we could go with the negative version:
>>>   allwinner,disable-internal-phy;
>>>
>>> Or what about introducing a new "allwinner,internal-mii-phy"
> compatible
>>> string for the *PHY* node and use that?
>>>
>>> I just want to avoid that we introduce a binding that causes us
>>> headaches later. I think we can still fix this with a followup
> patch
>>> before the driver and its binding hit a release kernel.
>>>
>>> Cheers,
>>> Andre.
>>>
>>
>> I just see some patch, where "phy-mode = internal" is valid.
>> I will try to find a way to use it
>
> Can you provide a link?

 https://lkml.org/lkml/2017/6/23/479

>
> I'm not a fan of using phy-mode for this. There's no guarantee
> what
> mode the internal PHY uses. That's what phy-mode is for.
>>>
>>> I can understand Chen-Yu's concerns, but ...
>>>
 For each soc the internal PHY mode is know and setted in
> emac_variant/internal_phy
 So its not a problem.
>>>
>>> that is true as well, at least for now.
>>>
>>> So while I agree that having a separate property to indicate
>>> the usage of the internal PHY would be nice, I am bit tempted
>>> to use this easier approach and piggy back on the existing
>>> phy-mode property.
>>
>> We're trying to fix an issue that works for now too.
>>
>> If we want to consider future weird cases, then we must
>> consider all of them. And the phy mode changing is definitely
>> not really far fetched.
>>
>> I agree with Chen-Yu, and I really feel like the compatible
>> solution you suggested would cover both your concerns, and
>> ours.
>
> So something like this?
>   emac: emac@1c3 {
>   compatible = "allwinner,sun8i-h3-emac";
>   ...
>   phy-mode = "mii";
>   phy-handle = <_mii_phy>;
>   ...
>
>   mdio: mdio {
>#address-cells = <1>;
> 

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Maxime Ripard]

On Tue, Jun 27, 2017 at 02:37:48PM +0200, Corentin Labbe wrote:
> On Tue, Jun 27, 2017 at 11:33:56AM +0100, Andre Przywara wrote:
> > Hi,
> > 
> > On 27/06/17 11:23, Icenowy Zheng wrote:
> > > 
> > > 
> > > 于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara  
> > > 写到:
> > >> Hi,
> > >>
> > >> On 27/06/17 10:41, Maxime Ripard wrote:
> > >>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
> >  Hi,
> > 
> >  (CC:ing some people from that Rockchip dmwac series)
> > 
> >  On 27/06/17 09:21, Corentin Labbe wrote:
> > > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> > >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
> > >>  wrote:
> > >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
> >  On 31/05/17 08:18, Corentin Labbe wrote:
> > > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> > > allwinner.
> > > In fact the only common part is the descriptor management and
> > >> the first
> > > register function.
> > 
> >  Hi,
> > 
> >  I know I am a bit late with this, but while adapting the U-Boot
> > >> driver
> >  to the new binding I was wondering about the internal PHY
> > >> detection:
> > 
> > 
> >  So here you seem to deduce the usage of the internal PHY by the
> > >> PHY
> >  interface specified in the DT (MII = internal, RGMII =
> > >> external).
> >  I think I raised this question before, but isn't it perfectly
> > >> legal for
> >  a board to use MII with an external PHY even on those SoCs that
> > >> feature
> >  an internal PHY?
> >  On the first glance that does not make too much sense, but apart
> > >> from
> >  not being the correct binding to describe all of the SoCs
> > >> features I see
> >  two scenarios:
> >  1) A board vendor might choose to not use the internal PHY
> > >> because it
> >  has bugs, lacks features (configurability) or has other issues.
> > >> For
> >  instance I have heard reports that the internal PHY makes the
> > >> SoC go
> >  rather hot, possibly limiting the CPU frequency. By using an
> > >> external
> >  MII PHY (which are still cheaper than RGMII PHYs) this can be
> > >> avoided.
> >  2) A PHY does not necessarily need to be directly connected to
> >  magnetics. Indeed quite some boards use (RG)MII to connect to a
> > >> switch
> >  IC or some other network circuitry, for instance fibre
> > >> connectors.
> > 
> >  So I was wondering if we would need an explicit:
> >    allwinner,use-internal-phy;
> >  boolean DT property to signal the usage of the internal PHY?
> >  Alternatively we could go with the negative version:
> >    allwinner,disable-internal-phy;
> > 
> >  Or what about introducing a new "allwinner,internal-mii-phy"
> > >> compatible
> >  string for the *PHY* node and use that?
> > 
> >  I just want to avoid that we introduce a binding that causes us
> >  headaches later. I think we can still fix this with a followup
> > >> patch
> >  before the driver and its binding hit a release kernel.
> > 
> >  Cheers,
> >  Andre.
> > 
> > >>>
> > >>> I just see some patch, where "phy-mode = internal" is valid.
> > >>> I will try to find a way to use it
> > >>
> > >> Can you provide a link?
> > >
> > > https://lkml.org/lkml/2017/6/23/479
> > >
> > >>
> > >> I'm not a fan of using phy-mode for this. There's no guarantee
> > >> what
> > >> mode the internal PHY uses. That's what phy-mode is for.
> > 
> >  I can understand Chen-Yu's concerns, but ...
> > 
> > > For each soc the internal PHY mode is know and setted in
> > >> emac_variant/internal_phy
> > > So its not a problem.
> > 
> >  that is true as well, at least for now.
> > 
> >  So while I agree that having a separate property to indicate
> >  the usage of the internal PHY would be nice, I am bit tempted
> >  to use this easier approach and piggy back on the existing
> >  phy-mode property.
> > >>>
> > >>> We're trying to fix an issue that works for now too.
> > >>>
> > >>> If we want to consider future weird cases, then we must
> > >>> consider all of them. And the phy mode changing is definitely
> > >>> not really far fetched.
> > >>>
> > >>> I agree with Chen-Yu, and I really feel like the compatible
> > >>> solution you suggested would cover both your concerns, and
> > >>> ours.
> > >>
> > >> So something like this?
> > >>  emac: emac@1c3 {
> > >>  compatible = "allwinner,sun8i-h3-emac";
> > >>  ...
> > >>  phy-mode = "mii";
> > >>  phy-handle = <_mii_phy>;
> > >>  ...
> > >>
> > >>  

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Maxime Ripard]

On Tue, Jun 27, 2017 at 11:33:56AM +0100, Andre Przywara wrote:
> Hi,
> 
> On 27/06/17 11:23, Icenowy Zheng wrote:
> > 
> > 
> > 于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara  
> > 写到:
> >> Hi,
> >>
> >> On 27/06/17 10:41, Maxime Ripard wrote:
> >>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>  Hi,
> 
>  (CC:ing some people from that Rockchip dmwac series)
> 
>  On 27/06/17 09:21, Corentin Labbe wrote:
> > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
> >>  wrote:
> >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>  On 31/05/17 08:18, Corentin Labbe wrote:
> > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> > allwinner.
> > In fact the only common part is the descriptor management and
> >> the first
> > register function.
> 
>  Hi,
> 
>  I know I am a bit late with this, but while adapting the U-Boot
> >> driver
>  to the new binding I was wondering about the internal PHY
> >> detection:
> 
> 
>  So here you seem to deduce the usage of the internal PHY by the
> >> PHY
>  interface specified in the DT (MII = internal, RGMII =
> >> external).
>  I think I raised this question before, but isn't it perfectly
> >> legal for
>  a board to use MII with an external PHY even on those SoCs that
> >> feature
>  an internal PHY?
>  On the first glance that does not make too much sense, but apart
> >> from
>  not being the correct binding to describe all of the SoCs
> >> features I see
>  two scenarios:
>  1) A board vendor might choose to not use the internal PHY
> >> because it
>  has bugs, lacks features (configurability) or has other issues.
> >> For
>  instance I have heard reports that the internal PHY makes the
> >> SoC go
>  rather hot, possibly limiting the CPU frequency. By using an
> >> external
>  MII PHY (which are still cheaper than RGMII PHYs) this can be
> >> avoided.
>  2) A PHY does not necessarily need to be directly connected to
>  magnetics. Indeed quite some boards use (RG)MII to connect to a
> >> switch
>  IC or some other network circuitry, for instance fibre
> >> connectors.
> 
>  So I was wondering if we would need an explicit:
>    allwinner,use-internal-phy;
>  boolean DT property to signal the usage of the internal PHY?
>  Alternatively we could go with the negative version:
>    allwinner,disable-internal-phy;
> 
>  Or what about introducing a new "allwinner,internal-mii-phy"
> >> compatible
>  string for the *PHY* node and use that?
> 
>  I just want to avoid that we introduce a binding that causes us
>  headaches later. I think we can still fix this with a followup
> >> patch
>  before the driver and its binding hit a release kernel.
> 
>  Cheers,
>  Andre.
> 
> >>>
> >>> I just see some patch, where "phy-mode = internal" is valid.
> >>> I will try to find a way to use it
> >>
> >> Can you provide a link?
> >
> > https://lkml.org/lkml/2017/6/23/479
> >
> >>
> >> I'm not a fan of using phy-mode for this. There's no guarantee
> >> what
> >> mode the internal PHY uses. That's what phy-mode is for.
> 
>  I can understand Chen-Yu's concerns, but ...
> 
> > For each soc the internal PHY mode is know and setted in
> >> emac_variant/internal_phy
> > So its not a problem.
> 
>  that is true as well, at least for now.
> 
>  So while I agree that having a separate property to indicate the
> >> usage
>  of the internal PHY would be nice, I am bit tempted to use this
> >> easier
>  approach and piggy back on the existing phy-mode property.
> >>>
> >>> We're trying to fix an issue that works for now too.
> >>>
> >>> If we want to consider future weird cases, then we must consider all
> >>> of them. And the phy mode changing is definitely not really far
> >>> fetched.
> >>>
> >>> I agree with Chen-Yu, and I really feel like the compatible solution
> >>> you suggested would cover both your concerns, and ours.
> >>
> >> So something like this?
> >>emac: emac@1c3 {
> >>compatible = "allwinner,sun8i-h3-emac";
> >>...
> >>phy-mode = "mii";
> >>phy-handle = <_mii_phy>;
> >>...
> >>
> >>mdio: mdio {
> >>#address-cells = <1>;
> >>#size-cells = <0>;
> >>int_mii_phy: ethernet-phy@1 {
> >>compatible = "allwinner,sun8i-h3-ephy";
> >>syscon = <>;
> > 
> > The MAC still needs to set some bits of syscon register.

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Corentin Labbe]

On Tue, Jun 27, 2017 at 11:33:56AM +0100, Andre Przywara wrote:
> Hi,
> 
> On 27/06/17 11:23, Icenowy Zheng wrote:
> > 
> > 
> > 于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara  
> > 写到:
> >> Hi,
> >>
> >> On 27/06/17 10:41, Maxime Ripard wrote:
> >>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>  Hi,
> 
>  (CC:ing some people from that Rockchip dmwac series)
> 
>  On 27/06/17 09:21, Corentin Labbe wrote:
> > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
> >>  wrote:
> >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>  On 31/05/17 08:18, Corentin Labbe wrote:
> > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> > allwinner.
> > In fact the only common part is the descriptor management and
> >> the first
> > register function.
> 
>  Hi,
> 
>  I know I am a bit late with this, but while adapting the U-Boot
> >> driver
>  to the new binding I was wondering about the internal PHY
> >> detection:
> 
> 
>  So here you seem to deduce the usage of the internal PHY by the
> >> PHY
>  interface specified in the DT (MII = internal, RGMII =
> >> external).
>  I think I raised this question before, but isn't it perfectly
> >> legal for
>  a board to use MII with an external PHY even on those SoCs that
> >> feature
>  an internal PHY?
>  On the first glance that does not make too much sense, but apart
> >> from
>  not being the correct binding to describe all of the SoCs
> >> features I see
>  two scenarios:
>  1) A board vendor might choose to not use the internal PHY
> >> because it
>  has bugs, lacks features (configurability) or has other issues.
> >> For
>  instance I have heard reports that the internal PHY makes the
> >> SoC go
>  rather hot, possibly limiting the CPU frequency. By using an
> >> external
>  MII PHY (which are still cheaper than RGMII PHYs) this can be
> >> avoided.
>  2) A PHY does not necessarily need to be directly connected to
>  magnetics. Indeed quite some boards use (RG)MII to connect to a
> >> switch
>  IC or some other network circuitry, for instance fibre
> >> connectors.
> 
>  So I was wondering if we would need an explicit:
>    allwinner,use-internal-phy;
>  boolean DT property to signal the usage of the internal PHY?
>  Alternatively we could go with the negative version:
>    allwinner,disable-internal-phy;
> 
>  Or what about introducing a new "allwinner,internal-mii-phy"
> >> compatible
>  string for the *PHY* node and use that?
> 
>  I just want to avoid that we introduce a binding that causes us
>  headaches later. I think we can still fix this with a followup
> >> patch
>  before the driver and its binding hit a release kernel.
> 
>  Cheers,
>  Andre.
> 
> >>>
> >>> I just see some patch, where "phy-mode = internal" is valid.
> >>> I will try to find a way to use it
> >>
> >> Can you provide a link?
> >
> > https://lkml.org/lkml/2017/6/23/479
> >
> >>
> >> I'm not a fan of using phy-mode for this. There's no guarantee
> >> what
> >> mode the internal PHY uses. That's what phy-mode is for.
> 
>  I can understand Chen-Yu's concerns, but ...
> 
> > For each soc the internal PHY mode is know and setted in
> >> emac_variant/internal_phy
> > So its not a problem.
> 
>  that is true as well, at least for now.
> 
>  So while I agree that having a separate property to indicate the
> >> usage
>  of the internal PHY would be nice, I am bit tempted to use this
> >> easier
>  approach and piggy back on the existing phy-mode property.
> >>>
> >>> We're trying to fix an issue that works for now too.
> >>>
> >>> If we want to consider future weird cases, then we must consider all
> >>> of them. And the phy mode changing is definitely not really far
> >>> fetched.
> >>>
> >>> I agree with Chen-Yu, and I really feel like the compatible solution
> >>> you suggested would cover both your concerns, and ours.
> >>
> >> So something like this?
> >>emac: emac@1c3 {
> >>compatible = "allwinner,sun8i-h3-emac";
> >>...
> >>phy-mode = "mii";
> >>phy-handle = <_mii_phy>;
> >>...
> >>
> >>mdio: mdio {
> >>#address-cells = <1>;
> >>#size-cells = <0>;
> >>int_mii_phy: ethernet-phy@1 {
> >>compatible = "allwinner,sun8i-h3-ephy";
> >>syscon = <>;
> > 
> > The MAC still needs to set some bits of syscon register.

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Andre Przywara]

Hi,

On 27/06/17 11:23, Icenowy Zheng wrote:
> 
> 
> 于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara  写到:
>> Hi,
>>
>> On 27/06/17 10:41, Maxime Ripard wrote:
>>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
 Hi,

 (CC:ing some people from that Rockchip dmwac series)

 On 27/06/17 09:21, Corentin Labbe wrote:
> On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
>> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>>  wrote:
>>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
 On 31/05/17 08:18, Corentin Labbe wrote:
> The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> allwinner.
> In fact the only common part is the descriptor management and
>> the first
> register function.

 Hi,

 I know I am a bit late with this, but while adapting the U-Boot
>> driver
 to the new binding I was wondering about the internal PHY
>> detection:


 So here you seem to deduce the usage of the internal PHY by the
>> PHY
 interface specified in the DT (MII = internal, RGMII =
>> external).
 I think I raised this question before, but isn't it perfectly
>> legal for
 a board to use MII with an external PHY even on those SoCs that
>> feature
 an internal PHY?
 On the first glance that does not make too much sense, but apart
>> from
 not being the correct binding to describe all of the SoCs
>> features I see
 two scenarios:
 1) A board vendor might choose to not use the internal PHY
>> because it
 has bugs, lacks features (configurability) or has other issues.
>> For
 instance I have heard reports that the internal PHY makes the
>> SoC go
 rather hot, possibly limiting the CPU frequency. By using an
>> external
 MII PHY (which are still cheaper than RGMII PHYs) this can be
>> avoided.
 2) A PHY does not necessarily need to be directly connected to
 magnetics. Indeed quite some boards use (RG)MII to connect to a
>> switch
 IC or some other network circuitry, for instance fibre
>> connectors.

 So I was wondering if we would need an explicit:
   allwinner,use-internal-phy;
 boolean DT property to signal the usage of the internal PHY?
 Alternatively we could go with the negative version:
   allwinner,disable-internal-phy;

 Or what about introducing a new "allwinner,internal-mii-phy"
>> compatible
 string for the *PHY* node and use that?

 I just want to avoid that we introduce a binding that causes us
 headaches later. I think we can still fix this with a followup
>> patch
 before the driver and its binding hit a release kernel.

 Cheers,
 Andre.

>>>
>>> I just see some patch, where "phy-mode = internal" is valid.
>>> I will try to find a way to use it
>>
>> Can you provide a link?
>
> https://lkml.org/lkml/2017/6/23/479
>
>>
>> I'm not a fan of using phy-mode for this. There's no guarantee
>> what
>> mode the internal PHY uses. That's what phy-mode is for.

 I can understand Chen-Yu's concerns, but ...

> For each soc the internal PHY mode is know and setted in
>> emac_variant/internal_phy
> So its not a problem.

 that is true as well, at least for now.

 So while I agree that having a separate property to indicate the
>> usage
 of the internal PHY would be nice, I am bit tempted to use this
>> easier
 approach and piggy back on the existing phy-mode property.
>>>
>>> We're trying to fix an issue that works for now too.
>>>
>>> If we want to consider future weird cases, then we must consider all
>>> of them. And the phy mode changing is definitely not really far
>>> fetched.
>>>
>>> I agree with Chen-Yu, and I really feel like the compatible solution
>>> you suggested would cover both your concerns, and ours.
>>
>> So something like this?
>>  emac: emac@1c3 {
>>  compatible = "allwinner,sun8i-h3-emac";
>>  ...
>>  phy-mode = "mii";
>>  phy-handle = <_mii_phy>;
>>  ...
>>
>>  mdio: mdio {
>>#address-cells = <1>;
>>#size-cells = <0>;
>>int_mii_phy: ethernet-phy@1 {
>>compatible = "allwinner,sun8i-h3-ephy";
>>syscon = <>;
> 
> The MAC still needs to set some bits of syscon register.

Yes, the syscon property needs also to be in the MAC node, that was
meant to be somewhere in the second "..." ;-)

But now since Chen-Yu mentioned that we need to set up the PHY *first*
to make it actually discoverable via MDIO, I wonder if we could change
this to:
1) have the DT as described here
2) Let the dwmac-sun8i 

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Icenowy Zheng]

于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara  写到:
>Hi,
>
>On 27/06/17 10:41, Maxime Ripard wrote:
>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>>> Hi,
>>>
>>> (CC:ing some people from that Rockchip dmwac series)
>>>
>>> On 27/06/17 09:21, Corentin Labbe wrote:
 On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>  wrote:
>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>>> On 31/05/17 08:18, Corentin Labbe wrote:
 The dwmac-sun8i is a heavy hacked version of stmmac hardware by
 allwinner.
 In fact the only common part is the descriptor management and
>the first
 register function.
>>>
>>> Hi,
>>>
>>> I know I am a bit late with this, but while adapting the U-Boot
>driver
>>> to the new binding I was wondering about the internal PHY
>detection:
>>>
>>>
>>> So here you seem to deduce the usage of the internal PHY by the
>PHY
>>> interface specified in the DT (MII = internal, RGMII =
>external).
>>> I think I raised this question before, but isn't it perfectly
>legal for
>>> a board to use MII with an external PHY even on those SoCs that
>feature
>>> an internal PHY?
>>> On the first glance that does not make too much sense, but apart
>from
>>> not being the correct binding to describe all of the SoCs
>features I see
>>> two scenarios:
>>> 1) A board vendor might choose to not use the internal PHY
>because it
>>> has bugs, lacks features (configurability) or has other issues.
>For
>>> instance I have heard reports that the internal PHY makes the
>SoC go
>>> rather hot, possibly limiting the CPU frequency. By using an
>external
>>> MII PHY (which are still cheaper than RGMII PHYs) this can be
>avoided.
>>> 2) A PHY does not necessarily need to be directly connected to
>>> magnetics. Indeed quite some boards use (RG)MII to connect to a
>switch
>>> IC or some other network circuitry, for instance fibre
>connectors.
>>>
>>> So I was wondering if we would need an explicit:
>>>   allwinner,use-internal-phy;
>>> boolean DT property to signal the usage of the internal PHY?
>>> Alternatively we could go with the negative version:
>>>   allwinner,disable-internal-phy;
>>>
>>> Or what about introducing a new "allwinner,internal-mii-phy"
>compatible
>>> string for the *PHY* node and use that?
>>>
>>> I just want to avoid that we introduce a binding that causes us
>>> headaches later. I think we can still fix this with a followup
>patch
>>> before the driver and its binding hit a release kernel.
>>>
>>> Cheers,
>>> Andre.
>>>
>>
>> I just see some patch, where "phy-mode = internal" is valid.
>> I will try to find a way to use it
>
> Can you provide a link?

 https://lkml.org/lkml/2017/6/23/479

>
> I'm not a fan of using phy-mode for this. There's no guarantee
>what
> mode the internal PHY uses. That's what phy-mode is for.
>>>
>>> I can understand Chen-Yu's concerns, but ...
>>>
 For each soc the internal PHY mode is know and setted in
>emac_variant/internal_phy
 So its not a problem.
>>>
>>> that is true as well, at least for now.
>>>
>>> So while I agree that having a separate property to indicate the
>usage
>>> of the internal PHY would be nice, I am bit tempted to use this
>easier
>>> approach and piggy back on the existing phy-mode property.
>> 
>> We're trying to fix an issue that works for now too.
>> 
>> If we want to consider future weird cases, then we must consider all
>> of them. And the phy mode changing is definitely not really far
>> fetched.
>> 
>> I agree with Chen-Yu, and I really feel like the compatible solution
>> you suggested would cover both your concerns, and ours.
>
>So something like this?
>   emac: emac@1c3 {
>   compatible = "allwinner,sun8i-h3-emac";
>   ...
>   phy-mode = "mii";
>   phy-handle = <_mii_phy>;
>   ...
>
>   mdio: mdio {
>#address-cells = <1>;
>#size-cells = <0>;
>int_mii_phy: ethernet-phy@1 {
>compatible = "allwinner,sun8i-h3-ephy";
>syscon = <>;

The MAC still needs to set some bits of syscon register.

>reg = <1>;
>clocks = < CLK_BUS_EPHY>;
>resets = < RST_BUS_EPHY>;
>};
>};
>};
>
>And then move the internal-PHY setup code into a separate PHY driver?
>
>That looks like the architecturally best solution to me, but is
>probably
>also a bit involved since it would require a separate PHY driver.
>Or can we make it simpler, but still use this binding?
>
>Cheers,
>Andre.

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Chen-Yu Tsai]

On Tue, Jun 27, 2017 at 6:15 PM, Andre Przywara  wrote:
> Hi,
>
> On 27/06/17 10:41, Maxime Ripard wrote:
>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>>> Hi,
>>>
>>> (CC:ing some people from that Rockchip dmwac series)
>>>
>>> On 27/06/17 09:21, Corentin Labbe wrote:
 On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>  wrote:
>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>>> On 31/05/17 08:18, Corentin Labbe wrote:
 The dwmac-sun8i is a heavy hacked version of stmmac hardware by
 allwinner.
 In fact the only common part is the descriptor management and the first
 register function.
>>>
>>> Hi,
>>>
>>> I know I am a bit late with this, but while adapting the U-Boot driver
>>> to the new binding I was wondering about the internal PHY detection:
>>>
>>>
>>> So here you seem to deduce the usage of the internal PHY by the PHY
>>> interface specified in the DT (MII = internal, RGMII = external).
>>> I think I raised this question before, but isn't it perfectly legal for
>>> a board to use MII with an external PHY even on those SoCs that feature
>>> an internal PHY?
>>> On the first glance that does not make too much sense, but apart from
>>> not being the correct binding to describe all of the SoCs features I see
>>> two scenarios:
>>> 1) A board vendor might choose to not use the internal PHY because it
>>> has bugs, lacks features (configurability) or has other issues. For
>>> instance I have heard reports that the internal PHY makes the SoC go
>>> rather hot, possibly limiting the CPU frequency. By using an external
>>> MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
>>> 2) A PHY does not necessarily need to be directly connected to
>>> magnetics. Indeed quite some boards use (RG)MII to connect to a switch
>>> IC or some other network circuitry, for instance fibre connectors.
>>>
>>> So I was wondering if we would need an explicit:
>>>   allwinner,use-internal-phy;
>>> boolean DT property to signal the usage of the internal PHY?
>>> Alternatively we could go with the negative version:
>>>   allwinner,disable-internal-phy;
>>>
>>> Or what about introducing a new "allwinner,internal-mii-phy" compatible
>>> string for the *PHY* node and use that?
>>>
>>> I just want to avoid that we introduce a binding that causes us
>>> headaches later. I think we can still fix this with a followup patch
>>> before the driver and its binding hit a release kernel.
>>>
>>> Cheers,
>>> Andre.
>>>
>>
>> I just see some patch, where "phy-mode = internal" is valid.
>> I will try to find a way to use it
>
> Can you provide a link?

 https://lkml.org/lkml/2017/6/23/479

>
> I'm not a fan of using phy-mode for this. There's no guarantee what
> mode the internal PHY uses. That's what phy-mode is for.
>>>
>>> I can understand Chen-Yu's concerns, but ...
>>>
 For each soc the internal PHY mode is know and setted in 
 emac_variant/internal_phy
 So its not a problem.
>>>
>>> that is true as well, at least for now.
>>>
>>> So while I agree that having a separate property to indicate the usage
>>> of the internal PHY would be nice, I am bit tempted to use this easier
>>> approach and piggy back on the existing phy-mode property.
>>
>> We're trying to fix an issue that works for now too.
>>
>> If we want to consider future weird cases, then we must consider all
>> of them. And the phy mode changing is definitely not really far
>> fetched.
>>
>> I agree with Chen-Yu, and I really feel like the compatible solution
>> you suggested would cover both your concerns, and ours.
>
> So something like this?
> emac: emac@1c3 {
> compatible = "allwinner,sun8i-h3-emac";
> ...
> phy-mode = "mii";
> phy-handle = <_mii_phy>;
> ...
>
> mdio: mdio {
> #address-cells = <1>;
> #size-cells = <0>;
> int_mii_phy: ethernet-phy@1 {
> compatible = "allwinner,sun8i-h3-ephy";
> syscon = <>;
> reg = <1>;
> clocks = < CLK_BUS_EPHY>;
> resets = < RST_BUS_EPHY>;
> };
> };
> };
>
> And then move the internal-PHY setup code into a separate PHY driver?
>
> That looks like the architecturally best solution to me, but is probably
> also a bit involved since it would require a separate PHY driver.
> Or can we make it simpler, but still use this binding?

This was my initial approach prior to handing it off to Corentin.

The MDIO bus is discoverable, so in the kernel 

Re: [linux-sunxi] Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Chen-Yu Tsai]

On Tue, Jun 27, 2017 at 6:17 PM, Icenowy Zheng  wrote:
>
>
> 于 2017年6月27日 GMT+08:00 下午6:11:47, Chen-Yu Tsai  写到:
>>On Tue, Jun 27, 2017 at 5:41 PM, Maxime Ripard
>> wrote:
>>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
 Hi,

 (CC:ing some people from that Rockchip dmwac series)

 On 27/06/17 09:21, Corentin Labbe wrote:
 > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
 >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
 >>  wrote:
 >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
  On 31/05/17 08:18, Corentin Labbe wrote:
 > The dwmac-sun8i is a heavy hacked version of stmmac hardware
>>by
 > allwinner.
 > In fact the only common part is the descriptor management and
>>the first
 > register function.
 
  Hi,
 
  I know I am a bit late with this, but while adapting the U-Boot
>>driver
  to the new binding I was wondering about the internal PHY
>>detection:
 
 
  So here you seem to deduce the usage of the internal PHY by the
>>PHY
  interface specified in the DT (MII = internal, RGMII =
>>external).
  I think I raised this question before, but isn't it perfectly
>>legal for
  a board to use MII with an external PHY even on those SoCs that
>>feature
  an internal PHY?
  On the first glance that does not make too much sense, but
>>apart from
  not being the correct binding to describe all of the SoCs
>>features I see
  two scenarios:
  1) A board vendor might choose to not use the internal PHY
>>because it
  has bugs, lacks features (configurability) or has other issues.
>>For
  instance I have heard reports that the internal PHY makes the
>>SoC go
  rather hot, possibly limiting the CPU frequency. By using an
>>external
  MII PHY (which are still cheaper than RGMII PHYs) this can be
>>avoided.
  2) A PHY does not necessarily need to be directly connected to
  magnetics. Indeed quite some boards use (RG)MII to connect to a
>>switch
  IC or some other network circuitry, for instance fibre
>>connectors.
 
  So I was wondering if we would need an explicit:
    allwinner,use-internal-phy;
  boolean DT property to signal the usage of the internal PHY?
  Alternatively we could go with the negative version:
    allwinner,disable-internal-phy;
 
  Or what about introducing a new "allwinner,internal-mii-phy"
>>compatible
  string for the *PHY* node and use that?
 
  I just want to avoid that we introduce a binding that causes us
  headaches later. I think we can still fix this with a followup
>>patch
  before the driver and its binding hit a release kernel.
 
  Cheers,
  Andre.
 
 >>>
 >>> I just see some patch, where "phy-mode = internal" is valid.
 >>> I will try to find a way to use it
 >>
 >> Can you provide a link?
 >
 > https://lkml.org/lkml/2017/6/23/479
 >
 >>
 >> I'm not a fan of using phy-mode for this. There's no guarantee
>>what
 >> mode the internal PHY uses. That's what phy-mode is for.

 I can understand Chen-Yu's concerns, but ...

 > For each soc the internal PHY mode is know and setted in
>>emac_variant/internal_phy
 > So its not a problem.

 that is true as well, at least for now.

 So while I agree that having a separate property to indicate the
>>usage
 of the internal PHY would be nice, I am bit tempted to use this
>>easier
 approach and piggy back on the existing phy-mode property.
>>>
>>> We're trying to fix an issue that works for now too.
>>>
>>> If we want to consider future weird cases, then we must consider all
>>> of them. And the phy mode changing is definitely not really far
>>> fetched.
>>
>>I guess the issue is whether it's likely that the vendor puts 2
>>internal
>>PHYs in one SoC, and they use different modes and can be switched
>>around.
>>Otherwise it's fixed for a given SoC, and we can just handle that with
>>the per-SoC GMAC compatible.
>>
>>Maybe Florian could tell us if this was one of the intended use cases
>>for the "internal" phy mode.
>>
>>As for Rockchip, AFAIK they have 2 MACs, one is connected to the
>>internal
>>PHY, while the other is connected to the external interface, and there
>>is
>>no muxing involved, unlike Allwinner's solution.
>>
>>> I agree with Chen-Yu, and I really feel like the compatible solution
>>> you suggested would cover both your concerns, and ours.
>>
>>If using a PHY compatible is the solution, we could just use the
>>"ethernet-phy-id." style one, and put in the bogus ID that
>>Allwinner used.
>>
>>Care must 

Re: [linux-sunxi] Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Icenowy Zheng]

于 2017年6月27日 GMT+08:00 下午6:11:47, Chen-Yu Tsai  写到:
>On Tue, Jun 27, 2017 at 5:41 PM, Maxime Ripard
> wrote:
>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>>> Hi,
>>>
>>> (CC:ing some people from that Rockchip dmwac series)
>>>
>>> On 27/06/17 09:21, Corentin Labbe wrote:
>>> > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
>>> >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>>> >>  wrote:
>>> >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>>>  On 31/05/17 08:18, Corentin Labbe wrote:
>>> > The dwmac-sun8i is a heavy hacked version of stmmac hardware
>by
>>> > allwinner.
>>> > In fact the only common part is the descriptor management and
>the first
>>> > register function.
>>> 
>>>  Hi,
>>> 
>>>  I know I am a bit late with this, but while adapting the U-Boot
>driver
>>>  to the new binding I was wondering about the internal PHY
>detection:
>>> 
>>> 
>>>  So here you seem to deduce the usage of the internal PHY by the
>PHY
>>>  interface specified in the DT (MII = internal, RGMII =
>external).
>>>  I think I raised this question before, but isn't it perfectly
>legal for
>>>  a board to use MII with an external PHY even on those SoCs that
>feature
>>>  an internal PHY?
>>>  On the first glance that does not make too much sense, but
>apart from
>>>  not being the correct binding to describe all of the SoCs
>features I see
>>>  two scenarios:
>>>  1) A board vendor might choose to not use the internal PHY
>because it
>>>  has bugs, lacks features (configurability) or has other issues.
>For
>>>  instance I have heard reports that the internal PHY makes the
>SoC go
>>>  rather hot, possibly limiting the CPU frequency. By using an
>external
>>>  MII PHY (which are still cheaper than RGMII PHYs) this can be
>avoided.
>>>  2) A PHY does not necessarily need to be directly connected to
>>>  magnetics. Indeed quite some boards use (RG)MII to connect to a
>switch
>>>  IC or some other network circuitry, for instance fibre
>connectors.
>>> 
>>>  So I was wondering if we would need an explicit:
>>>    allwinner,use-internal-phy;
>>>  boolean DT property to signal the usage of the internal PHY?
>>>  Alternatively we could go with the negative version:
>>>    allwinner,disable-internal-phy;
>>> 
>>>  Or what about introducing a new "allwinner,internal-mii-phy"
>compatible
>>>  string for the *PHY* node and use that?
>>> 
>>>  I just want to avoid that we introduce a binding that causes us
>>>  headaches later. I think we can still fix this with a followup
>patch
>>>  before the driver and its binding hit a release kernel.
>>> 
>>>  Cheers,
>>>  Andre.
>>> 
>>> >>>
>>> >>> I just see some patch, where "phy-mode = internal" is valid.
>>> >>> I will try to find a way to use it
>>> >>
>>> >> Can you provide a link?
>>> >
>>> > https://lkml.org/lkml/2017/6/23/479
>>> >
>>> >>
>>> >> I'm not a fan of using phy-mode for this. There's no guarantee
>what
>>> >> mode the internal PHY uses. That's what phy-mode is for.
>>>
>>> I can understand Chen-Yu's concerns, but ...
>>>
>>> > For each soc the internal PHY mode is know and setted in
>emac_variant/internal_phy
>>> > So its not a problem.
>>>
>>> that is true as well, at least for now.
>>>
>>> So while I agree that having a separate property to indicate the
>usage
>>> of the internal PHY would be nice, I am bit tempted to use this
>easier
>>> approach and piggy back on the existing phy-mode property.
>>
>> We're trying to fix an issue that works for now too.
>>
>> If we want to consider future weird cases, then we must consider all
>> of them. And the phy mode changing is definitely not really far
>> fetched.
>
>I guess the issue is whether it's likely that the vendor puts 2
>internal
>PHYs in one SoC, and they use different modes and can be switched
>around.
>Otherwise it's fixed for a given SoC, and we can just handle that with
>the per-SoC GMAC compatible.
>
>Maybe Florian could tell us if this was one of the intended use cases
>for the "internal" phy mode.
>
>As for Rockchip, AFAIK they have 2 MACs, one is connected to the
>internal
>PHY, while the other is connected to the external interface, and there
>is
>no muxing involved, unlike Allwinner's solution.
>
>> I agree with Chen-Yu, and I really feel like the compatible solution
>> you suggested would cover both your concerns, and ours.
>
>If using a PHY compatible is the solution, we could just use the
>"ethernet-phy-id." style one, and put in the bogus ID that
>Allwinner used.
>
>Care must be taken to put this at the board level for boards using
>the internal PHY, or we'd have to delete or override the property
>in all other boards.
>
>Ideally I think the internal PHY device node should _not_ be 

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Andre Przywara]

Hi,

On 27/06/17 10:41, Maxime Ripard wrote:
> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>> Hi,
>>
>> (CC:ing some people from that Rockchip dmwac series)
>>
>> On 27/06/17 09:21, Corentin Labbe wrote:
>>> On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
 On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
  wrote:
> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>> On 31/05/17 08:18, Corentin Labbe wrote:
>>> The dwmac-sun8i is a heavy hacked version of stmmac hardware by
>>> allwinner.
>>> In fact the only common part is the descriptor management and the first
>>> register function.
>>
>> Hi,
>>
>> I know I am a bit late with this, but while adapting the U-Boot driver
>> to the new binding I was wondering about the internal PHY detection:
>>
>>
>> So here you seem to deduce the usage of the internal PHY by the PHY
>> interface specified in the DT (MII = internal, RGMII = external).
>> I think I raised this question before, but isn't it perfectly legal for
>> a board to use MII with an external PHY even on those SoCs that feature
>> an internal PHY?
>> On the first glance that does not make too much sense, but apart from
>> not being the correct binding to describe all of the SoCs features I see
>> two scenarios:
>> 1) A board vendor might choose to not use the internal PHY because it
>> has bugs, lacks features (configurability) or has other issues. For
>> instance I have heard reports that the internal PHY makes the SoC go
>> rather hot, possibly limiting the CPU frequency. By using an external
>> MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
>> 2) A PHY does not necessarily need to be directly connected to
>> magnetics. Indeed quite some boards use (RG)MII to connect to a switch
>> IC or some other network circuitry, for instance fibre connectors.
>>
>> So I was wondering if we would need an explicit:
>>   allwinner,use-internal-phy;
>> boolean DT property to signal the usage of the internal PHY?
>> Alternatively we could go with the negative version:
>>   allwinner,disable-internal-phy;
>>
>> Or what about introducing a new "allwinner,internal-mii-phy" compatible
>> string for the *PHY* node and use that?
>>
>> I just want to avoid that we introduce a binding that causes us
>> headaches later. I think we can still fix this with a followup patch
>> before the driver and its binding hit a release kernel.
>>
>> Cheers,
>> Andre.
>>
>
> I just see some patch, where "phy-mode = internal" is valid.
> I will try to find a way to use it

 Can you provide a link?
>>>
>>> https://lkml.org/lkml/2017/6/23/479
>>>

 I'm not a fan of using phy-mode for this. There's no guarantee what
 mode the internal PHY uses. That's what phy-mode is for.
>>
>> I can understand Chen-Yu's concerns, but ...
>>
>>> For each soc the internal PHY mode is know and setted in 
>>> emac_variant/internal_phy
>>> So its not a problem.
>>
>> that is true as well, at least for now.
>>
>> So while I agree that having a separate property to indicate the usage
>> of the internal PHY would be nice, I am bit tempted to use this easier
>> approach and piggy back on the existing phy-mode property.
> 
> We're trying to fix an issue that works for now too.
> 
> If we want to consider future weird cases, then we must consider all
> of them. And the phy mode changing is definitely not really far
> fetched.
> 
> I agree with Chen-Yu, and I really feel like the compatible solution
> you suggested would cover both your concerns, and ours.

So something like this?
emac: emac@1c3 {
compatible = "allwinner,sun8i-h3-emac";
...
phy-mode = "mii";
phy-handle = <_mii_phy>;
...

mdio: mdio {
#address-cells = <1>;
#size-cells = <0>;
int_mii_phy: ethernet-phy@1 {
compatible = "allwinner,sun8i-h3-ephy";
syscon = <>;
reg = <1>;
clocks = < CLK_BUS_EPHY>;
resets = < RST_BUS_EPHY>;
};
};
};

And then move the internal-PHY setup code into a separate PHY driver?

That looks like the architecturally best solution to me, but is probably
also a bit involved since it would require a separate PHY driver.
Or can we make it simpler, but still use this binding?

Cheers,
Andre.

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Chen-Yu Tsai]

On Tue, Jun 27, 2017 at 5:41 PM, Maxime Ripard
 wrote:
> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>> Hi,
>>
>> (CC:ing some people from that Rockchip dmwac series)
>>
>> On 27/06/17 09:21, Corentin Labbe wrote:
>> > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
>> >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>> >>  wrote:
>> >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>>  On 31/05/17 08:18, Corentin Labbe wrote:
>> > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
>> > allwinner.
>> > In fact the only common part is the descriptor management and the first
>> > register function.
>> 
>>  Hi,
>> 
>>  I know I am a bit late with this, but while adapting the U-Boot driver
>>  to the new binding I was wondering about the internal PHY detection:
>> 
>> 
>>  So here you seem to deduce the usage of the internal PHY by the PHY
>>  interface specified in the DT (MII = internal, RGMII = external).
>>  I think I raised this question before, but isn't it perfectly legal for
>>  a board to use MII with an external PHY even on those SoCs that feature
>>  an internal PHY?
>>  On the first glance that does not make too much sense, but apart from
>>  not being the correct binding to describe all of the SoCs features I see
>>  two scenarios:
>>  1) A board vendor might choose to not use the internal PHY because it
>>  has bugs, lacks features (configurability) or has other issues. For
>>  instance I have heard reports that the internal PHY makes the SoC go
>>  rather hot, possibly limiting the CPU frequency. By using an external
>>  MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
>>  2) A PHY does not necessarily need to be directly connected to
>>  magnetics. Indeed quite some boards use (RG)MII to connect to a switch
>>  IC or some other network circuitry, for instance fibre connectors.
>> 
>>  So I was wondering if we would need an explicit:
>>    allwinner,use-internal-phy;
>>  boolean DT property to signal the usage of the internal PHY?
>>  Alternatively we could go with the negative version:
>>    allwinner,disable-internal-phy;
>> 
>>  Or what about introducing a new "allwinner,internal-mii-phy" compatible
>>  string for the *PHY* node and use that?
>> 
>>  I just want to avoid that we introduce a binding that causes us
>>  headaches later. I think we can still fix this with a followup patch
>>  before the driver and its binding hit a release kernel.
>> 
>>  Cheers,
>>  Andre.
>> 
>> >>>
>> >>> I just see some patch, where "phy-mode = internal" is valid.
>> >>> I will try to find a way to use it
>> >>
>> >> Can you provide a link?
>> >
>> > https://lkml.org/lkml/2017/6/23/479
>> >
>> >>
>> >> I'm not a fan of using phy-mode for this. There's no guarantee what
>> >> mode the internal PHY uses. That's what phy-mode is for.
>>
>> I can understand Chen-Yu's concerns, but ...
>>
>> > For each soc the internal PHY mode is know and setted in 
>> > emac_variant/internal_phy
>> > So its not a problem.
>>
>> that is true as well, at least for now.
>>
>> So while I agree that having a separate property to indicate the usage
>> of the internal PHY would be nice, I am bit tempted to use this easier
>> approach and piggy back on the existing phy-mode property.
>
> We're trying to fix an issue that works for now too.
>
> If we want to consider future weird cases, then we must consider all
> of them. And the phy mode changing is definitely not really far
> fetched.

I guess the issue is whether it's likely that the vendor puts 2 internal
PHYs in one SoC, and they use different modes and can be switched around.
Otherwise it's fixed for a given SoC, and we can just handle that with
the per-SoC GMAC compatible.

Maybe Florian could tell us if this was one of the intended use cases
for the "internal" phy mode.

As for Rockchip, AFAIK they have 2 MACs, one is connected to the internal
PHY, while the other is connected to the external interface, and there is
no muxing involved, unlike Allwinner's solution.

> I agree with Chen-Yu, and I really feel like the compatible solution
> you suggested would cover both your concerns, and ours.

If using a PHY compatible is the solution, we could just use the
"ethernet-phy-id." style one, and put in the bogus ID that
Allwinner used.

Care must be taken to put this at the board level for boards using
the internal PHY, or we'd have to delete or override the property
in all other boards.

Ideally I think the internal PHY device node should _not_ be in
the SoC level .dtsi file. If we select the external interface, then
there's no connection to the internal PHY, and that device node becomes
unusable and bogus. This is something I think should be 

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Maxime Ripard]

On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
> Hi,
> 
> (CC:ing some people from that Rockchip dmwac series)
> 
> On 27/06/17 09:21, Corentin Labbe wrote:
> > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
> >>  wrote:
> >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>  On 31/05/17 08:18, Corentin Labbe wrote:
> > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> > allwinner.
> > In fact the only common part is the descriptor management and the first
> > register function.
> 
>  Hi,
> 
>  I know I am a bit late with this, but while adapting the U-Boot driver
>  to the new binding I was wondering about the internal PHY detection:
> 
> 
>  So here you seem to deduce the usage of the internal PHY by the PHY
>  interface specified in the DT (MII = internal, RGMII = external).
>  I think I raised this question before, but isn't it perfectly legal for
>  a board to use MII with an external PHY even on those SoCs that feature
>  an internal PHY?
>  On the first glance that does not make too much sense, but apart from
>  not being the correct binding to describe all of the SoCs features I see
>  two scenarios:
>  1) A board vendor might choose to not use the internal PHY because it
>  has bugs, lacks features (configurability) or has other issues. For
>  instance I have heard reports that the internal PHY makes the SoC go
>  rather hot, possibly limiting the CPU frequency. By using an external
>  MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
>  2) A PHY does not necessarily need to be directly connected to
>  magnetics. Indeed quite some boards use (RG)MII to connect to a switch
>  IC or some other network circuitry, for instance fibre connectors.
> 
>  So I was wondering if we would need an explicit:
>    allwinner,use-internal-phy;
>  boolean DT property to signal the usage of the internal PHY?
>  Alternatively we could go with the negative version:
>    allwinner,disable-internal-phy;
> 
>  Or what about introducing a new "allwinner,internal-mii-phy" compatible
>  string for the *PHY* node and use that?
> 
>  I just want to avoid that we introduce a binding that causes us
>  headaches later. I think we can still fix this with a followup patch
>  before the driver and its binding hit a release kernel.
> 
>  Cheers,
>  Andre.
> 
> >>>
> >>> I just see some patch, where "phy-mode = internal" is valid.
> >>> I will try to find a way to use it
> >>
> >> Can you provide a link?
> > 
> > https://lkml.org/lkml/2017/6/23/479
> > 
> >>
> >> I'm not a fan of using phy-mode for this. There's no guarantee what
> >> mode the internal PHY uses. That's what phy-mode is for.
> 
> I can understand Chen-Yu's concerns, but ...
> 
> > For each soc the internal PHY mode is know and setted in 
> > emac_variant/internal_phy
> > So its not a problem.
> 
> that is true as well, at least for now.
>
> So while I agree that having a separate property to indicate the usage
> of the internal PHY would be nice, I am bit tempted to use this easier
> approach and piggy back on the existing phy-mode property.

We're trying to fix an issue that works for now too.

If we want to consider future weird cases, then we must consider all
of them. And the phy mode changing is definitely not really far
fetched.

I agree with Chen-Yu, and I really feel like the compatible solution
you suggested would cover both your concerns, and ours.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Andre Przywara]

Hi,

(CC:ing some people from that Rockchip dmwac series)

On 27/06/17 09:21, Corentin Labbe wrote:
> On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
>> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>>  wrote:
>>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
 On 31/05/17 08:18, Corentin Labbe wrote:
> The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> allwinner.
> In fact the only common part is the descriptor management and the first
> register function.

 Hi,

 I know I am a bit late with this, but while adapting the U-Boot driver
 to the new binding I was wondering about the internal PHY detection:


 So here you seem to deduce the usage of the internal PHY by the PHY
 interface specified in the DT (MII = internal, RGMII = external).
 I think I raised this question before, but isn't it perfectly legal for
 a board to use MII with an external PHY even on those SoCs that feature
 an internal PHY?
 On the first glance that does not make too much sense, but apart from
 not being the correct binding to describe all of the SoCs features I see
 two scenarios:
 1) A board vendor might choose to not use the internal PHY because it
 has bugs, lacks features (configurability) or has other issues. For
 instance I have heard reports that the internal PHY makes the SoC go
 rather hot, possibly limiting the CPU frequency. By using an external
 MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
 2) A PHY does not necessarily need to be directly connected to
 magnetics. Indeed quite some boards use (RG)MII to connect to a switch
 IC or some other network circuitry, for instance fibre connectors.

 So I was wondering if we would need an explicit:
   allwinner,use-internal-phy;
 boolean DT property to signal the usage of the internal PHY?
 Alternatively we could go with the negative version:
   allwinner,disable-internal-phy;

 Or what about introducing a new "allwinner,internal-mii-phy" compatible
 string for the *PHY* node and use that?

 I just want to avoid that we introduce a binding that causes us
 headaches later. I think we can still fix this with a followup patch
 before the driver and its binding hit a release kernel.

 Cheers,
 Andre.

>>>
>>> I just see some patch, where "phy-mode = internal" is valid.
>>> I will try to find a way to use it
>>
>> Can you provide a link?
> 
> https://lkml.org/lkml/2017/6/23/479
> 
>>
>> I'm not a fan of using phy-mode for this. There's no guarantee what
>> mode the internal PHY uses. That's what phy-mode is for.

I can understand Chen-Yu's concerns, but ...

> For each soc the internal PHY mode is know and setted in 
> emac_variant/internal_phy
> So its not a problem.

that is true as well, at least for now.

So while I agree that having a separate property to indicate the usage
of the internal PHY would be nice, I am bit tempted to use this easier
approach and piggy back on the existing phy-mode property.

Are there any insights from the people involved with the Rockchip
internal PHY?
It is worth to introduce a generic boolean property for an internal PHY?
Or shall we actually move this more into the PHY code, introducing new
compatibles for the internal Allwinner and Rockchip Ethernet PHYs?

Cheers,
Andre.

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Corentin Labbe]

On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
> On 31/05/17 08:18, Corentin Labbe wrote:
> > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> > allwinner.
> > In fact the only common part is the descriptor management and the first
> > register function.
> 
> Hi,
> 
> I know I am a bit late with this, but while adapting the U-Boot driver
> to the new binding I was wondering about the internal PHY detection:
> 
> 
> So here you seem to deduce the usage of the internal PHY by the PHY
> interface specified in the DT (MII = internal, RGMII = external).
> I think I raised this question before, but isn't it perfectly legal for
> a board to use MII with an external PHY even on those SoCs that feature
> an internal PHY?
> On the first glance that does not make too much sense, but apart from
> not being the correct binding to describe all of the SoCs features I see
> two scenarios:
> 1) A board vendor might choose to not use the internal PHY because it
> has bugs, lacks features (configurability) or has other issues. For
> instance I have heard reports that the internal PHY makes the SoC go
> rather hot, possibly limiting the CPU frequency. By using an external
> MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
> 2) A PHY does not necessarily need to be directly connected to
> magnetics. Indeed quite some boards use (RG)MII to connect to a switch
> IC or some other network circuitry, for instance fibre connectors.
> 
> So I was wondering if we would need an explicit:
>   allwinner,use-internal-phy;
> boolean DT property to signal the usage of the internal PHY?
> Alternatively we could go with the negative version:
>   allwinner,disable-internal-phy;
> 
> Or what about introducing a new "allwinner,internal-mii-phy" compatible
> string for the *PHY* node and use that?
> 
> I just want to avoid that we introduce a binding that causes us
> headaches later. I think we can still fix this with a followup patch
> before the driver and its binding hit a release kernel.
> 
> Cheers,
> Andre.
> 

I just see some patch, where "phy-mode = internal" is valid.
I will try to find a way to use it

Regards

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Corentin Labbe]

On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>  wrote:
> > On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
> >> On 31/05/17 08:18, Corentin Labbe wrote:
> >> > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> >> > allwinner.
> >> > In fact the only common part is the descriptor management and the first
> >> > register function.
> >>
> >> Hi,
> >>
> >> I know I am a bit late with this, but while adapting the U-Boot driver
> >> to the new binding I was wondering about the internal PHY detection:
> >>
> >>
> >> So here you seem to deduce the usage of the internal PHY by the PHY
> >> interface specified in the DT (MII = internal, RGMII = external).
> >> I think I raised this question before, but isn't it perfectly legal for
> >> a board to use MII with an external PHY even on those SoCs that feature
> >> an internal PHY?
> >> On the first glance that does not make too much sense, but apart from
> >> not being the correct binding to describe all of the SoCs features I see
> >> two scenarios:
> >> 1) A board vendor might choose to not use the internal PHY because it
> >> has bugs, lacks features (configurability) or has other issues. For
> >> instance I have heard reports that the internal PHY makes the SoC go
> >> rather hot, possibly limiting the CPU frequency. By using an external
> >> MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
> >> 2) A PHY does not necessarily need to be directly connected to
> >> magnetics. Indeed quite some boards use (RG)MII to connect to a switch
> >> IC or some other network circuitry, for instance fibre connectors.
> >>
> >> So I was wondering if we would need an explicit:
> >>   allwinner,use-internal-phy;
> >> boolean DT property to signal the usage of the internal PHY?
> >> Alternatively we could go with the negative version:
> >>   allwinner,disable-internal-phy;
> >>
> >> Or what about introducing a new "allwinner,internal-mii-phy" compatible
> >> string for the *PHY* node and use that?
> >>
> >> I just want to avoid that we introduce a binding that causes us
> >> headaches later. I think we can still fix this with a followup patch
> >> before the driver and its binding hit a release kernel.
> >>
> >> Cheers,
> >> Andre.
> >>
> >
> > I just see some patch, where "phy-mode = internal" is valid.
> > I will try to find a way to use it
> 
> Can you provide a link?

https://lkml.org/lkml/2017/6/23/479

> 
> I'm not a fan of using phy-mode for this. There's no guarantee what
> mode the internal PHY uses. That's what phy-mode is for.

For each soc the internal PHY mode is know and setted in 
emac_variant/internal_phy
So its not a problem.

Patch comming soon

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Chen-Yu Tsai]

On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
 wrote:
> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>> On 31/05/17 08:18, Corentin Labbe wrote:
>> > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
>> > allwinner.
>> > In fact the only common part is the descriptor management and the first
>> > register function.
>>
>> Hi,
>>
>> I know I am a bit late with this, but while adapting the U-Boot driver
>> to the new binding I was wondering about the internal PHY detection:
>>
>>
>> So here you seem to deduce the usage of the internal PHY by the PHY
>> interface specified in the DT (MII = internal, RGMII = external).
>> I think I raised this question before, but isn't it perfectly legal for
>> a board to use MII with an external PHY even on those SoCs that feature
>> an internal PHY?
>> On the first glance that does not make too much sense, but apart from
>> not being the correct binding to describe all of the SoCs features I see
>> two scenarios:
>> 1) A board vendor might choose to not use the internal PHY because it
>> has bugs, lacks features (configurability) or has other issues. For
>> instance I have heard reports that the internal PHY makes the SoC go
>> rather hot, possibly limiting the CPU frequency. By using an external
>> MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
>> 2) A PHY does not necessarily need to be directly connected to
>> magnetics. Indeed quite some boards use (RG)MII to connect to a switch
>> IC or some other network circuitry, for instance fibre connectors.
>>
>> So I was wondering if we would need an explicit:
>>   allwinner,use-internal-phy;
>> boolean DT property to signal the usage of the internal PHY?
>> Alternatively we could go with the negative version:
>>   allwinner,disable-internal-phy;
>>
>> Or what about introducing a new "allwinner,internal-mii-phy" compatible
>> string for the *PHY* node and use that?
>>
>> I just want to avoid that we introduce a binding that causes us
>> headaches later. I think we can still fix this with a followup patch
>> before the driver and its binding hit a release kernel.
>>
>> Cheers,
>> Andre.
>>
>
> I just see some patch, where "phy-mode = internal" is valid.
> I will try to find a way to use it

Can you provide a link?

I'm not a fan of using phy-mode for this. There's no guarantee what
mode the internal PHY uses. That's what phy-mode is for.

In any case, we should fix this before 4.13 is released.

ChenYu

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[André Przywara]

On 31/05/17 08:18, Corentin Labbe wrote:
> The dwmac-sun8i is a heavy hacked version of stmmac hardware by
> allwinner.
> In fact the only common part is the descriptor management and the first
> register function.

Hi,

I know I am a bit late with this, but while adapting the U-Boot driver
to the new binding I was wondering about the internal PHY detection:

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> new file mode 100644
> index ..1a6bfe6c958f
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -0,0 +1,990 @@



> +static int sun8i_dwmac_probe(struct platform_device *pdev)
> +{
> + struct plat_stmmacenet_data *plat_dat;
> + struct stmmac_resources stmmac_res;
> + struct sunxi_priv_data *gmac;
> + struct device *dev = >dev;
> + int ret;
> +
> + ret = stmmac_get_platform_resources(pdev, _res);
> + if (ret)
> + return ret;
> +
> + plat_dat = stmmac_probe_config_dt(pdev, _res.mac);
> + if (IS_ERR(plat_dat))
> + return PTR_ERR(plat_dat);
> +
> + gmac = devm_kzalloc(dev, sizeof(*gmac), GFP_KERNEL);
> + if (!gmac)
> + return -ENOMEM;
> +
> + gmac->variant = of_device_get_match_data(>dev);
> + if (!gmac->variant) {
> + dev_err(>dev, "Missing dwmac-sun8i variant\n");
> + return -EINVAL;
> + }
> +
> + gmac->tx_clk = devm_clk_get(dev, "stmmaceth");
> + if (IS_ERR(gmac->tx_clk)) {
> + dev_err(dev, "Could not get TX clock\n");
> + return PTR_ERR(gmac->tx_clk);
> + }
> +
> + /* Optional regulator for PHY */
> + gmac->regulator = devm_regulator_get_optional(dev, "phy");
> + if (IS_ERR(gmac->regulator)) {
> + if (PTR_ERR(gmac->regulator) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_info(dev, "No regulator found\n");
> + gmac->regulator = NULL;
> + }
> +
> + gmac->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +"syscon");
> + if (IS_ERR(gmac->regmap)) {
> + ret = PTR_ERR(gmac->regmap);
> + dev_err(>dev, "Unable to map syscon: %d\n", ret);
> + return ret;
> + }
> +
> + plat_dat->interface = of_get_phy_mode(dev->of_node);
> + if (plat_dat->interface == gmac->variant->internal_phy) {
> + dev_info(>dev, "Will use internal PHY\n");

So here you seem to deduce the usage of the internal PHY by the PHY
interface specified in the DT (MII = internal, RGMII = external).
I think I raised this question before, but isn't it perfectly legal for
a board to use MII with an external PHY even on those SoCs that feature
an internal PHY?
On the first glance that does not make too much sense, but apart from
not being the correct binding to describe all of the SoCs features I see
two scenarios:
1) A board vendor might choose to not use the internal PHY because it
has bugs, lacks features (configurability) or has other issues. For
instance I have heard reports that the internal PHY makes the SoC go
rather hot, possibly limiting the CPU frequency. By using an external
MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
2) A PHY does not necessarily need to be directly connected to
magnetics. Indeed quite some boards use (RG)MII to connect to a switch
IC or some other network circuitry, for instance fibre connectors.

So I was wondering if we would need an explicit:
allwinner,use-internal-phy;
boolean DT property to signal the usage of the internal PHY?
Alternatively we could go with the negative version:
allwinner,disable-internal-phy;

Or what about introducing a new "allwinner,internal-mii-phy" compatible
string for the *PHY* node and use that?

I just want to avoid that we introduce a binding that causes us
headaches later. I think we can still fix this with a followup patch
before the driver and its binding hit a release kernel.

Cheers,
Andre.

> + gmac->use_internal_phy = true;
> + gmac->ephy_clk = of_clk_get(plat_dat->phy_node, 0);
> + if (IS_ERR(gmac->ephy_clk)) {
> + ret = PTR_ERR(gmac->ephy_clk);
> + dev_err(>dev, "Cannot get EPHY clock: %d\n", ret);
> + return -EINVAL;
> + }
> +
> + gmac->rst_ephy = of_reset_control_get(plat_dat->phy_node, NULL);
> + if (IS_ERR(gmac->rst_ephy)) {
> + ret = PTR_ERR(gmac->rst_ephy);
> + if (ret == -EPROBE_DEFER)
> + return ret;
> + dev_err(>dev, "No EPHY reset control found %d\n",
> + ret);
> + return -EINVAL;
> + }
> + } else {
> + dev_info(>dev, "Will use external PHY\n");
> + 

[PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Corentin Labbe]

The dwmac-sun8i is a heavy hacked version of stmmac hardware by
allwinner.
In fact the only common part is the descriptor management and the first
register function.

Signed-off-by: Corentin Labbe 
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig|  11 +
 drivers/net/ethernet/stmicro/stmmac/Makefile   |   1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  | 990 +
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  15 +
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   9 +-
 include/linux/stmmac.h |   1 +
 6 files changed, 1025 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig 
b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index cfbe3634dfa1..85c0e41f8021 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -145,6 +145,17 @@ config DWMAC_SUNXI
  This selects Allwinner SoC glue layer support for the
  stmmac device driver. This driver is used for A20/A31
  GMAC ethernet controller.
+
+config DWMAC_SUN8I
+   tristate "Allwinner sun8i GMAC support"
+   default ARCH_SUNXI
+   depends on OF && (ARCH_SUNXI || COMPILE_TEST)
+   ---help---
+ Support for Allwinner H3 A83T A64 EMAC ethernet controllers.
+
+ This selects Allwinner SoC glue layer support for the
+ stmmac device driver. This driver is used for H3/A83T/A64
+ EMAC ethernet controller.
 endif
 
 config STMMAC_PCI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile 
b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 700c60336674..fd4937a7fcab 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA)   += dwmac-altr-socfpga.o
 obj-$(CONFIG_DWMAC_STI)+= dwmac-sti.o
 obj-$(CONFIG_DWMAC_STM32)  += dwmac-stm32.o
 obj-$(CONFIG_DWMAC_SUNXI)  += dwmac-sunxi.o
+obj-$(CONFIG_DWMAC_SUN8I)  += dwmac-sun8i.o
 obj-$(CONFIG_DWMAC_DWC_QOS_ETH)+= dwmac-dwc-qos-eth.o
 obj-$(CONFIG_DWMAC_GENERIC)+= dwmac-generic.o
 stmmac-platform-objs:= stmmac_platform.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
new file mode 100644
index ..1a6bfe6c958f
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -0,0 +1,990 @@
+/*
+ * dwmac-sun8i.c - Allwinner sun8i DWMAC specific glue layer
+ *
+ * Copyright (C) 2017 Corentin Labbe 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "stmmac.h"
+#include "stmmac_platform.h"
+
+/* General notes on dwmac-sun8i:
+ * Locking: no locking is necessary in this file because all necessary locking
+ * is done in the "stmmac files"
+ */
+
+/* struct emac_variant - Descrive dwmac-sun8i hardware variant
+ * @default_syscon_value:  The default value of the EMAC register in syscon
+ * This value is used for disabling properly EMAC
+ * and used as a good starting value in case of the
+ * boot process(uboot) leave some stuff.
+ * @internal_phy:  Does the MAC embed an internal PHY
+ * @support_mii:   Does the MAC handle MII
+ * @support_rmii:  Does the MAC handle RMII
+ * @support_rgmii: Does the MAC handle RGMII
+ */
+struct emac_variant {
+   u32 default_syscon_value;
+   int internal_phy;
+   bool support_mii;
+   bool support_rmii;
+   bool support_rgmii;
+};
+
+/* struct sunxi_priv_data - hold all sunxi private data
+ * @tx_clk:reference to MAC TX clock
+ * @ephy_clk:  reference to the optional EPHY clock for the internal PHY
+ * @regulator: reference to the optional regulator
+ * @rst_ephy:  reference to the optional EPHY reset for the internal PHY
+ * @variant:   reference to the current board variant
+ * @regmap:regmap for using the syscon
+ * @use_internal_phy: Does the current PHY choice imply using the internal PHY
+ */
+struct sunxi_priv_data {
+   struct clk *tx_clk;
+   struct clk *ephy_clk;
+   struct regulator *regulator;
+   struct