On Wednesday 27 February 2013 14:49:58 Tijs Van Buggenhout wrote:
> 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
>
Been testing on this some more, but not clear to me what exactly is the
problem. I can say it is related to resetting the switch via gpio.. Somehow
switch driver is behaving differently from the et driver, although the code
looks the same. Maybe some other registry value that influences the reset? If
so I can not find it in the etcgmac.c/bcmrobo.c source code.
At boot-up from CFE or normal linksys firmware, I can see that when the switch
has just been reset, the led control reg (still) has the right value, as the
leds behave normally afterwards. Compared to openwrt boot, this is the
point where they get misconfigured.
I tryed changing the code in switch-robo.c and bcma/driver_gpio.c to better
match the original specs, by loosing the additional gpio_set_value, but to
no avail:
--- package/switch/src/switch-robo.c 2013-03-11 20:16:21.918608668 +0100
+++ package/switch/src/switch-robo.c 2013-03-11 20:35:02.225019136 +0100
@@ -363,7 +363,6 @@
}
gpio_set_value(robo.gpio_robo_reset, 0);
gpio_direction_output(robo.gpio_robo_reset, 1);
- gpio_set_value(robo.gpio_robo_reset, 0);
mdelay(50);
gpio_set_value(robo.gpio_robo_reset, 1);
--- a/drivers/bcma/driver_gpio.c 2013-02-26 11:06:07.677236710 +0100
+++ b/drivers/bcma/driver_gpio.c 2013-03-11 20:33:50.677980619 +0100
@@ -48,7 +48,7 @@
struct bcma_drv_cc *cc = bcma_gpio_get_cc(chip);
bcma_chipco_gpio_outen(cc, 1 << gpio, 1 << gpio);
- bcma_chipco_gpio_out(cc, 1 << gpio, value ? 1 << gpio : 0);
+/* bcma_chipco_gpio_out(cc, 1 << gpio, value ? 1 << gpio : 0); */
return 0;
}
When leaving out the gpio_set_values alltogether, the led control register
stays in tact.. At switch reset, I can see that the ack led blinks when
gpio_direction_output is called, but that probably isn't enough for a
real/full reset.
Hence I came up with the following workaround, which should work for all
devices (I hope), to read the led control register before the reset, and
compare it with the value after the reset. If it is changed, restore it back
to the original. This ofcourse assumes that prior to resetting the switch, the
led control register is in correct state. I guess this should be reasonable,
as the system can't boot without CFE, in which the switch is initialised...
Any thoughts about this?
Thanks,
Tijs
---
--- a/package/switch/src/switch-robo.c 2013-03-11 20:16:21.918608668 +0100
+++ b/package/switch/src/switch-robo.c 2013-03-11 20:17:55.000168121 +0100
@@ -355,6 +355,8 @@
robo.gpio_robo_reset = get_gpio_pin("robo_reset");
if (robo.gpio_robo_reset >= 0) {
+ u16 led_ctl, led_ctl_chk;
+ led_ctl = robo_read16(ROBO_CTRL_PAGE, 0x12);
err = gpio_request(robo.gpio_robo_reset, "robo_reset");
if (err) {
printk(KERN_ERR PFX "error (%i) requesting robo_reset
gpio (%i)\n",
@@ -368,6 +370,9 @@
gpio_set_value(robo.gpio_robo_reset, 1);
mdelay(20);
+ led_ctl_chk = robo_read16(ROBO_CTRL_PAGE, 0x12);
+ if (led_ctl != led_ctl_chk)
+ robo_write16(ROBO_CTRL_PAGE, 0x12, led_ctl);
} else {
// TODO: reset the internal robo switch
}
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
