All of lore.kernel.org
 help / color / mirror / Atom feed
* etsec2 attached to sgmii phy
@ 2017-10-04  5:56 Jörg Willmann
  2017-10-04 13:36 ` Andrew Lunn
  0 siblings, 1 reply; 6+ messages in thread
From: Jörg Willmann @ 2017-10-04  5:56 UTC (permalink / raw)
  To: netdev

Hi,

we use a QorIQ P1011 connected via SGMII to a switch (Marvell 88E6352). 
Currently we still use a really old linux kernel (2.6.33) successfully.

For configuration of the MDIO Bus attached to the corresponding eTSEC/TBI 
Phy we use the following settings in the device tree:

                         mdio@25000 {
                                    #address-cells = <0x1>;
                                    #size-cells = <0x0>;
                                    compatible = "fsl,etsec2-tbi";
                                    reg = <0x25000 0x1000 0xb1030 0x4>;

                                    tbi-phy@11 {
                                                reg = <0x11>;
                                                device_type = "tbi-phy";
                                                linux,phandle = <0x5>;
                                                phandle = <0x5>;
                                    };
                         };

First resource in the mdio config is the base address of the MDIO bus, the 
second resource defines the address of TBIPA.

Doing tests with a newer kernel (4.4.71)  I was faced with problems 
communicating with the TBIPHY.

Debugging this I found out that TBIPA is no longer parameterized correctly 
and I found out why:

In commit afae5ad78b342f401c28b0bb1adb3cd494cb125a initialization of TIBPA 
has been changed:

In former versions in get_gfar_tbipa(...) for compatible "etsec2-tbi" the 
second resource of the reg entry has been used. But now only first 
resource is mapped and get_etsec_tbipa(...) which is now
responsible to return the proper address does not (and has no way due to 
missing *np as function argument) do so but returns the first resource 
(which is the base address of mdio controller).

What do you think would be the correct way to solve this issue? Re-add *np 
to the get_*_tba() functions to be able to return the proper address?

Thank's for reading,
Best regards,

Joerg

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: etsec2 attached to sgmii phy
  2017-10-04  5:56 etsec2 attached to sgmii phy Jörg Willmann
@ 2017-10-04 13:36 ` Andrew Lunn
  2017-10-04 14:19   ` Jörg Willmann
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2017-10-04 13:36 UTC (permalink / raw)
  To: Jörg Willmann; +Cc: netdev

On Wed, Oct 04, 2017 at 07:56:53AM +0200, Jörg Willmann wrote:
> Hi,
> 
> we use a QorIQ P1011 connected via SGMII to a switch (Marvell 88E6352).
> Currently we still use a really old linux kernel (2.6.33) successfully.
> 
> For configuration of the MDIO Bus attached to the corresponding eTSEC/TBI
> Phy we use the following settings in the device tree:
> 
>                         mdio@25000 {
>                                    #address-cells = <0x1>;
>                                    #size-cells = <0x0>;
>                                    compatible = "fsl,etsec2-tbi";
>                                    reg = <0x25000 0x1000 0xb1030 0x4>;

Hi Joerg

Is 0xb1030 0x4 fixed by the silicon? Can it be expressed as an offset from
0x25000?

It seems like the idea behind the patch is to hard code some
things. If you can hard code the offset into get_etsec_tbipa(), i
think that would be an O.K. solution to your problem.

      Andrew

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: etsec2 attached to sgmii phy
  2017-10-04 13:36 ` Andrew Lunn
@ 2017-10-04 14:19   ` Jörg Willmann
  2017-10-04 15:34     ` Andrew Lunn
  0 siblings, 1 reply; 6+ messages in thread
From: Jörg Willmann @ 2017-10-04 14:19 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev

[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]

On Wed, 4 Oct 2017, Andrew Lunn wrote:

> On Wed, Oct 04, 2017 at 07:56:53AM +0200, Jörg Willmann wrote:
>> Hi,
>>
>> we use a QorIQ P1011 connected via SGMII to a switch (Marvell 88E6352).
>> Currently we still use a really old linux kernel (2.6.33) successfully.
>>
>> For configuration of the MDIO Bus attached to the corresponding eTSEC/TBI
>> Phy we use the following settings in the device tree:
>>
>>                         mdio@25000 {
>>                                    #address-cells = <0x1>;
>>                                    #size-cells = <0x0>;
>>                                    compatible = "fsl,etsec2-tbi";
>>                                    reg = <0x25000 0x1000 0xb1030 0x4>;
>
> Hi Joerg
>
> Is 0xb1030 0x4 fixed by the silicon? Can it be expressed as an offset from
> 0x25000?
>
> It seems like the idea behind the patch is to hard code some
> things. If you can hard code the offset into get_etsec_tbipa(), i
> think that would be an O.K. solution to your problem.
>
>      Andrew
>
Yes, the adress 0xb1030 is fixed but it's something totally different than 
the address range of 0x25000. 0xb0000, 0xb1000 and 0xb2000 are base 
addresses of the eTSEC MAC (TPIPA is a register within the MAC) and 
0x24000, 0x25000 and 0x26000 are the base registers of the corresponding 
MDIO controllers. So I wouldn't add a dependency between these two things.
>From my point of view, the implementation in the old kernel where 
get_gfar_tbipa() got the device tree node pointer as argument was not soo 
bad ;-)

 	Joerg

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: etsec2 attached to sgmii phy
  2017-10-04 14:19   ` Jörg Willmann
@ 2017-10-04 15:34     ` Andrew Lunn
  2017-10-04 16:40       ` Jörg Willmann
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2017-10-04 15:34 UTC (permalink / raw)
  To: Jörg Willmann; +Cc: netdev

On Wed, Oct 04, 2017 at 04:19:23PM +0200, Jörg Willmann wrote:
> On Wed, 4 Oct 2017, Andrew Lunn wrote:
> 
> >On Wed, Oct 04, 2017 at 07:56:53AM +0200, Jörg Willmann wrote:
> >>Hi,
> >>
> >>we use a QorIQ P1011 connected via SGMII to a switch (Marvell 88E6352).
> >>Currently we still use a really old linux kernel (2.6.33) successfully.
> >>
> >>For configuration of the MDIO Bus attached to the corresponding eTSEC/TBI
> >>Phy we use the following settings in the device tree:
> >>
> >>                        mdio@25000 {
> >>                                   #address-cells = <0x1>;
> >>                                   #size-cells = <0x0>;
> >>                                   compatible = "fsl,etsec2-tbi";
> >>                                   reg = <0x25000 0x1000 0xb1030 0x4>;
> >
> >Hi Joerg
> >
> >Is 0xb1030 0x4 fixed by the silicon? Can it be expressed as an offset from
> >0x25000?
> >
> >It seems like the idea behind the patch is to hard code some
> >things. If you can hard code the offset into get_etsec_tbipa(), i
> >think that would be an O.K. solution to your problem.
> >
> >     Andrew
> >
> Yes, the adress 0xb1030 is fixed but it's something totally different than
> the address range of 0x25000. 0xb0000, 0xb1000 and 0xb2000 are base
> addresses of the eTSEC MAC (TPIPA is a register within the MAC) and 0x24000,
> 0x25000 and 0x26000 are the base registers of the corresponding MDIO
> controllers. So I wouldn't add a dependency between these two things.
> >From my point of view, the implementation in the old kernel where
> get_gfar_tbipa() got the device tree node pointer as argument was not soo
> bad ;-)

I took a quick look at the current device tree files. They all seem to
have the 0xb1030 0x4. So reading it inside of get_etsec_tbipa() is
O.K.

Looks like you need to modify all the get_tbipa() functions to take a
device_node *, and this code looks like it needs to change:

                        /*
                         * Add consistency check to make sure TBI is contained
                         * within the mapped range (not because we would get a
                         * segfault, rather to catch bugs in computing TBI
                         * address). Print error message but continue anyway.
                         */
                        if ((void *)tbipa > priv->map + resource_size(&res) - 4)
                                dev_err(&pdev->dev, "invalid register map (should be at least 0x%04zx to contain TBI address)\n",
                                        ((void *)tbipa - priv->map) + 4);

                        iowrite32be(be32_to_cpup(prop), tbipa);

	Andrew

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: etsec2 attached to sgmii phy
  2017-10-04 15:34     ` Andrew Lunn
@ 2017-10-04 16:40       ` Jörg Willmann
  0 siblings, 0 replies; 6+ messages in thread
From: Jörg Willmann @ 2017-10-04 16:40 UTC (permalink / raw)
  To: Andrew Lunn, netdev; +Cc: timur

Am 04.10.2017 um 17:34 schrieb Andrew Lunn:
> On Wed, Oct 04, 2017 at 04:19:23PM +0200, Jörg Willmann wrote:
>> On Wed, 4 Oct 2017, Andrew Lunn wrote:
>>
>>> On Wed, Oct 04, 2017 at 07:56:53AM +0200, Jörg Willmann wrote:
>>>> Hi,
>>>>
>>>> we use a QorIQ P1011 connected via SGMII to a switch (Marvell 88E6352).
>>>> Currently we still use a really old linux kernel (2.6.33) successfully.
>>>>
>>>> For configuration of the MDIO Bus attached to the corresponding eTSEC/TBI
>>>> Phy we use the following settings in the device tree:
>>>>
>>>>                         mdio@25000 {
>>>>                                    #address-cells = <0x1>;
>>>>                                    #size-cells = <0x0>;
>>>>                                    compatible = "fsl,etsec2-tbi";
>>>>                                    reg = <0x25000 0x1000 0xb1030 0x4>;
>>> Hi Joerg
>>>
>>> Is 0xb1030 0x4 fixed by the silicon? Can it be expressed as an offset from
>>> 0x25000?
>>>
>>> It seems like the idea behind the patch is to hard code some
>>> things. If you can hard code the offset into get_etsec_tbipa(), i
>>> think that would be an O.K. solution to your problem.
>>>
>>>      Andrew
>>>
>> Yes, the adress 0xb1030 is fixed but it's something totally different than
>> the address range of 0x25000. 0xb0000, 0xb1000 and 0xb2000 are base
>> addresses of the eTSEC MAC (TPIPA is a register within the MAC) and 0x24000,
>> 0x25000 and 0x26000 are the base registers of the corresponding MDIO
>> controllers. So I wouldn't add a dependency between these two things.
>> >From my point of view, the implementation in the old kernel where
>> get_gfar_tbipa() got the device tree node pointer as argument was not soo
>> bad ;-)
> I took a quick look at the current device tree files. They all seem to
> have the 0xb1030 0x4. So reading it inside of get_etsec_tbipa() is
> O.K.
>
> Looks like you need to modify all the get_tbipa() functions to take a
> device_node *, and this code looks like it needs to change:
>
>                          /*
>                           * Add consistency check to make sure TBI is contained
>                           * within the mapped range (not because we would get a
>                           * segfault, rather to catch bugs in computing TBI
>                           * address). Print error message but continue anyway.
>                           */
>                          if ((void *)tbipa > priv->map + resource_size(&res) - 4)
>                                  dev_err(&pdev->dev, "invalid register map (should be at least 0x%04zx to contain TBI address)\n",
>                                          ((void *)tbipa - priv->map) + 4);
>
>                          iowrite32be(be32_to_cpup(prop), tbipa);
>
> 	Andrew
>
Yes, exactly - I already stumbled over these lines, too. Are there any 
suggestions how to implement this the best way?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* etsec2 attached to sgmii phy
@ 2017-10-04  5:50 Jörg Willmann
  0 siblings, 0 replies; 6+ messages in thread
From: Jörg Willmann @ 2017-10-04  5:50 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]

Hi,

we use a QorIQ P1011 connected via SGMII to a switch (Marvell 88E6352). 
Currently we still use a really old linux kernel (2.6.33) successfully.

For configuration of the MDIO Bus attached to the corresponding eTSEC/TBI 
Phy we use the following settings in the device tree:

                         mdio@25000 {
                                    #address-cells = <0x1>;
                                    #size-cells = <0x0>;
                                    compatible = "fsl,etsec2-tbi";
                                    reg = <0x25000 0x1000 0xb1030 0x4>;

                                    tbi-phy@11 {
                                                reg = <0x11>;
                                                device_type = "tbi-phy";
                                                linux,phandle = <0x5>;
                                                phandle = <0x5>;
                                    };
                         };

First resource in the mdio config is the base address of the MDIO bus, the 
second resource defines the address of TBIPA.

Doing tests with a newer kernel (4.4.71)  I was faced with problems 
communicating with the TBIPHY.

Debugging this I found out that TBIPA is no longer parameterized correctly 
and I found out why:

In commit afae5ad78b342f401c28b0bb1adb3cd494cb125a initialization of TIBPA 
has been changed:

In former versions in get_gfar_tbipa(...) for compatible "etsec2-tbi" the 
second resource of the reg entry has been used. But now only first resource 
is mapped and get_etsec_tbipa(...) which is now responsible to return the 
proper address does not (and has no way due to missing *np as function 
argument) do so but returns the first resource (which is the base 
address of mdio controller).

What do you think would be the correct way to solve this issue? Re-add *np 
to the get_*_tba() functions to be able to return the proper address?

Thank’s for reading,
Best regards,

Joerg


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-10-04 16:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-04  5:56 etsec2 attached to sgmii phy Jörg Willmann
2017-10-04 13:36 ` Andrew Lunn
2017-10-04 14:19   ` Jörg Willmann
2017-10-04 15:34     ` Andrew Lunn
2017-10-04 16:40       ` Jörg Willmann
  -- strict thread matches above, loose matches on Subject: below --
2017-10-04  5:50 Jörg Willmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.