On Monday 25 February 2013 21:30:07 Tijs Van Buggenhout wrote:
> On Monday 25 February 2013 21:07:09 Hauke Mehrtens wrote:
> > On 02/25/2013 03:08 PM, Tijs Van Buggenhout wrote:
> > > On Monday 25 February 2013 14:06:13 Jonas Gorski wrote:
> > >> On 21 February 2013 15:53, Hauke Mehrtens <ha...@hauke-m.de> wrote:
> > >>> On 02/21/2013 02:57 PM, Tijs Van Buggenhout wrote:
> > >>>> From linksys firmware :
> > >>>> 
> > >>>> # et -i eth0 robord 0x0 0x0e
> > >>>> 0x0000008b
> > >>>> # et -i eth0 robord 0x0 0x12
> > >>>> 0x0000020b
> > >>>> 
> > >>>> root@OpenWrt:/# robocfg robord 0x0e
> > >>>> Page 0x00, Reg 0x0e: 00bb
> > >>>> root@OpenWrt:/# robocfg robord 0x12
> > >>>> Page 0x00, Reg 0x12: 0321
> > >>>> 
> > >>>> 1. Fix led control register:
> > >>>> 
> > >>>> On normal ports only one led was used for both link and activity
> > >>>> (green),
> > >>>> internet port additionally had orange led on all the time.
> > >>>> 
> > >>>> I compared the value of registry 0x12 on linksys firmware vs openwrt,
> > >>>> and
> > >>>> it showed a different value (0x20b for linksys vs 0x321 on openwrt).
> > >>>> 
> > >>>> Now the green led is always used to signal link, the orange led
> > >>>> (only)
> > >>>> blinks on link activity.
> > >>>> 
> > >>>> 2. Adjust IMP port status
> > >>>> 
> > >>>> As I was comparing register values/bcmrobo.c source code, I noticed a
> > >>>> difference for the IMP port status register (0x0e), linksys firmware
> > >>>> showed
> > >>>> 0x8b vs 0xbb for openwrt.
> > >>>> 
> > >>>> From bcmrobo.c - int bcm_robo_enable_switch:
> > >>>>         if ((robo->devid == DEVID53115) || (robo->devid ==
> > >>>>         DEVID53125))
> > >>>>         {
> > >>>>         
> > >>>>                 /* Over ride IMP port status to make it link by
> > >>>>                 default
> > >>>>                 */
> > >>>>                 val8 = 0;
> > >>>>                 robo->ops->read_reg(robo, PAGE_CTRL, REG_CTRL_MIIPO,
> > >>>>                 &val8,
> > >>>> 
> > >>>> sizeof(val8));
> > >>>> 
> > >>>>                 val8 |= 0x81;   /* Make Link pass and override it. */
> > >>>>                 robo->ops->write_reg(robo, PAGE_CTRL, REG_CTRL_MIIPO,
> > >>>>                 &val8,
> > >>>> 
> > >>>> sizeof(val8));
> > >>>> 
> > >>>>         }
> > >>>> 
> > >>>> Hauke, did you use '0xb1' intentionally instead of '0x81'? Or do we
> > >>>> want
> > >>>> this to be the same?
> > >>> 
> > >>> I used the Asus ac66u source code as a reference as this contains the
> > >>> most recent source code I know of. There val8 |= 0xb1; is used.
> > >> 
> > >> Bits 5 and 6 enable rx/tx flow control - not sure if this is
> > >> potentially harmfull or not.
> > >> 
> > >>> Does chaining this to 0x81 changes the behavior of your device?
> > >>> 
> > >>>> Signed-off-by: Tijs Van Buggenhout <t...@able.be>
> > >>>> 
> > >>>> diff --git a/package/switch/src/switch-robo.c
> > >>>> b/package/switch/src/switch-
> > >>>> robo.c
> > >>>> index f715972..ac4855a 100644
> > >>>> --- a/package/switch/src/switch-robo.c
> > >>>> +++ b/package/switch/src/switch-robo.c
> > >>>> @@ -252,8 +252,13 @@ static int robo_switch_enable(void)
> > >>>> 
> > >>>>         if (robo.devid == ROBO_DEVICE_ID_53125) {
> > >>>>         
> > >>>>                 /* Make IM port status link by default */
> > >>>> 
> > >>>> -               val = robo_read16(ROBO_CTRL_PAGE,
> > >>>> ROBO_PORT_OVERRIDE_CTRL) | 0xb1;
> > >>>> +               val = robo_read16(ROBO_CTRL_PAGE,
> > >>>> ROBO_PORT_OVERRIDE_CTRL) | 0x81;
> > >>>> 
> > >>>>                 robo_write16(ROBO_CTRL_PAGE, ROBO_PORT_OVERRIDE_CTRL,
> > >>>>                 val);
> > >>>> 
> > >>>> +
> > >>>> +               /* Write LED control register */
> > >>>> +               val = 0x020b;
> > >>>> +               robo_write16(ROBO_CTRL_PAGE, 0x12, val);
> > >>>> +
> > >>> 
> > >>> With this change my Asus n66u does not blink on activity any more.
> > >> 
> > >> Maybe there is some nvram value for it? the led configuration is
> > >> likely device specific, not chip specific.
> > >> 
> > >> 
> > >> Jonas
> > > 
> > > 'et_swleds' nvram variable seems to be intended for this purpose, but
> > > doesn't (seem to) exist by default for the e3200.. is it for the asus?
> > 
> > No, et_swleds is not set on any of my devices.
> > This code change is probably the device specific "configuration".
> > 
> > I found the following code in the switch driver and it is missing in
> > kmod-switch. You could try to implement that, si_pmu_chipcontrol() is
> > the same as bcma_chipco_chipctl_maskset() expect that you have to
> > inverse the mask parameter.
> > 
> > /* Enable switch leds */
> > if (sih->chip == BCM5356_CHIP_ID) {
> > 
> >     si_pmu_chipcontrol(sih, 2, (1 << 25), (1 << 25));
> >     /* also enable fast MII clocks */
> >     si_pmu_chipcontrol(sih, 0, (1 << 1), (1 << 1));
> > 
> > } else if ((sih->chip == BCM5357_CHIP_ID) || (sih->chip ==
> > BCM53572_CHIP_ID)) {
> > 
> >     uint32 led_gpios = 0;
> >     const char *var;
> >     
> >     if (((sih->chip == BCM5357_CHIP_ID) && (sih->chippkg !=
> > 
> > BCM47186_PKG_ID)) ||
> > 
> >         ((sih->chip == BCM53572_CHIP_ID) && (sih->chippkg !=
> >         BCM47188_PKG_ID)))
> > 
> > led_gpios = 0x1f;
> > 
> >     var = getvar(vars, "et_swleds");
> >     if (var)
> >     
> >             led_gpios = bcm_strtoul(var, NULL, 0);
> >     
> >     if (led_gpios)
> >     
> >             si_pmu_chipcontrol(sih, 2, (0x3ff << 8), (led_gpios << 8));
> >     
> >     }
> 
> I was looking at the same code, tried even patching in bcma code, but then
> it occured to me that the e3200 seems to have BCM47186_PKG_ID. The leds are
> enabled by default on the switch, and thus don't need to be explicitely
> enabled I guess.. I tried the following patch:
> 
> --- a/drivers/bcma/driver_chipcommon_pmu.c.orig       2013-02-20
> 12:41:03.026487835 +0100
> +++ b/drivers/bcma/driver_chipcommon_pmu.c    2013-02-25 17:16:01.072808066
> +0100
> @@ -139,6 +139,24 @@
>                                                    
> BCMA_CCTRL_43224B0_12MA_LED_DRIVE); }
>                 break;
> +       case BCMA_CHIP_ID_BCM5357:
> +               if (bus->chipinfo.pkg != BCMA_PKG_ID_BCM47186) {
> +                       bcma_warn(bus, "Workarounds for BCM5357 and not pkg
> id 47186 (LEDS)\n");
> +                       /* enable LEDs */
> +                       bcma_chipco_chipctl_maskset(cc, 2, (0x3ff << 8),
> (0x1f << 8));
> +               } else {
> +                       bcma_warn(bus, "Workarounds for BCM5357, but this is
> pkg id 47186 : do nothing\n");
> +               }
> +               break;
> +       case BCMA_CHIP_ID_BCM53572:
> +               if (bus->chipinfo.pkg != BCMA_PKG_ID_BCM47188) {
> +                       bcma_warn(bus, "Workarounds for BCM53572 and not pkg
> id 47188 (LEDS)\n");
> +                       /* enable LEDs */
> +                       bcma_chipco_chipctl_maskset(cc, 2, (0x3ff << 8),
> (0x1f << 8));
> +               } else {
> +                       bcma_warn(bus, "Workarounds for BCM53572, but this
> is pkg id 47188 : do nothing\n");
> +               }
> +               break;
>         default:
>                 bcma_debug(bus, "Workarounds unknown or not needed for
> device 0x%04X\n",
>                            bus->chipinfo.id);
> 
> Been looking at it some further, and somehow remembered that a few weeks
> back, when the switch <-> bgmac driver was not yet working (for me), I
> never saw any strange led behaviour, although the link/act leds were
> working. So I tried to narrow down the driver changes from then until now,
> and got it down to the gpio reset code...
> 
> robo.gpio_robo_reset = get_gpio_pin("robo_reset");
> if (robo.gpio_robo_reset >= 0) {
> ....
> }
> 
> When I leave out this code, leds are behaving correctly, and register 0x12
> reads 0x20b by default...
> 
> Will have to investigate some further.
> 
> Regards,
> Tijs
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

I made a dump of ROBO_CTL_PAGE page 0 (first 4096 bits) right before (lines
starting with -) the switch is reset, and right after (lines starting with +) 
- in switch-robo.c - and once again when the system is up (lines starting with
with *) - using robocfg dump.

static int robo_probe(char *devname)
...
robo.gpio_robo_reset = get_gpio_pin("robo_reset");
if (robo.gpio_robo_reset >= 0) {
   <print registers>
   gpio_set_value(robo.gpio_robo_reset, 0);
   gpio_direction_output(robo.gpio_robo_reset, 1);
   ....
   gpio_set_value(robo.gpio_robo_reset, 1);
   mdelay(20);
   <print registers>
}
...

- 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0006 FFFF 0000 008B 
0083
+ 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0006 FFFF 0000 000A 
0083
* 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0006 FFFF 0000 00BB 
0083

- 0121 0000 020B 0000 01FF 0000 001F 0000 01FF 0000 01FF 0000 0000 008F 0000 
000B
+ 0121 0000 0321 0000 01FF 0000 001F 0000 01FF 0000 01FF 0000 0000 008F 0000 
000B
* 0121 0000 0321 0000 01FF 0000 001F 0000 01FF 0000 01FF 0000 0000 008F 0000 
000B

- 0000 0001 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0002
+ 0000 0001 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0002
* 0000 0001 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0002

- 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0000
+ 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0000
* 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0000

- 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0000
+ 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0000
* 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0000

- 0000 0000 0000 0000 0000 0000 0000 0000 000B 000B 000B 000B 000B 000B 0000 
0000
+ 0000 0000 0000 0000 0000 0000 0000 0000 000B 000B 000B 000B 000B 000B 0000 
0000
* 0000 0000 0000 0000 0000 0000 0000 0000 000B 000B 000B 000B 000B 000B 0000 
0000

- 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0000
+ 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0000
* 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0000

- 0010 0011 0012 0013 0014 0015 0016 0017 0018 0000 0000 0000 0000 0000 0000 
0000
+ 0010 0011 0012 0013 0014 0015 0016 0017 0018 0000 0000 0000 0000 0000 0000 
0000
* 0010 0011 0012 0013 0014 0015 0016 0017 0018 0000 0000 0000 0000 0000 0000 
0000

The first block shows a difference in register 0x0E. Apparently the hardware
resets the value (from 0x008B) to 0x000A. Later switch-robo.ko overwrites the
value to 0x00BB (bitwise OR with 0x00B1).

The second block shows a difference in register 0x12. So here again the
hardware seems to reset the value (from 0x020B) to 0x0321. Without the patch
this value remains untouched.

I can not see any other differences. I'm a bit puzzled on how to continue from
here. If 0x0321 is the default value for that register on the switch, I can
understand that it gets set somewhere in CFE boot to 0x020B. The switch keeps
the settings until switch-robo is loaded, which resets the switch, but never
alters the value again. The problem is, I don't seem to be able to find the
code in bcmrobo.c that would alter the value in that register anywhere...

Could someone post the value for registry 0x12 on the n66u ?

ps. Am I the only one with e3200 device perceiving this oddity?

Thanks & regards,
Tijs
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to