Le 04/05/2013 15:30, Сергей Василюгин a écrit :
> 
> 
> 04.05.2013, 18:39, "John Crispin" <[email protected]>:
>> Hi
>>
>> i just pushed a fix that addresses this problem properly ... (i hope)
>>
> 
> 
> +       of_property_read_string(np, "ralink,uartmux", &uart);
> +       if (uart) {
> +               mode |= rt_gpio_pinmux.uart_mask << rt_gpio_pinmux.uart_shift;
> +               if (ralink_mux_mask(uart, rt_gpio_pinmux.uart, &m)) {
> +                       pr_err("pinmux: failed to load uartmux \"%s\"\n", 
> uart);
> +               } else {
> +                       if (m != rt_gpio_pinmux.uart_mask)
> +                               mode &= ~(m << rt_gpio_pinmux.uart_shift);
> +                       pr_debug("pinmux: registered uartmux \"%s\"\n", uart);
> +               }
> +       }
> 
> No need to check. This code obviously set inverted modes: 0->7 (uartf->gpio), 
> 1->6 and so on. I think it should look like:
> 
> +       of_property_read_string(np, "ralink,uartmux", &uart);
> +       if (uart) {
> +               mode |= rt_gpio_pinmux.uart_mask << 
> rt_gpio_pinmux.uart_shift;                      /* set default gpio mode*/
> +               if (ralink_mux_mask(uart, rt_gpio_pinmux.uart, &m)) {
> +                       pr_err("pinmux: failed to load uartmux \"%s\, set 
> \"gpio\" mode"\n", uart);
> +               } else {
> +                       mode &= ~(rt_gpio_pinmux.uart_mask << 
> rt_gpio_pinmux.uart_shift);        /* clean gpio mode */
> +                       mode |= (m << rt_gpio_pinmux.uart_shift);             
>                                  /* and set requested mode*/
> +                       pr_debug("pinmux: registered uartmux \"%s\"\n", uart);
> +               }
> +       }
> 
> or 
> 
> +       of_property_read_string(np, "ralink,uartmux", &uart);
> +       if (uart) {
> +               if (ralink_mux_mask(uart, rt_gpio_pinmux.uart, &m)) {
> +                       mode |= rt_gpio_pinmux.uart_mask << 
> rt_gpio_pinmux.uart_shift;             /* set default gpio mode*/
> +                       pr_err("pinmux: failed to load uartmux \"%s\, set 
> \"gpio\" mode"\n", uart);
> +               } else {
> +                       mode |= (m << rt_gpio_pinmux.uart_shift);             
>                                  /* sipmly set requested mode*/
> +                       pr_debug("pinmux: registered uartmux \"%s\"\n", uart);
> +               }
> +       }
> 
> Am I right?

Yes, the pushed code only works correctly for gpio mode.

My preference goes to the more explicit second proposal, where you clearly see 
the default gpio mode and don't perform an undo!

-Michel
> 
> ---
> serge
> _______________________________________________
> openwrt-devel mailing list
> [email protected]
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> 
_______________________________________________
openwrt-devel mailing list
[email protected]
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to