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

[Kristian Evensen]

On Wed, Sep 6, 2017 at 10:07 AM, Mathias Kresin <dev at kresin.me> wrote:
> 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.

Thanks for the comments. Preparing a v3 now, will send it after some
quick testing.

-Kristian