Let make problem simple. We want LINK led can show speed status and keep light not flashing. The GBE0__ACT led show yellow when data transport will flashing,but it not always flashing.
Our spec
1000 LINK LED : orange color without flash
100 LINK LED : green color without flash
GBE0_ACT : when data has transport flash led and color is yellow
Yes, I already knew your spec. Please confirm if my previous comment can work or not. I don’t see your update yet.
Some forum users told us that the function of GBE_LINK_1000 and GBE_LINK_ACT seems need to be swapped but we cannot verify it since devkit only has 2 LED, so need your kindly help to verify.
That line just indicates to read shadow register BCM89610_SHD_LEDS1 and return value to reg.
It is from upstream kernel. Please refer to below.
For Broadcom phy, there are two modes of LED modes.
LOM-LED mode. This is the default mode which uses only 2 LED to control. The behavior is like
1000M → led1 ON, led2 OFF
100M → led1 OFF, led2 ON
10 M → Both OFF
However, datasheet does not mention about activity mode under such case.
Default LED mode. This is what the code patch we are doing, which uses 3 LEDS.
1000M → led1 ON, led2 ON
100 M → led1 ON, led2 OFF
10 M → led1 OFF, led2 ON.
It is like you are writing the previous old setting from BCM89610_SHD_LEDS1 to BCM89610_SHD_LEDS2.
This line has undefined behavior to me since I can only see these 3 lines code here.
To make it easier to understand, you could just take
BCM89610_SHD_LEDS1_LED1 as LED1 → activity led which is your yellow.
BCM89610_SHD_LEDS1_LED2 as LED2 → 100 M led which is your green.
BCM89610_SHD_LEDS2_LED3 as LED3 → 1000M led which is your orange.
(actually, the color does not matter, you could just say 100/1000 and act led)
so each programming would assign each LED a function.
First, I don’t know what would be the default value of “reg” in your code because you only share one line here.
bcm_phy_write_shadow(phydev,BCM89610_SHD_LEDS2,reg); // what are you writing for BCM89610_SHD_LEDS2?
Also,
reg = bcm_phy_read_shadow(phydev, BCM89610_SHD_LEDS1); // you read the value from LEDS1 register to reg
bcm_phy_write_shadow(phydev, BCM89610_SHD_LEDS1,reg); // you write the same value in reg back to LEDS1 again. Why to do this?
I would like to address again. In #27, I shared the LED modes for you. This is defined by Broadcom hardware (phy) and the only thing you could do is to assign each LED a function.
Whether it would light up depends on Broadcom’s definition. If you want to further understand any detail, please contact Cypress(Broadcom) for the datasheet.
Please notice that BCM89610_SHD_LEDS2 and BCM89610_SHD_LEDS1 are just register.
The bit 0~3 in BCM89610_SHD_LEDS1 indicates LED1 while the bit 4~7 in BCM89610_SHD_LEDS1 indicates LED2.
That is why there are some functions defined to shift bit.
Same rule applies to BCM89610_SHD_LEDS2 for LED3 and LED4. However, we only have 3 LED pinout, so just ignore LED4.
This one looks better now. But still some suggestions for you.
Please share the full code you added to the driver. In #5, there are some extra codes in front of these line and those are necessary too. I don’t know if you add that or not.
Also, I think it would be better stopping telling us the color. It is easy to get confused because the color does not mean anything. Please just tell us your LED mapping.
For example, a sentence like GBE_LINK_ACT flashing, GBE_LINK_100 ON and GBE_LINK_1000 OFF.
following function i added pr_info() to see speed.
like wise i want to see led status.
ex:yellow -on/off.
so which function containing this information
static int bcm5482_read_status(struct phy_device *phydev)
{
int err;
err = genphy_read_status(phydev);
if (phydev->dev_flags & PHY_BCM_FLAGS_MODE_1000BX) {
/*
* Only link status matters for 1000Base-X mode, so force
* 1000 Mbit/s full-duplex status
*/
if (phydev->link) {
phydev->speed = SPEED_1000;
pr_info("file:%s ,line:%d, speed:%d \n",__FILE__,__LINE__,phydev->speed);
phydev->duplex = DUPLEX_FULL;
}
}
return err;