[LEDE-DEV] [PATCH] ramips: Add support for the HNET C108

[Mathias Kresin]

06.09.2017 09:26, Kristian Evensen:
> On Wed, Sep 6, 2017 at 9:03 AM, Mathias Kresin <dev at kresin.me> wrote:
>>> * Wifi LED. It is always switched on.
>>
>>
>> Always switched on in terms of no relation to the up/down state of the
>> wireless interface or it doesn't blink on activity?
>>
>> Is the LED connected to the SoC? Have you tried to set the "wled" group to
>> the gpio function? The wled group is only GPIO#72 (&gpio3 0).
> 
> No relation to the state. I tried to set the wled group + set gpio 72
> to high/low, but it had no effect. Will update comment.

Might be that the LED is connected to VCC and not controllable at all. 
Strange hw design but who knows..

> 
>>> diff --git a/target/linux/ramips/base-files/etc/diag.sh
>>> b/target/linux/ramips/base-files/etc/diag.sh
>>> index 960e189283..7b267a6854 100644
>>> --- a/target/linux/ramips/base-files/etc/diag.sh
>>> +++ b/target/linux/ramips/base-files/etc/diag.sh
>>> @@ -296,6 +296,9 @@ get_status_led() {
>>>          zbt-wg3526-32M)
>>>                  status_led="zbt-wg3526:green:status"
>>>                  ;;
>>> +       c108)
>>> +               status_len="$board:green:lan"
>>> +               ;;
>>
>>
>> Please keep alphabetical order!
> 
> I was trying to figure out the lexicographical order in this file :)
> The different cases seems to group together devices with different
> names, so I thought it safest to place my entry at the end since I did
> not find another device with same LED name. Any suggestions for a
> different location (or LED name) are more than welcome.

Yeah, the file is out of alphabetical order again. Please add the c108 
to line 106, right before the block starting with cf-wr800n.

> 
> I missed the typo ("status_len"), so I will fix that for a v3.
> 
>>> +               power_modem {
>>> +                       gpio-export,name = "power_modem";
>>> +                       gpio-export,output = <GPIO_ACTIVE_LOW>;
>>
>>
>> You set the output value to 1 here. Please use gpio-export,output = 1.
> 
> Based on feedback I have got on earlier patches, it is preferred to
> use the _LOW/_HIGH constants than 0/1 directly.

Most likely it was me who gave that advice. It was meant for the gpios 
device tree property. Here you are setting a value instead of describing 
active state. If there would be a macro, it should be something like 
GPIO_LOW or just LOW.

Mathias